Re: [PATCH 13/16] bcache: Make bch_dump_read() fail if copying to user space fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2018-03-16 at 01:00 +0800, Coly Li wrote:
> On 15/03/2018 11:08 PM, Bart Van Assche wrote:
> > 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 <bart.vanassche@xxxxxxx>
> > ---
> >  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.

Thanks,

Bart.






[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux