On 16/03/2018 1:06 AM, Bart Van Assche wrote:

On Fri, 2018-03-16 at 01:00 +0800, Coly Li wrote:

quoted

On 15/03/2018 11:08 PM, Bart Van Assche wrote:

quoted

copy_to_user() returns the number of remaining bytes. Avoid that
a larger value is returned than the number of bytes that have
been copied by returning -EFAULT if not all bytes have been copied.

Signed-off-by: Bart Van Assche <redacted>
---
 drivers/md/bcache/debug.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c
index af89408befe8..376899cfcbf1 100644
--- a/drivers/md/bcache/debug.c
+++ b/drivers/md/bcache/debug.c

@@ -175,9 +175,8 @@ static ssize_t bch_dump_read(struct file *file, char __user *buf,
 		struct keybuf_key *w;
 		unsigned bytes = min(i->bytes, size);
 
-		int err = copy_to_user(buf, i->buf, bytes);
-		if (err)
-			return err;
+		if (copy_to_user(buf, i->buf, bytes))
+			return -EFAULT;
 

Hi Bart,

I am not sure whether this change is correct. -EFAULT seems not an
expected return value of read(2), while -1 is the expected return value
when error occurs.

Maybe if copy_to_user() returns value in (0, size], "ret + (size - err)"
should be returned. An exception is when copy_to_user() returns 0 and
ret is 0 too, in this situation -1 should be returned.

Correct me if I am wrong.

Hello Coly,

I'm not familiar enough with bcache to provide the answer to your question so
I will drop this patch from this series.

Hi Bart,

It seems you catch a code bug here. I will look into this and handle it
here.

Thanks for the hint :-)

Coly Li