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

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

[Lorenzo Bianconi]

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 <lorenzo@kernel.org>
---
 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

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

[Kalle Valo]

Lorenzo Bianconi <lorenzo@kernel.org> 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 <lorenzo@kernel.org>

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

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

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

> Lorenzo Bianconi <lorenzo@kernel.org> 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 <lorenzo@kernel.org>
> 
> 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/cover.1607164041.git.lorenzo@kernel.org/

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Felix Fietkau]

On 2020-12-17 18:11, Lorenzo Bianconi wrote:
>> Lorenzo Bianconi <lorenzo@kernel.org> 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 <lorenzo@kernel.org>
>> 
>> 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/cover.1607164041.git.lorenzo@kernel.org/
> 
> @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

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

[Kalle Valo]

Felix Fietkau <nbd@nbd.name> writes:

> On 2020-12-17 18:11, Lorenzo Bianconi wrote:
>>> Lorenzo Bianconi <lorenzo@kernel.org> 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 <lorenzo@kernel.org>
>>> 
>>> 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/cover.1607164041.git.lorenzo@kernel.org/
>> 
>> @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

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

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

On Dec 17, Kalle Valo wrote:
> Felix Fietkau <nbd@nbd.name> writes:
> 
> > On 2020-12-17 18:11, Lorenzo Bianconi wrote:
> >>> Lorenzo Bianconi <lorenzo@kernel.org> 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 <lorenzo@kernel.org>
> >>> 
> >>> 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/cover.1607164041.git.lorenzo@kernel.org/
> >> 
> >> @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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Kalle Valo]

Lorenzo Bianconi <lorenzo@kernel.org> 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 <lorenzo@kernel.org>

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