[PATCH] iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

[PATCH] iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

[Jose Ignacio Tornos Martinez]

Extend the applied solution with commit 14a3aacf517a9
("iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue")
to other cases where the station queues are also disabled and the related
lists are not emptied. In the same way, if some new element is added later
to the list in iwl_mvm_mac_wake_tx_queue, it can match with the old one
and produce the same commented BUG.

Again, in order to avoid this problem, we must also remove the related
lists when station queues are disabled.

Fixes: cfbc6c4c5b91c ("iwlwifi: mvm: support mac80211 TXQs model")
Reported-by: Petr Stourac <pstourac@redhat.com>
Tested-by: Petr Stourac <pstourac@redhat.com>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 2661d4e819ce..e28505a1f2c4 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -367,6 +367,7 @@ static int iwl_mvm_disable_txq(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 			iwl_mvm_txq_from_tid(sta, tid);
 
 		mvmtxq->txq_id = IWL_MVM_INVALID_QUEUE;
+		list_del_init(&mvmtxq->list);
 	}
 
 	/* Regardless if this is a reserved TXQ for a STA - mark it as false */
@@ -461,6 +462,7 @@ static int iwl_mvm_remove_sta_queue_marking(struct iwl_mvm *mvm, int queue)
 		mvmsta->tid_data[tid].txq_id = IWL_MVM_INVALID_QUEUE;
 
 		mvmtxq->txq_id = IWL_MVM_INVALID_QUEUE;
+		list_del_init(&mvmtxq->list);
 	}
 
 	mvmsta->tfd_queue_msk &= ~BIT(queue); /* Don't use this queue anymore */
-- 
2.37.2

Re: [PATCH] iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

[Jose Ignacio Tornos Martinez]

Could someone please review this patch?
This is just a second part of a previous patch already acked (14a3aacf517a9)

[PATCH v2] iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

[Jose Ignacio Tornos Martinez]

BUGs like this are still reproducible:

[   31.509616] list_add corruption. prev->next should be next (ffff8f8644242300), but was ffff8f86493fd300. (prev=ffff8f86493fd300).
[   31.521544] ------------[ cut here ]------------
[   31.526248] kernel BUG at lib/list_debug.c:30!
[   31.530781] invalid opcode: 0000 [#1] PREEMPT SMP PTI
[   31.535831] CPU: 1 PID: 626 Comm: wpa_supplicant Not tainted 6.0.0+ #7
[   31.542450] Hardware name: Dell Inc. Inspiron 660s/0478VN       , BIOS A07 08/24/2012
[   31.550484] RIP: 0010:__list_add_valid.cold+0x3a/0x5b
[   31.555537] Code: f2 4c 89 c1 48 89 fe 48 c7 c7 28 20 69 89 e8 4c e3 fd ff 0f 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 d0 1f 69 89 e8 35 e3 fd ff <0f> 0b 4c 89 c1 48 c7 c7 78 1f 69 89 e8 24 e3 fd ff 0f 0b 48 c7 c7
[   31.574605] RSP: 0018:ffff9f6f00dc3748 EFLAGS: 00010286
[   31.579990] RAX: 0000000000000075 RBX: ffff8f8644242080 RCX: 0000000000000000
[   31.587155] RDX: 0000000000000201 RSI: ffffffff8967862d RDI: 00000000ffffffff
[   31.594482] RBP: ffff8f86493fd2e8 R08: 0000000000000000 R09: 00000000ffffdfff
[   31.601735] R10: ffff9f6f00dc3608 R11: ffffffff89f46128 R12: ffff8f86493fd300
[   31.608986] R13: ffff8f86493fd300 R14: ffff8f8644242300 R15: ffff8f8643dd3f2c
[   31.616151] FS:  00007f3bb9a707c0(0000) GS:ffff8f865a300000(0000) knlGS:0000000000000000
[   31.624447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   31.630286] CR2: 00007fe3647d5600 CR3: 00000001125a6002 CR4: 00000000000606e0
[   31.637539] Call Trace:
[   31.639936]  <TASK>
[   31.642143]  iwl_mvm_mac_wake_tx_queue+0x71/0x90 [iwlmvm]
[   31.647569]  ieee80211_queue_skb+0x4b6/0x720 [mac80211]
...

So, it is necessary to extend the applied solution with commit 14a3aacf517a9
("iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue")
to all other cases where the station queues are invalidated and the related
lists are not emptied. Because, otherwise as before, if some new element is 
added later to the list in iwl_mvm_mac_wake_tx_queue, it can match with the 
old one and produce the same commented BUG.

That is, in order to avoid this problem completely, we must also remove the 
related lists for the other cases when station queues are invalidated.

Fixes: cfbc6c4c5b91c ("iwlwifi: mvm: support mac80211 TXQs model")
Reported-by: Petr Stourac <pstourac@redhat.com>
Tested-by: Petr Stourac <pstourac@redhat.com>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
V1 -> V2:
- More detailed explanation of the patch and add a real case.

 drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index cc92706b3d16..cbd8053a9e35 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -384,6 +384,7 @@ static int iwl_mvm_disable_txq(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 			iwl_mvm_txq_from_tid(sta, tid);
 
 		mvmtxq->txq_id = IWL_MVM_INVALID_QUEUE;
+		list_del_init(&mvmtxq->list);
 	}
 
 	/* Regardless if this is a reserved TXQ for a STA - mark it as false */
@@ -478,6 +479,7 @@ static int iwl_mvm_remove_sta_queue_marking(struct iwl_mvm *mvm, int queue)
 		mvmsta->tid_data[tid].txq_id = IWL_MVM_INVALID_QUEUE;
 
 		mvmtxq->txq_id = IWL_MVM_INVALID_QUEUE;
+		list_del_init(&mvmtxq->list);
 	}
 
 	mvmsta->tfd_queue_msk &= ~BIT(queue); /* Don't use this queue anymore */
-- 
2.37.3

Re: [PATCH v2] iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

[Kalle Valo]

Jose Ignacio Tornos Martinez <jtornosm@redhat.com> writes:

> BUGs like this are still reproducible:
>
> [   31.509616] list_add corruption. prev->next should be next (ffff8f8644242300), but was ffff8f86493fd300. (prev=ffff8f86493fd300).
> [   31.521544] ------------[ cut here ]------------
> [   31.526248] kernel BUG at lib/list_debug.c:30!
> [   31.530781] invalid opcode: 0000 [#1] PREEMPT SMP PTI
> [   31.535831] CPU: 1 PID: 626 Comm: wpa_supplicant Not tainted 6.0.0+ #7
> [   31.542450] Hardware name: Dell Inc. Inspiron 660s/0478VN       , BIOS A07 08/24/2012
> [   31.550484] RIP: 0010:__list_add_valid.cold+0x3a/0x5b
> [   31.555537] Code: f2 4c 89 c1 48 89 fe 48 c7 c7 28 20 69 89 e8 4c e3 fd ff 0f 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 d0 1f 69 89 e8 35 e3 fd ff <0f> 0b 4c 89 c1 48 c7 c7 78 1f 69 89 e8 24 e3 fd ff 0f 0b 48 c7 c7
> [   31.574605] RSP: 0018:ffff9f6f00dc3748 EFLAGS: 00010286
> [   31.579990] RAX: 0000000000000075 RBX: ffff8f8644242080 RCX: 0000000000000000
> [   31.587155] RDX: 0000000000000201 RSI: ffffffff8967862d RDI: 00000000ffffffff
> [   31.594482] RBP: ffff8f86493fd2e8 R08: 0000000000000000 R09: 00000000ffffdfff
> [   31.601735] R10: ffff9f6f00dc3608 R11: ffffffff89f46128 R12: ffff8f86493fd300
> [   31.608986] R13: ffff8f86493fd300 R14: ffff8f8644242300 R15: ffff8f8643dd3f2c
> [   31.616151] FS:  00007f3bb9a707c0(0000) GS:ffff8f865a300000(0000) knlGS:0000000000000000
> [   31.624447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   31.630286] CR2: 00007fe3647d5600 CR3: 00000001125a6002 CR4: 00000000000606e0
> [   31.637539] Call Trace:
> [   31.639936]  <TASK>
> [   31.642143]  iwl_mvm_mac_wake_tx_queue+0x71/0x90 [iwlmvm]
> [   31.647569]  ieee80211_queue_skb+0x4b6/0x720 [mac80211]
> ...
>
> So, it is necessary to extend the applied solution with commit 14a3aacf517a9
> ("iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue")
> to all other cases where the station queues are invalidated and the related
> lists are not emptied. Because, otherwise as before, if some new element is 
> added later to the list in iwl_mvm_mac_wake_tx_queue, it can match with the 
> old one and produce the same commented BUG.
>
> That is, in order to avoid this problem completely, we must also remove the 
> related lists for the other cases when station queues are invalidated.
>
> Fixes: cfbc6c4c5b91c ("iwlwifi: mvm: support mac80211 TXQs model")
> Reported-by: Petr Stourac <pstourac@redhat.com>
> Tested-by: Petr Stourac <pstourac@redhat.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

Gregory, can I take this to wireless tree? I already assigned it to me
on patchwork.

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

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

Re: [v2] wifi: iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

[Kalle Valo]

Jose Ignacio Tornos Martinez <jtornosm@redhat.com> wrote:

> BUGs like this are still reproducible:
> 
> [   31.509616] list_add corruption. prev->next should be next (ffff8f8644242300), but was ffff8f86493fd300. (prev=ffff8f86493fd300).
> [   31.521544] ------------[ cut here ]------------
> [   31.526248] kernel BUG at lib/list_debug.c:30!
> [   31.530781] invalid opcode: 0000 [#1] PREEMPT SMP PTI
> [   31.535831] CPU: 1 PID: 626 Comm: wpa_supplicant Not tainted 6.0.0+ #7
> [   31.542450] Hardware name: Dell Inc. Inspiron 660s/0478VN       , BIOS A07 08/24/2012
> [   31.550484] RIP: 0010:__list_add_valid.cold+0x3a/0x5b
> [   31.555537] Code: f2 4c 89 c1 48 89 fe 48 c7 c7 28 20 69 89 e8 4c e3 fd ff 0f 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 d0 1f 69 89 e8 35 e3 fd ff <0f> 0b 4c 89 c1 48 c7 c7 78 1f 69 89 e8 24 e3 fd ff 0f 0b 48 c7 c7
> [   31.574605] RSP: 0018:ffff9f6f00dc3748 EFLAGS: 00010286
> [   31.579990] RAX: 0000000000000075 RBX: ffff8f8644242080 RCX: 0000000000000000
> [   31.587155] RDX: 0000000000000201 RSI: ffffffff8967862d RDI: 00000000ffffffff
> [   31.594482] RBP: ffff8f86493fd2e8 R08: 0000000000000000 R09: 00000000ffffdfff
> [   31.601735] R10: ffff9f6f00dc3608 R11: ffffffff89f46128 R12: ffff8f86493fd300
> [   31.608986] R13: ffff8f86493fd300 R14: ffff8f8644242300 R15: ffff8f8643dd3f2c
> [   31.616151] FS:  00007f3bb9a707c0(0000) GS:ffff8f865a300000(0000) knlGS:0000000000000000
> [   31.624447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   31.630286] CR2: 00007fe3647d5600 CR3: 00000001125a6002 CR4: 00000000000606e0
> [   31.637539] Call Trace:
> [   31.639936]  <TASK>
> [   31.642143]  iwl_mvm_mac_wake_tx_queue+0x71/0x90 [iwlmvm]
> [   31.647569]  ieee80211_queue_skb+0x4b6/0x720 [mac80211]
> ...
> 
> So, it is necessary to extend the applied solution with commit 14a3aacf517a9
> ("iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue")
> to all other cases where the station queues are invalidated and the related
> lists are not emptied. Because, otherwise as before, if some new element is 
> added later to the list in iwl_mvm_mac_wake_tx_queue, it can match with the 
> old one and produce the same commented BUG.
> 
> That is, in order to avoid this problem completely, we must also remove the 
> related lists for the other cases when station queues are invalidated.
> 
> Fixes: cfbc6c4c5b91c ("iwlwifi: mvm: support mac80211 TXQs model")
> Reported-by: Petr Stourac <pstourac@redhat.com>
> Tested-by: Petr Stourac <pstourac@redhat.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

Patch applied to wireless.git, thanks.

95b0f66649bb wifi: iwlwifi: mvm: fix double list_add at iwl_mvm_mac_wake_tx_queue (other cases)

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20221010081611.145027-1-jtornosm@redhat.com/

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