diff mbox

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

Message ID 20170313124421.28587-1-johan@kernel.org
State Awaiting Upstream, archived
Delegated to: David Miller
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: 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(+)

Comments

Johan Hovold April 3, 2017, 8:42 a.m. UTC | #1
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
Kalle Valo April 3, 2017, 9:34 a.m. UTC | #2
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/

> 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.
Kalle Valo April 3, 2017, 1:02 p.m. UTC | #3
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?
Johan Hovold April 3, 2017, 1:16 p.m. UTC | #4
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...

Thanks,
Johan
Kalle Valo April 3, 2017, 1:21 p.m. UTC | #5
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.
Johan Hovold April 3, 2017, 1:26 p.m. UTC | #6
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
Kalle Valo April 5, 2017, 7:35 a.m. UTC | #7
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
diff mbox

Patch

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;