On Thu, Sep 14, 2023 at 05:58:07PM +0300, Dan Carpenter wrote:
> The copy_to_user() function returns the number of bytes remaining but
> we want to return -EFAULT to the user.
>
> Fixes: e0750d947352 ("bcachefs: Initial commit")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Thanks - never liked this about copy_to_user(), that behaviour is
practically never what we want, maybe we could get a helper that returns
the proper error code someday...
> ---
> fs/bcachefs/chardev.c | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
> index fb603df099a5..e5e9fddddfb5 100644
> --- a/fs/bcachefs/chardev.c
> +++ b/fs/bcachefs/chardev.c
> @@ -149,9 +149,10 @@ static long bch2_global_ioctl(unsigned cmd, void __user *arg)
> static long bch2_ioctl_query_uuid(struct bch_fs *c,
> struct bch_ioctl_query_uuid __user *user_arg)
> {
> - return copy_to_user(&user_arg->uuid,
> - &c->sb.user_uuid,
> - sizeof(c->sb.user_uuid));
> + if (copy_to_user(&user_arg->uuid, &c->sb.user_uuid,
> + sizeof(c->sb.user_uuid)))
> + return -EFAULT;
> + return 0;
> }
>
> #if 0
> @@ -338,7 +339,10 @@ static ssize_t bch2_data_job_read(struct file *file, char __user *buf,
> if (len < sizeof(e))
> return -EINVAL;
>
> - return copy_to_user(buf, &e, sizeof(e)) ?: sizeof(e);
> + if (copy_to_user(buf, &e, sizeof(e)))
> + return -EFAULT;
> +
> + return sizeof(e);
> }
>
> static const struct file_operations bcachefs_data_ops = {
> @@ -466,9 +470,11 @@ static long bch2_ioctl_fs_usage(struct bch_fs *c,
> percpu_up_read(&c->mark_lock);
> kfree(src);
>
> - if (!ret)
> - ret = copy_to_user(user_arg, arg,
> - sizeof(*arg) + arg->replica_entries_bytes);
> + if (ret)
> + goto err;
> + if (copy_to_user(user_arg, arg,
> + sizeof(*arg) + arg->replica_entries_bytes))
> + ret = -EFAULT;
> err:
> kfree(arg);
> return ret;
> @@ -513,7 +519,10 @@ static long bch2_ioctl_dev_usage(struct bch_fs *c,
>
> percpu_ref_put(&ca->ref);
>
> - return copy_to_user(user_arg, &arg, sizeof(arg));
> + if (copy_to_user(user_arg, &arg, sizeof(arg)))
> + return -EFAULT;
> +
> + return 0;
> }
>
> static long bch2_ioctl_read_super(struct bch_fs *c,
> @@ -550,8 +559,9 @@ static long bch2_ioctl_read_super(struct bch_fs *c,
> goto err;
> }
>
> - ret = copy_to_user((void __user *)(unsigned long)arg.sb,
> - sb, vstruct_bytes(sb));
> + if (copy_to_user((void __user *)(unsigned long)arg.sb, sb,
> + vstruct_bytes(sb)))
> + ret = -EFAULT;
> err:
> if (!IS_ERR_OR_NULL(ca))
> percpu_ref_put(&ca->ref);
> --
> 2.39.2
>
