wireless: ath9k_htc: fix NULL-deref at probe

PATCH In review Johan Hovold 3 participants 13 messages Mar 13, 2017 – Apr 5, 2017

Subsystems (4)

Networking Network drivers Qualcomm Atheros ATH9K wireless driver ZD1211RW wireless driver

lore

Share & tools

lore

Johan Hovold

author

2017-03-13 12:44 UTC #0 lore raw

PATCH 1/2 wireless: ath9k_htc: fix NULL-deref at probe

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 36bcce430657 ("ath9k_htc: Handle storage devices")

Cc Sujith Manoharan <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/wireless/ath/ath9k/hif_usb.c +3 -0

index de2d212f39ec..9206955e865a 100644
@@ -1219,6 +1219,9 @@ static int send_eject_command(struct usb_interface *interface)
 	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;

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:44 UTC #1 lore raw

PATCH 2/2 wireless: zd1211rw: fix NULL-deref at probe

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 <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/net/wireless/zydas/zd1211rw/zd_usb.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/wireless/zydas/zd1211rw/zd_usb.c +3 -0

index c5effd6c6be9..01ca1d57b3d9 100644
@@ -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;

Signature · 1 line

2.12.0

Kalle Valo

2017-03-22 09:04 UTC #2 lore raw

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

Quoted 7 lines · Johan Hovold

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 <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

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

ca260ece6a57 zd1211rw: fix NULL-deref at probe

Signature · 6 lines

https://patchwork.kernel.org/patch/9620721/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Johan Hovold

author

2017-03-22 12:45 UTC #3 lore raw

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

Quoted 5 lines · Kalle Valo

Quoted 4 lines · Johan Hovold
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.

Quoted 7 lines

Quoted 3 lines
Fixes a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM device into WLAN device")
Cc Daniel Drake <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
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

2017-03-22 13:02 UTC #4 lore raw

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

Quoted 11 lines · Johan Hovold

Quoted 10 lines · Kalle Valo
Quoted 8 lines · Johan Hovold
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 <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Quoted 6 lines

Quoted 3 lines
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.

Quoted 2 lines

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.

Quoted 1 line

why only apply to -next?

I didn't see that the fix was important enough for 4.11.

Signature · 1 line

Kalle Valo

Johan Hovold

author

2017-03-22 13:24 UTC #5 lore raw

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

Quoted 15 lines · Kalle Valo

Quoted 13 lines · Johan Hovold
Quoted 12 lines · Kalle Valo
Quoted 8 lines · Johan Hovold
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 <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
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.

Quoted 4 lines

Quoted 2 lines
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.

Quoted 3 lines

Quoted 1 line
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

Johan Hovold

author

2017-04-03 08:42 UTC #6 lore raw

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Is this one still in your queue, Kalle?

As I mentioned earlier, I should have added a

Cc stable <[email protected]> # 2.6.39

but left it out as I mistakingly thought the net recommendations to do so applied also to wireless.

Thanks,
Johan

Kalle Valo

2017-04-03 09:34 UTC #7 lore raw

Quoted 10 lines · Johan Hovold

Quoted 7 lines · Johan Hovold
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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
Is this one still in your queue, Kalle?

Yes, I'm just lacking behing:

https://patchwork.kernel.org/patch/9620723/

Quoted 6 lines

As I mentioned earlier, I should have added a
Cc stable <[email protected]> # 2.6.39
but left it out as I mistakingly thought the net recommendations to do so applied also to wireless.

Ok, I'll add that.

Signature · 5 lines

Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kalle Valo

2017-04-03 13:02 UTC #8 lore raw

Quoted 11 lines · Kalle Valo

Quoted 9 lines · Johan Hovold
Quoted 7 lines · Johan Hovold
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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Quoted 5 lines

Quoted 1 line
Is this one still in your queue, Kalle?
Yes, I'm just lacking behing:
https://patchwork.kernel.org/patch/9620723/

Meant "lagging" of course. Mondays..

Quoted 8 lines

Quoted 6 lines
As I mentioned earlier, I should have added a
Cc stable <[email protected]> # 2.6.39
but left it out as I mistakingly thought the net recommendations to do so applied also to wireless.
Ok, I'll add that.

But is 2.6.39 really correct? Shouldn't it be 2.6.39+ so that it means all versions since 2.6.39?

Signature · 1 line

Kalle Valo

Johan Hovold

author

2017-04-03 13:16 UTC #9 lore raw

Quoted 25 lines · Kalle Valo

Quoted 11 lines · Kalle Valo
Quoted 9 lines · Johan Hovold
Quoted 7 lines · Johan Hovold
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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
Quoted 5 lines
Quoted 1 line
Is this one still in your queue, Kalle?
Yes, I'm just lacking behing:
https://patchwork.kernel.org/patch/9620723/
Meant "lagging" of course. Mondays..
Quoted 4 lines
Quoted 4 lines
As I mentioned earlier, I should have added a
Cc stable <[email protected]> # 2.6.39

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
but left it out as I mistakingly thought the net recommendations to do so applied also to wireless.
Ok, I'll add that.
But is 2.6.39 really correct? Shouldn't it be 2.6.39+ so that it means all versions since 2.6.39?

Either way is fine, the stable maintainers apply them to all later versions.

I notice now that adding a plus sign is more common, but it's still a 1:2 ratio judging from quick grep, while the stable-kernel-rules.rst actually uses a minus sign...

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Kalle Valo

2017-04-03 13:21 UTC #10 lore raw

Quoted 34 lines · Johan Hovold

Quoted 25 lines · Kalle Valo
Quoted 11 lines · Kalle Valo
Quoted 9 lines · Johan Hovold
Quoted 7 lines · Johan Hovold
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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
Quoted 5 lines
Quoted 1 line
Is this one still in your queue, Kalle?
Yes, I'm just lacking behing:
https://patchwork.kernel.org/patch/9620723/
Meant "lagging" of course. Mondays..
Quoted 4 lines
Quoted 4 lines
As I mentioned earlier, I should have added a
Cc stable <[email protected]> # 2.6.39
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
but left it out as I mistakingly thought the net recommendations to do so applied also to wireless.
Ok, I'll add that.
But is 2.6.39 really correct? Shouldn't it be 2.6.39+ so that it means all versions since 2.6.39?

Quoted 6 lines

Either way is fine, the stable maintainers apply them to all later versions.
I notice now that adding a plus sign is more common, but it's still a 1:2 ratio judging from quick grep, while the stable-kernel-rules.rst actually uses a minus sign...

Heh, quite confusing :) I added the plus sign already to the patch in my pending branch so unless you object I'll keep it.

Signature · 1 line

Kalle Valo

Johan Hovold

author

2017-04-03 13:26 UTC #11 lore raw

Quoted 39 lines · Kalle Valo

Quoted 37 lines · Johan Hovold
Quoted 25 lines · Kalle Valo
Quoted 11 lines · Kalle Valo
Quoted 9 lines · Johan Hovold
Quoted 7 lines · Johan Hovold
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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
Quoted 5 lines
Quoted 1 line
Is this one still in your queue, Kalle?
Yes, I'm just lacking behing:
https://patchwork.kernel.org/patch/9620723/
Meant "lagging" of course. Mondays..
Quoted 4 lines
Quoted 4 lines
As I mentioned earlier, I should have added a
Cc stable <[email protected]> # 2.6.39
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
but left it out as I mistakingly thought the net recommendations to do so applied also to wireless.
Ok, I'll add that.
But is 2.6.39 really correct? Shouldn't it be 2.6.39+ so that it means all versions since 2.6.39?
Either way is fine, the stable maintainers apply them to all later versions.

Quoted 6 lines

Quoted 3 lines
I notice now that adding a plus sign is more common, but it's still a 1:2 ratio judging from quick grep, while the stable-kernel-rules.rst actually uses a minus sign...
Heh, quite confusing :) I added the plus sign already to the patch in my pending branch so unless you object I'll keep it.

Please do, no objection. :)

Thanks,
Johan

Kalle Valo

2017-04-05 07:35 UTC #12 lore raw

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

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 36bcce430657 ("ath9k_htc: Handle storage devices")
Cc Sujith Manoharan <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

ebeb36670eca ath9k_htc: fix NULL-deref at probe

Signature · 6 lines

https://patchwork.kernel.org/patch/9620723/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches