2020-12-08 09:22:56

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
can occur if status thread runs before allocating tx queues

Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/mediatek/mt76/sdio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/sdio.c b/drivers/net/wireless/mediatek/mt76/sdio.c
index 7cd995118257..0b6facb17ff7 100644
--- a/drivers/net/wireless/mediatek/mt76/sdio.c
+++ b/drivers/net/wireless/mediatek/mt76/sdio.c
@@ -157,10 +157,14 @@ static void mt76s_net_worker(struct mt76_worker *w)

static int mt76s_process_tx_queue(struct mt76_dev *dev, struct mt76_queue *q)
{
- bool mcu = q == dev->q_mcu[MT_MCUQ_WM];
struct mt76_queue_entry entry;
int nframes = 0;
+ bool mcu;

+ if (!q)
+ return 0;
+
+ mcu = q == dev->q_mcu[MT_MCUQ_WM];
while (q->queued > 0) {
if (!q->entry[q->tail].done)
break;
--
2.28.0


2020-12-17 16:38:49

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

Lorenzo Bianconi <[email protected]> wrote:

> Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
> can occur if status thread runs before allocating tx queues
>
> Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
> Signed-off-by: Lorenzo Bianconi <[email protected]>

Failed to apply to wireless-drivers:

fatal: sha1 information is lacking or useless (drivers/net/wireless/mediatek/mt76/sdio.c).
error: could not build fake ancestor
Applying: mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue
Patch failed at 0001 mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

--
https://patchwork.kernel.org/project/linux-wireless/patch/b49c1b4edacd87b2241a9fd0431dd4864c8963f6.1607418933.git.lorenzo@kernel.org/

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

2020-12-17 17:15:24

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

> Lorenzo Bianconi <[email protected]> wrote:
>
> > Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
> > can occur if status thread runs before allocating tx queues
> >
> > Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
> > Signed-off-by: Lorenzo Bianconi <[email protected]>
>
> Failed to apply to wireless-drivers:

Hi Kalle,

sorry for the noise. I guess to apply this patch we need to apply even the
following series:
https://patchwork.kernel.org/project/linux-wireless/cover/[email protected]/

@Felix: do you think it is ok to apply "remove wake queue tx logic for
usb/sdio" series to wireless-drivers?

If not I can rebase this path ontop of current wireless-drivers tree.

Regards,
Lorenzo

>
> fatal: sha1 information is lacking or useless (drivers/net/wireless/mediatek/mt76/sdio.c).
> error: could not build fake ancestor
> Applying: mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue
> Patch failed at 0001 mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue
> The copy of the patch that failed is found in: .git/rebase-apply/patch
>
> Patch set to Changes Requested.
>
> --
> https://patchwork.kernel.org/project/linux-wireless/patch/b49c1b4edacd87b2241a9fd0431dd4864c8963f6.1607418933.git.lorenzo@kernel.org/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>


Attachments:
(No filename) (1.43 kB)
signature.asc (235.00 B)
Download all attachments

2020-12-17 17:24:24

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

On 2020-12-17 18:11, Lorenzo Bianconi wrote:
>> Lorenzo Bianconi <[email protected]> wrote:
>>
>> > Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
>> > can occur if status thread runs before allocating tx queues
>> >
>> > Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
>> > Signed-off-by: Lorenzo Bianconi <[email protected]>
>>
>> Failed to apply to wireless-drivers:
>
> Hi Kalle,
>
> sorry for the noise. I guess to apply this patch we need to apply even the
> following series:
> https://patchwork.kernel.org/project/linux-wireless/cover/[email protected]/
>
> @Felix: do you think it is ok to apply "remove wake queue tx logic for
> usb/sdio" series to wireless-drivers?
Yes, that makes sense.

- Felix

2020-12-17 17:46:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

Felix Fietkau <[email protected]> writes:

> On 2020-12-17 18:11, Lorenzo Bianconi wrote:
>>> Lorenzo Bianconi <[email protected]> wrote:
>>>
>>> > Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
>>> > can occur if status thread runs before allocating tx queues
>>> >
>>> > Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
>>> > Signed-off-by: Lorenzo Bianconi <[email protected]>
>>>
>>> Failed to apply to wireless-drivers:
>>
>> Hi Kalle,
>>
>> sorry for the noise. I guess to apply this patch we need to apply even the
>> following series:
>> https://patchwork.kernel.org/project/linux-wireless/cover/[email protected]/
>>
>> @Felix: do you think it is ok to apply "remove wake queue tx logic for
>> usb/sdio" series to wireless-drivers?
>
> Yes, that makes sense.

Ok, I assigned the series to me and changed this back to New state.

The commit logs in series don't really answer to "why?", though.
Lorenzo, can you reply to those patches and give more info how they
help? Or are they just cleanup?

--
https://patchwork.kernel.org/project/linux-wireless/list/

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

2020-12-17 18:04:03

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

On Dec 17, Kalle Valo wrote:
> Felix Fietkau <[email protected]> writes:
>
> > On 2020-12-17 18:11, Lorenzo Bianconi wrote:
> >>> Lorenzo Bianconi <[email protected]> wrote:
> >>>
> >>> > Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
> >>> > can occur if status thread runs before allocating tx queues
> >>> >
> >>> > Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
> >>> > Signed-off-by: Lorenzo Bianconi <[email protected]>
> >>>
> >>> Failed to apply to wireless-drivers:
> >>
> >> Hi Kalle,
> >>
> >> sorry for the noise. I guess to apply this patch we need to apply even the
> >> following series:
> >> https://patchwork.kernel.org/project/linux-wireless/cover/[email protected]/
> >>
> >> @Felix: do you think it is ok to apply "remove wake queue tx logic for
> >> usb/sdio" series to wireless-drivers?
> >
> > Yes, that makes sense.
>
> Ok, I assigned the series to me and changed this back to New state.
>
> The commit logs in series don't really answer to "why?", though.
> Lorenzo, can you reply to those patches and give more info how they
> help? Or are they just cleanup?

It is mostly a cleanup since after commit
90d494c99a99fa2eb858754345c4a9c851b409a0 ("mt76: improve tx queue stop/wake"),
we do not need the wake logic anymore in the status path since the queues
are no longer stopped in the tx path.

Regards,
Lorenzo

>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


Attachments:
(No filename) (1.58 kB)
signature.asc (235.00 B)
Download all attachments

2020-12-20 12:09:49

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

Lorenzo Bianconi <[email protected]> wrote:

> Fix a possible NULL pointer dereference in mt76s_process_tx_queue that
> can occur if status thread runs before allocating tx queues
>
> Fixes: 6a618acb7e62 ("mt76: sdio: convert {status/net}_work to mt76_worker")
> Signed-off-by: Lorenzo Bianconi <[email protected]>

Patch applied to wireless-drivers.git, thanks.

f7217f718747 mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

--
https://patchwork.kernel.org/project/linux-wireless/patch/b49c1b4edacd87b2241a9fd0431dd4864c8963f6.1607418933.git.lorenzo@kernel.org/

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