RE: [patch 2/2] ACPI / debugger: copy_to_user doesn't return errors

Zheng, Lv
2015-12-25T03:25:23Z

Hi,

> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Friday, December 25, 2015 3:16 AM
> Subject: Re: [patch 2/2] ACPI / debugger: copy_to_user doesn't return errors
>
> On Thu, Dec 24, 2015 at 08:39:06AM +0100, Julia Lawall wrote:
> > On Thu, 24 Dec 2015, Dan Carpenter wrote:
> >
> > > The copy_to/from_user() functions don't return error codes, they return
> > > the number of bytes remaining. We had intended to return -EFUALT here.
> > > We actually have already checked access_ok() in an earlier function so
> > > I don't think these functions will fail but let's fix it anyway.
> > >
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > >
> > > diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
> > > index f2c92ab..2a1777b 100644
> > > --- a/drivers/acpi/acpi_dbg.c
> > > +++ b/drivers/acpi/acpi_dbg.c
> > > @@ -592,9 +592,10 @@ static int acpi_aml_read_user(char __user *buf,
> int len)
> > > smp_rmb();
> > > p = &crc->buf[crc->tail];
> > > n = min(len, circ_count_to_end(crc));
> > > - ret = copy_to_user(buf, p, n);
> > > - if (IS_ERR_VALUE(ret))
> >
> > I'm not familiar with IS_ERR_VALUE. Is it to allow functions that
> > return unsigned values to also return negative error codes?
>
> Yes, but here it is used as a substitue for checking if (ret < 0) or if
> (ret >= 0). I would have prefered to open code it. Also I prefer
> if (ret) for when the value is either zero or negative error codes but
> here we have standardized on IS_ERR_VALUE(ret) for everything.
[Lv Zheng]
This patch looks correct.
It's good for me to know the rule for copy_to/from_user().
Thanks for pointing out.

Thanks and best regards
-Lv

>
> regards,
> dan carpenter