| Message ID | 20190322154425.3852517-5-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | candidates from spreadtrum 4.4 product kernel | expand |
On Fri, Mar 22, 2019 at 04:43:55PM +0100, Arnd Bergmann wrote: > From: Josh Boyer <jwboyer@fedoraproject.org> > > The iowarrior driver expects at least one valid endpoint. If given > malicious descriptors that specify 0 for the number of endpoints, > it will crash in the probe function. Ensure there is at least > one endpoint on the interface before using it. > > The full report of this issue can be found here: > http://seclists.org/bugtraq/2016/Mar/87 > > Reported-by: Ralf Spenneberg <ralf@spenneberg.net> > Cc: stable <stable@vger.kernel.org> > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry picked from commit 4ec0ef3a82125efc36173062a50624550a900ae0) > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/usb/misc/iowarrior.c | 6 ++++++ > 1 file changed, 6 insertions(+) This commit has been in the tree for a long time. It was in the 4.4.7 release, back in April 2016. And then it was reverted in commit b7321e81fc36 ("USB: iowarrior: fix NULL-deref at probe") as it broke systems. So why add it back, the correct functionality should be there today, right? thanks, greg k-h
On Tue, Mar 26, 2019 at 2:23 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Fri, Mar 22, 2019 at 04:43:55PM +0100, Arnd Bergmann wrote: > > From: Josh Boyer <jwboyer@fedoraproject.org> > > > > The iowarrior driver expects at least one valid endpoint. If given > > malicious descriptors that specify 0 for the number of endpoints, > > it will crash in the probe function. Ensure there is at least > > one endpoint on the interface before using it. > > > > The full report of this issue can be found here: > > http://seclists.org/bugtraq/2016/Mar/87 > > > > Reported-by: Ralf Spenneberg <ralf@spenneberg.net> > > Cc: stable <stable@vger.kernel.org> > > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > (cherry picked from commit 4ec0ef3a82125efc36173062a50624550a900ae0) > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/usb/misc/iowarrior.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > This commit has been in the tree for a long time. It was in the 4.4.7 > release, back in April 2016. And then it was reverted in commit > b7321e81fc36 ("USB: iowarrior: fix NULL-deref at probe") as it broke > systems. So why add it back, the correct functionality should be there > today, right? Sorry I missed that history. The script I used to identify patches noticed that this patch was not applied, but I did not have a check for already- reverted patches. Chunyan, Baolin: it seems the spreadtrum 4.4 kernel got this wrong as well, by backporting the patch again on top of 4.4.172. Can you check the latest internal version for this? Arnd
On Tue, 26 Mar 2019 at 16:21, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Mar 26, 2019 at 2:23 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Fri, Mar 22, 2019 at 04:43:55PM +0100, Arnd Bergmann wrote: > > > From: Josh Boyer <jwboyer@fedoraproject.org> > > > > > > The iowarrior driver expects at least one valid endpoint. If given > > > malicious descriptors that specify 0 for the number of endpoints, > > > it will crash in the probe function. Ensure there is at least > > > one endpoint on the interface before using it. > > > > > > The full report of this issue can be found here: > > > http://seclists.org/bugtraq/2016/Mar/87 > > > > > > Reported-by: Ralf Spenneberg <ralf@spenneberg.net> > > > Cc: stable <stable@vger.kernel.org> > > > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > (cherry picked from commit 4ec0ef3a82125efc36173062a50624550a900ae0) > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > --- > > > drivers/usb/misc/iowarrior.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > This commit has been in the tree for a long time. It was in the 4.4.7 > > release, back in April 2016. And then it was reverted in commit > > b7321e81fc36 ("USB: iowarrior: fix NULL-deref at probe") as it broke > > systems. So why add it back, the correct functionality should be there > > today, right? > > Sorry I missed that history. The script I used to identify patches noticed > that this patch was not applied, but I did not have a check for already- > reverted patches. > > Chunyan, Baolin: it seems the spreadtrum 4.4 kernel got this wrong > as well, by backporting the patch again on top of 4.4.172. Can you check > the latest internal version for this? Yes, I saw this patch in our 4.4 kernel. Orson, we should revert this patch from our kernel as Greg mentioned.
Hi Arnd, Baolin, Thank you! I am in travel. I 'll apply it when I am back. Best, Orson -------- Good patches are always welcome!
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c index 5e43fd881a9c..381a92a0ebb6 100644 --- a/drivers/usb/misc/iowarrior.c +++ b/drivers/usb/misc/iowarrior.c @@ -787,6 +787,12 @@ static int iowarrior_probe(struct usb_interface *interface, iface_desc = interface->cur_altsetting; dev->product_id = le16_to_cpu(udev->descriptor.idProduct); + if (iface_desc->desc.bNumEndpoints < 1) { + dev_err(&interface->dev, "Invalid number of endpoints\n"); + retval = -EINVAL; + goto error; + } + /* set up the endpoint information */ for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { endpoint = &iface_desc->endpoint[i].desc;