Re: [PATCH] bcachefs: debug: Fix copy_to_user() error code in flush_buf()

[Kent Overstreet]

On Fri, Sep 15, 2023 at 03:56:26PM +0300, Dan Carpenter wrote:
> The copy_to_user() function returns the number of bytes remaining to
> be copied but we want to return -EFAULT to the user.
> 
> Fixes: e0750d947352 ("bcachefs: Initial commit")
> Signed-off-by: Dan Carpenter <dan.carpen...@linaro.org>
> ---
>  fs/bcachefs/debug.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/bcachefs/debug.c b/fs/bcachefs/debug.c
> index 404148bd348a..e65c0ab0c9ad 100644
> --- a/fs/bcachefs/debug.c
> +++ b/fs/bcachefs/debug.c
> @@ -319,10 +319,9 @@ static ssize_t flush_buf(struct dump_iter *i)
>  {
>       if (i->buf.pos) {
>               size_t bytes = min_t(size_t, i->buf.pos, i->size);
> -             int err = copy_to_user(i->ubuf, i->buf.buf, bytes);
>  
> -             if (err)
> -                     return err;
> +             if (copy_to_user(i->ubuf, i->buf.buf, bytes))
> +                     return -EFAULT;
>  
>               i->ret   += bytes;
>               i->ubuf  += bytes;
> -- 
> 2.39.2
> 

Applying this fix instead:

>From 60a714b71846c3ea95ccad6699658890b24969c2 Mon Sep 17 00:00:00 2001
From: Kent Overstreet <kent.overstr...@linux.dev>
Date: Tue, 19 Sep 2023 17:09:22 -0400
Subject: [PATCH] bcachefs: Fix copy_to_user() usage in flush_buf()

copy_to_user() returns the number of bytes successfully copied - not an
errcode.

Reported-by: Dan Carpenter <dan.carpen...@linaro.org>
Signed-off-by: Kent Overstreet <kent.overstr...@linux.dev>

diff --git a/fs/bcachefs/debug.c b/fs/bcachefs/debug.c
index 404148bd348a..2c365bf25aca 100644
--- a/fs/bcachefs/debug.c
+++ b/fs/bcachefs/debug.c
@@ -319,16 +319,19 @@ static ssize_t flush_buf(struct dump_iter *i)
 {
        if (i->buf.pos) {
                size_t bytes = min_t(size_t, i->buf.pos, i->size);
-               int err = copy_to_user(i->ubuf, i->buf.buf, bytes);
+               int copied = copy_to_user(i->ubuf, i->buf.buf, bytes);
 
                if (err)
                        return err;
 
-               i->ret   += bytes;
-               i->ubuf  += bytes;
-               i->size  -= bytes;
-               i->buf.pos -= bytes;
-               memmove(i->buf.buf, i->buf.buf + bytes, i->buf.pos);
+               i->ret   += copied;
+               i->ubuf  += copied;
+               i->size  -= copied;
+               i->buf.pos -= copied;
+               memmove(i->buf.buf, i->buf.buf + copied, i->buf.pos);
+
+               if (copied != bytes)
+                       return -EFAULT;
        }
 
        return i->size ? 0 : i->ret;