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

[Kent Overstreet <kent.overstreet@xxxxxxxxx>]

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.carpenter@xxxxxxxxxx>
> ---
>  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.overstreet@xxxxxxxxx>
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.carpenter@xxxxxxxxxx>
Signed-off-by: Kent Overstreet <kent.overstreet@xxxxxxxxx>

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;