| Message ID | 1391041651-15689-6-git-send-email-Gerhard@Heift.Name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-01-30 Gerhard Heift <gerhard@heift.name>: > By copying each found item seperatly to userspace, we do not need extra > memory in the kernel. This allows to run a large search inside of a single call. > > Signed-off-by: Gerhard Heift <Gerhard@Heift.Name> > --- > fs/btrfs/ioctl.c | 52 ++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 36 insertions(+), 16 deletions(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 10b9931..6daf23b 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1855,7 +1855,7 @@ static noinline int copy_to_sk(struct btrfs_root *root, > struct btrfs_key *key, > struct btrfs_ioctl_search_key *sk, > size_t *buf_size, > - char *buf, > + char __user *buf, > unsigned long *sk_offset, > int *num_found) > { > @@ -1890,7 +1890,7 @@ static noinline int copy_to_sk(struct btrfs_root *root, > if (sizeof(sh) + item_len > *buf_size) { > if (*num_found) { > ret = 1; > - goto overflow; > + goto err; > } > > /* > @@ -1905,7 +1905,7 @@ static noinline int copy_to_sk(struct btrfs_root *root, > > if (sizeof(sh) + item_len + *sk_offset > *buf_size) { > ret = 1; > - goto overflow; > + goto err; > } > > sh.objectid = key->objectid; > @@ -1915,20 +1915,28 @@ static noinline int copy_to_sk(struct btrfs_root *root, > sh.transid = found_transid; > > /* copy search result header */ > - memcpy(buf + *sk_offset, &sh, sizeof(sh)); > + if (copy_to_user(buf + *sk_offset, &sh, sizeof(sh))) { > + ret = -EFAULT; > + goto err; > + } > + > *sk_offset += sizeof(sh); > > if (item_len) { > - char *p = buf + *sk_offset; > + char __user *p = buf + *sk_offset; > /* copy the item */ > - read_extent_buffer(leaf, p, > - item_off, item_len); > + if (read_extent_buffer_to_user(leaf, p, > + item_off, item_len)) { > + ret = -EFAULT; > + goto err; > + } > + > *sk_offset += item_len; > } > (*num_found)++; > > if (ret) /* -EOVERFLOW from above */ > - goto overflow; > + goto err; > > if (*num_found >= sk->nr_items) > break; > @@ -1946,14 +1954,24 @@ advance_key: > key->objectid++; > } else > ret = 1; > -overflow: > +err: > + /* > + * 0: all items from this leaf copied, continue with next > + * 1: * more items can be copied, but unused buffer is too small > + * * all items were found > + * Either way, it will stops the loop which iterates to the next > + * leaf > + * -EOVERFLOW: item was to large for buffer > + * -EFAULT: could not copy extent buffer back to userspace > + */ > + > return ret; > } > > static noinline int search_ioctl(struct inode *inode, > struct btrfs_ioctl_search_key *sk, > size_t *buf_size, > - char *buf) > + char __user *buf) > { > struct btrfs_root *root; > struct btrfs_key key; > @@ -2004,6 +2022,7 @@ static noinline int search_ioctl(struct inode *inode, > ret = copy_to_sk(root, path, &key, sk, buf_size, buf, > &sk_offset, &num_found); > btrfs_release_path(path); > + > if (ret || num_found >= sk->nr_items) > break; > > @@ -2019,7 +2038,8 @@ err: > static noinline int btrfs_ioctl_tree_search(struct file *file, > void __user *argp) > { > - struct btrfs_ioctl_search_args *args; > + struct btrfs_ioctl_search_args __user *args; > + struct btrfs_ioctl_search_key sk; > struct inode *inode; > int ret; > size_t buf_size; > @@ -2027,14 +2047,15 @@ static noinline int btrfs_ioctl_tree_search(struct file *file, > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > > - args = memdup_user(argp, sizeof(*args)); > - if (IS_ERR(args)) > - return PTR_ERR(args); > + args = (struct btrfs_ioctl_search_args __user *)argp; > + > + if (copy_from_user(&sk, &args->key, sizeof(sk))) > + return -EFAULT; > > buf_size = sizeof(args->buf); > > inode = file_inode(file); > - ret = search_ioctl(inode, &args->key, &buf_size, args->buf); > + ret = search_ioctl(inode, &sk, &buf_size, args->buf); > > /* > * In the origin implementation an overflow is handled by returning a > @@ -2045,7 +2066,6 @@ static noinline int btrfs_ioctl_tree_search(struct file *file, > > if (ret == 0 && copy_to_user(argp, args, sizeof(*args))) Here I forgot to replace the full copy with only copying the search key. The correct copy looks like this: copy_to_user(&args->key, &sk, sizeof(sk)) I will post another version. > ret = -EFAULT; > - kfree(args); > return ret; > } > > -- > 1.8.5.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 10b9931..6daf23b 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1855,7 +1855,7 @@ static noinline int copy_to_sk(struct btrfs_root *root, struct btrfs_key *key, struct btrfs_ioctl_search_key *sk, size_t *buf_size, - char *buf, + char __user *buf, unsigned long *sk_offset, int *num_found) { @@ -1890,7 +1890,7 @@ static noinline int copy_to_sk(struct btrfs_root *root, if (sizeof(sh) + item_len > *buf_size) { if (*num_found) { ret = 1; - goto overflow; + goto err; } /* @@ -1905,7 +1905,7 @@ static noinline int copy_to_sk(struct btrfs_root *root, if (sizeof(sh) + item_len + *sk_offset > *buf_size) { ret = 1; - goto overflow; + goto err; } sh.objectid = key->objectid; @@ -1915,20 +1915,28 @@ static noinline int copy_to_sk(struct btrfs_root *root, sh.transid = found_transid; /* copy search result header */ - memcpy(buf + *sk_offset, &sh, sizeof(sh)); + if (copy_to_user(buf + *sk_offset, &sh, sizeof(sh))) { + ret = -EFAULT; + goto err; + } + *sk_offset += sizeof(sh); if (item_len) { - char *p = buf + *sk_offset; + char __user *p = buf + *sk_offset; /* copy the item */ - read_extent_buffer(leaf, p, - item_off, item_len); + if (read_extent_buffer_to_user(leaf, p, + item_off, item_len)) { + ret = -EFAULT; + goto err; + } + *sk_offset += item_len; } (*num_found)++; if (ret) /* -EOVERFLOW from above */ - goto overflow; + goto err; if (*num_found >= sk->nr_items) break; @@ -1946,14 +1954,24 @@ advance_key: key->objectid++; } else ret = 1; -overflow: +err: + /* + * 0: all items from this leaf copied, continue with next + * 1: * more items can be copied, but unused buffer is too small + * * all items were found + * Either way, it will stops the loop which iterates to the next + * leaf + * -EOVERFLOW: item was to large for buffer + * -EFAULT: could not copy extent buffer back to userspace + */ + return ret; } static noinline int search_ioctl(struct inode *inode, struct btrfs_ioctl_search_key *sk, size_t *buf_size, - char *buf) + char __user *buf) { struct btrfs_root *root; struct btrfs_key key; @@ -2004,6 +2022,7 @@ static noinline int search_ioctl(struct inode *inode, ret = copy_to_sk(root, path, &key, sk, buf_size, buf, &sk_offset, &num_found); btrfs_release_path(path); + if (ret || num_found >= sk->nr_items) break; @@ -2019,7 +2038,8 @@ err: static noinline int btrfs_ioctl_tree_search(struct file *file, void __user *argp) { - struct btrfs_ioctl_search_args *args; + struct btrfs_ioctl_search_args __user *args; + struct btrfs_ioctl_search_key sk; struct inode *inode; int ret; size_t buf_size; @@ -2027,14 +2047,15 @@ static noinline int btrfs_ioctl_tree_search(struct file *file, if (!capable(CAP_SYS_ADMIN)) return -EPERM; - args = memdup_user(argp, sizeof(*args)); - if (IS_ERR(args)) - return PTR_ERR(args); + args = (struct btrfs_ioctl_search_args __user *)argp; + + if (copy_from_user(&sk, &args->key, sizeof(sk))) + return -EFAULT; buf_size = sizeof(args->buf); inode = file_inode(file); - ret = search_ioctl(inode, &args->key, &buf_size, args->buf); + ret = search_ioctl(inode, &sk, &buf_size, args->buf); /* * In the origin implementation an overflow is handled by returning a @@ -2045,7 +2066,6 @@ static noinline int btrfs_ioctl_tree_search(struct file *file, if (ret == 0 && copy_to_user(argp, args, sizeof(*args))) ret = -EFAULT; - kfree(args); return ret; }
By copying each found item seperatly to userspace, we do not need extra memory in the kernel. This allows to run a large search inside of a single call. Signed-off-by: Gerhard Heift <Gerhard@Heift.Name> --- fs/btrfs/ioctl.c | 52 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 16 deletions(-)