[BACKPORT,4.4.y,04/25] USB: iowarrior: fix oops with malicious USB descriptors

Message ID 20190322154425.3852517-5-arnd@arndb.de
State New
Headers show
Series
  • candidates from spreadtrum 4.4 product kernel
Related show

Commit Message

Arnd Bergmann March 22, 2019, 3:43 p.m.
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(+)

-- 
2.20.0

Comments

Greg KH March 26, 2019, 1:13 a.m. | #1
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
(Exiting) Baolin Wang March 26, 2019, 9:35 a.m. | #2
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.

-- 
Baolin Wang
Best Regards

Patch

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;