On Mon, Jan 27, 2014 at 02:28:30PM +0100, Gerhard Heift wrote:
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2032,6 +2032,52 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
> return ret;
> }
>
> +static noinline int btrfs_ioctl_tree_search_v2(struct file *file,
> + void __user *argp)
> +{
> + struct btrfs_ioctl_search_args_v2 *args;
> + struct inode *inode;
> + int ret;
> + char *buf;
> + size_t buf_size;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + /* copy search header and buffer size */
> + args = memdup_user(argp, sizeof(*args));
> + if (IS_ERR(args))
> + return PTR_ERR(args);
> +
> + buf_size = args->buf_size;
> +
> + if (buf_size < sizeof(struct btrfs_ioctl_search_header)) {
> + kfree(args);
> + return -ENOMEM;
ENOMEM does not seem correct here, it's not a memory allocation failure
but rather an underflow of the buffer size, possibly EINVAL or EOVERFLOW
as the other checks return.
> + }
> +
> + /* limit memory */
> + if (buf_size > PAGE_SIZE * 32)
> + buf_size = PAGE_SIZE * 32;
> +
> + buf = memdup_user(argp->buf, buf_size);
Memory allocations are not that easy, getting a contiguous 32 * 4k = 128k
buffer may often fail. And the point of the V2 ioctl was to avoid
allocating the buffer, and do copy_to_user directly.
Also, you remove this code in the next patch, I don't think this level
of patch granularity is necessary.
> + if (IS_ERR(buf)) {
> + kfree(args);
> + return PTR_ERR(buf);
> + }
> +
> + inode = file_inode(file);
> + ret = search_ioctl(inode, &args->key, buf_size, buf);
> + if (ret == 0 && (
> + copy_to_user(argp, args, sizeof(*args)) ||
> + copy_to_user(argp->buf, buf, buf_size)
> + ))
> + ret = -EFAULT;
> + kfree(buf);
> + kfree(args);
> + return ret;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
