diff mbox

[2/2] wireless: zd1211rw: fix NULL-deref at probe

Message ID 20170313124421.28587-2-johan@kernel.org (mailing list archive)
State Accepted
Commit ca260ece6a57dc7d751e0685f51fa2c55d851873
Delegated to: Kalle Valo
Headers show

Commit Message

Johan Hovold March 13, 2017, 12:44 p.m. UTC
Make sure to check the number of endpoints to avoid dereferencing a
NULL-pointer or accessing memory beyond the endpoint array should a
malicious device lack the expected endpoints.

Fixes: a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM device
into WLAN device")
Cc: Daniel Drake <dsd@gentoo.org>

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/wireless/zydas/zd1211rw/zd_usb.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Kalle Valo March 22, 2017, 9:04 a.m. UTC | #1
Johan Hovold <johan@kernel.org> wrote:
> Make sure to check the number of endpoints to avoid dereferencing a
> NULL-pointer or accessing memory beyond the endpoint array should a
> malicious device lack the expected endpoints.
> 
> Fixes: a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM device into WLAN device")
> Cc: Daniel Drake <dsd@gentoo.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Patch applied to wireless-drivers-next.git, thanks.

ca260ece6a57 zd1211rw: fix NULL-deref at probe
Johan Hovold March 22, 2017, 12:45 p.m. UTC | #2
On Wed, Mar 22, 2017 at 09:04:15AM +0000, Kalle Valo wrote:
> Johan Hovold <johan@kernel.org> wrote:
> > Make sure to check the number of endpoints to avoid dereferencing a
> > NULL-pointer or accessing memory beyond the endpoint array should a
> > malicious device lack the expected endpoints.
> > 
> > Fixes: a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM device into WLAN device")
> > Cc: Daniel Drake <dsd@gentoo.org>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> Patch applied to wireless-drivers-next.git, thanks.
> 
> ca260ece6a57 zd1211rw: fix NULL-deref at probe

What about patch 1/2 which fixes the same bug (literally copied from the
zd1211rw driver)?

And as these fixes should be backported to stable (I left out the tag
for networking drivers), why only apply to -next?

Thanks,
Johan
Kalle Valo March 22, 2017, 1:02 p.m. UTC | #3
Johan Hovold <johan@kernel.org> writes:

> On Wed, Mar 22, 2017 at 09:04:15AM +0000, Kalle Valo wrote:
>> Johan Hovold <johan@kernel.org> wrote:
>> > Make sure to check the number of endpoints to avoid dereferencing a
>> > NULL-pointer or accessing memory beyond the endpoint array should a
>> > malicious device lack the expected endpoints.
>> > 
>> > Fixes: a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM
>> > device into WLAN device")
>> > Cc: Daniel Drake <dsd@gentoo.org>
>> > Signed-off-by: Johan Hovold <johan@kernel.org>
>> 
>> Patch applied to wireless-drivers-next.git, thanks.
>> 
>> ca260ece6a57 zd1211rw: fix NULL-deref at probe
>
> What about patch 1/2 which fixes the same bug (literally copied from the
> zd1211rw driver)?

I will apply that to my separate ath.git tree, just didn't get to your
patch yet.

> And as these fixes should be backported to stable (I left out the tag
> for networking drivers)

Actually for wireless drivers you should add the stable tag.

> why only apply to -next?

I didn't see that the fix was important enough for 4.11.
Johan Hovold March 22, 2017, 1:24 p.m. UTC | #4
On Wed, Mar 22, 2017 at 03:02:12PM +0200, Kalle Valo wrote:
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Wed, Mar 22, 2017 at 09:04:15AM +0000, Kalle Valo wrote:
> >> Johan Hovold <johan@kernel.org> wrote:
> >> > Make sure to check the number of endpoints to avoid dereferencing a
> >> > NULL-pointer or accessing memory beyond the endpoint array should a
> >> > malicious device lack the expected endpoints.
> >> > 
> >> > Fixes: a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM
> >> > device into WLAN device")
> >> > Cc: Daniel Drake <dsd@gentoo.org>
> >> > Signed-off-by: Johan Hovold <johan@kernel.org>
> >> 
> >> Patch applied to wireless-drivers-next.git, thanks.
> >> 
> >> ca260ece6a57 zd1211rw: fix NULL-deref at probe
> >
> > What about patch 1/2 which fixes the same bug (literally copied from the
> > zd1211rw driver)?
> 
> I will apply that to my separate ath.git tree, just didn't get to your
> patch yet.

Ah, ok. 

> > And as these fixes should be backported to stable (I left out the tag
> > for networking drivers)
> 
> Actually for wireless drivers you should add the stable tag.

Alright, will do in the future.

> > why only apply to -next?
> 
> I didn't see that the fix was important enough for 4.11.

Ok, but fixes for these types of crashes that can be triggered by a
malicious device have typically gone into the current -rc (a couple just
went in through the net tree for example).

Thanks,
Johan
diff mbox

Patch

diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_usb.c b/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
index c5effd6c6be9..01ca1d57b3d9 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
@@ -1278,6 +1278,9 @@  static int eject_installer(struct usb_interface *intf)
 	u8 bulk_out_ep;
 	int r;
 
+	if (iface_desc->desc.bNumEndpoints < 2)
+		return -ENODEV;
+
 	/* Find bulk out endpoint */
 	for (r = 1; r >= 0; r--) {
 		endpoint = &iface_desc->endpoint[r].desc;