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

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

[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 <Sujith.Manoharan@atheros.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index de2d212f39ec..9206955e865a 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -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;
-- 
2.12.0

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

[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 <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(+)

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;
-- 
2.12.0

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

[Kalle Valo]

Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

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

ca260ece6a57 zd1211rw: fix NULL-deref at probe

-- 
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

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

[Johan Hovold]

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

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

[Kalle Valo]

Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Wed, Mar 22, 2017 at 09:04:15AM +0000, Kalle Valo wrote:
>> Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
>> > Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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.

-- 
Kalle Valo

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

[Johan Hovold]

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

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

[Johan Hovold]

On Mon, Mar 13, 2017 at 01:44:20PM +0100, Johan Hovold 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: 36bcce430657 ("ath9k_htc: Handle storage devices")
> Cc: Sujith Manoharan <Sujith.Manoharan@atheros.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Is this one still in your queue, Kalle?

As I mentioned earlier, I should have added a

Cc: stable <stable@vger.kernel.org>     # 2.6.39

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

Thanks,
Johan

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

[Kalle Valo]

Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Mon, Mar 13, 2017 at 01:44:20PM +0100, Johan Hovold 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: 36bcce430657 ("ath9k_htc: Handle storage devices")
>> Cc: Sujith Manoharan <Sujith.Manoharan-DlyHzToyqoxBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Is this one still in your queue, Kalle?

Yes, I'm just lacking behing:

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

> As I mentioned earlier, I should have added a
>
> Cc: stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>     # 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.

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

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

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

> Johan Hovold <johan@kernel.org> writes:
>
>> On Mon, Mar 13, 2017 at 01:44:20PM +0100, Johan Hovold 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: 36bcce430657 ("ath9k_htc: Handle storage devices")
>>> Cc: Sujith Manoharan <Sujith.Manoharan@atheros.com>
>>> Signed-off-by: Johan Hovold <johan@kernel.org>
>>
>> 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..

>> As I mentioned earlier, I should have added a
>>
>> Cc: stable <stable@vger.kernel.org>     # 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?

-- 
Kalle Valo

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

[Johan Hovold]

On Mon, Apr 03, 2017 at 01:02:28PM +0000, Kalle Valo wrote:
> Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> writes:
> 
> > Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> >
> >> On Mon, Mar 13, 2017 at 01:44:20PM +0100, Johan Hovold 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: 36bcce430657 ("ath9k_htc: Handle storage devices")
> >>> Cc: Sujith Manoharan <Sujith.Manoharan-DlyHzToyqoxBDgjK7y7TUQ@public.gmane.org>
> >>> Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>
> >> 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..
> 
> >> As I mentioned earlier, I should have added a
> >>
> >> Cc: stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>     # 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?

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Kalle Valo]

Johan Hovold <johan@kernel.org> writes:

> On Mon, Apr 03, 2017 at 01:02:28PM +0000, Kalle Valo wrote:
>> Kalle Valo <kvalo@codeaurora.org> writes:
>> 
>> > Johan Hovold <johan@kernel.org> writes:
>> >
>> >> On Mon, Mar 13, 2017 at 01:44:20PM +0100, Johan Hovold 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: 36bcce430657 ("ath9k_htc: Handle storage devices")
>> >>> Cc: Sujith Manoharan <Sujith.Manoharan@atheros.com>
>> >>> Signed-off-by: Johan Hovold <johan@kernel.org>
>> >>
>> >> 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..
>> 
>> >> As I mentioned earlier, I should have added a
>> >>
>> >> Cc: stable <stable@vger.kernel.org>     # 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?
>
> 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.

-- 
Kalle Valo

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

[Johan Hovold]

On Mon, Apr 03, 2017 at 01:21:08PM +0000, Kalle Valo wrote:
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Mon, Apr 03, 2017 at 01:02:28PM +0000, Kalle Valo wrote:
> >> Kalle Valo <kvalo@codeaurora.org> writes:
> >> 
> >> > Johan Hovold <johan@kernel.org> writes:
> >> >
> >> >> On Mon, Mar 13, 2017 at 01:44:20PM +0100, Johan Hovold 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: 36bcce430657 ("ath9k_htc: Handle storage devices")
> >> >>> Cc: Sujith Manoharan <Sujith.Manoharan@atheros.com>
> >> >>> Signed-off-by: Johan Hovold <johan@kernel.org>
> >> >>
> >> >> 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..
> >> 
> >> >> As I mentioned earlier, I should have added a
> >> >>
> >> >> Cc: stable <stable@vger.kernel.org>     # 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?
> >
> > 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.

Please do, no objection. :)

Thanks,
Johan

Re: [1/2] wireless: ath9k_htc: fix NULL-deref at probe

[Kalle Valo]

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: 36bcce430657 ("ath9k_htc: Handle storage devices")
> Cc: Sujith Manoharan <Sujith.Manoharan@atheros.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>

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

ebeb36670eca ath9k_htc: fix NULL-deref at probe

-- 
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