diff mbox series

mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue

Message ID b49c1b4edacd87b2241a9fd0431dd4864c8963f6.1607418933.git.lorenzo@kernel.org
State New
Headers show
Series mt76: mt76s: fix NULL pointer dereference in mt76s_process_tx_queue | expand

Commit Message

Lorenzo Bianconi Dec. 8, 2020, 9:18 a.m. UTC
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(-)

Comments

Kalle Valo Dec. 17, 2020, 4:36 p.m. UTC | #1
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
Lorenzo Bianconi Dec. 17, 2020, 5:11 p.m. UTC | #2
> 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

>
Felix Fietkau Dec. 17, 2020, 5:20 p.m. UTC | #3
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
Kalle Valo Dec. 17, 2020, 5:44 p.m. UTC | #4
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
Lorenzo Bianconi Dec. 17, 2020, 5:59 p.m. UTC | #5
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
Kalle Valo Dec. 20, 2020, 12:05 p.m. UTC | #6
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
diff mbox series

Patch

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;