| Message ID | ca6b79b47313aa7ee9d8c24c5a7f595772764171.1587690539.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [USB,1/2] usb: raw-gadget: fix return value of ep read ioctls | expand |
On Fri, Apr 24, 2020 at 03:09:58AM +0200, Andrey Konovalov wrote: > They must return the number of bytes transferred during the data stage. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > --- Is this a bugfix? If so, does it need to go to older kernels? A "Fixes:" tag would be nice... thanks, greg k-h
On Fri, Apr 24, 2020 at 03:09:58AM +0200, Andrey Konovalov wrote: > They must return the number of bytes transferred during the data stage. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> This was my mistake. Please add a Fixes tag. Fixes: 068fbff4f860 ("usb: raw-gadget: Fix copy_to/from_user() checks") I should have seen that bug... I thought I was being careful and I even singled out that part of the commit and mentioned it in the commit message but I messed up. Sorry. regards, dan carpenter
On Fri, Apr 24, 2020 at 10:43 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > On Fri, Apr 24, 2020 at 03:09:58AM +0200, Andrey Konovalov wrote: > > They must return the number of bytes transferred during the data stage. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > This was my mistake. Please add a Fixes tag. > > Fixes: 068fbff4f860 ("usb: raw-gadget: Fix copy_to/from_user() checks") > > I should have seen that bug... I thought I was being careful and I > even singled out that part of the commit and mentioned it in the > commit message but I messed up. Sorry. No worries, the bug was actually present before your change, but in a slightly different form. So FWIW we can also add: Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface") However raw-gadget is not present in any released kernel yet, so need of backporting AFAIU.
On Fri, Apr 24, 2020 at 03:16:35PM +0200, Andrey Konovalov wrote: > On Fri, Apr 24, 2020 at 10:43 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > On Fri, Apr 24, 2020 at 03:09:58AM +0200, Andrey Konovalov wrote: > > > They must return the number of bytes transferred during the data stage. > > > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > > > This was my mistake. Please add a Fixes tag. > > > > Fixes: 068fbff4f860 ("usb: raw-gadget: Fix copy_to/from_user() checks") > > > > I should have seen that bug... I thought I was being careful and I > > even singled out that part of the commit and mentioned it in the > > commit message but I messed up. Sorry. > > No worries, the bug was actually present before your change, but in a > slightly different form. So FWIW we can also add: > > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface") > > However raw-gadget is not present in any released kernel yet, so need > of backporting AFAIU. The Fixes: tag lets scripts know that it's not required to back port it. regards, dan carpenter
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c index ca7d95bf7397..7b241992ad5a 100644 --- a/drivers/usb/gadget/legacy/raw_gadget.c +++ b/drivers/usb/gadget/legacy/raw_gadget.c @@ -669,12 +669,14 @@ static int raw_ioctl_ep0_read(struct raw_dev *dev, unsigned long value) if (IS_ERR(data)) return PTR_ERR(data); ret = raw_process_ep0_io(dev, &io, data, false); - if (ret) + if (ret < 0) goto free; length = min(io.length, (unsigned int)ret); if (copy_to_user((void __user *)(value + sizeof(io)), data, length)) ret = -EFAULT; + else + ret = length; free: kfree(data); return ret; @@ -964,12 +966,14 @@ static int raw_ioctl_ep_read(struct raw_dev *dev, unsigned long value) if (IS_ERR(data)) return PTR_ERR(data); ret = raw_process_ep_io(dev, &io, data, false); - if (ret) + if (ret < 0) goto free; length = min(io.length, (unsigned int)ret); if (copy_to_user((void __user *)(value + sizeof(io)), data, length)) ret = -EFAULT; + else + ret = length; free: kfree(data); return ret;
They must return the number of bytes transferred during the data stage. Signed-off-by: Andrey Konovalov <andreyknvl@google.com> --- drivers/usb/gadget/legacy/raw_gadget.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)