Re: Patch "wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()" has been added to the 6.14-stable tree

[Remi Pommarel <repk@triplefau.lt>]

Hello,

On Sun, Apr 20, 2025 at 11:00:53AM -0400, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
> 
>     wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
> 
> to the 6.14-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      wifi-mac80211-update-skb-s-control-block-key-in-ieee.patch
> and it can be found in the queue-6.14 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
> 

Not sure this patch should go to stable. @Johannes haven't you revert
it in your tree ?

Thanks,

-- 
Remi

> 
> 
> commit 9209089b629b7c29ae393cded89e77c169f18dfb
> Author: Remi Pommarel <repk@triplefau.lt>
> Date:   Mon Mar 24 17:28:20 2025 +0100
> 
>     wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
>     
>     [ Upstream commit a104042e2bf6528199adb6ca901efe7b60c2c27f ]
>     
>     The ieee80211 skb control block key (set when skb was queued) could have
>     been removed before ieee80211_tx_dequeue() call. ieee80211_tx_dequeue()
>     already called ieee80211_tx_h_select_key() to get the current key, but
>     the latter do not update the key in skb control block in case it is
>     NULL. Because some drivers actually use this key in their TX callbacks
>     (e.g. ath1{1,2}k_mac_op_tx()) this could lead to the use after free
>     below:
>     
>       BUG: KASAN: slab-use-after-free in ath11k_mac_op_tx+0x590/0x61c
>       Read of size 4 at addr ffffff803083c248 by task kworker/u16:4/1440
>     
>       CPU: 3 UID: 0 PID: 1440 Comm: kworker/u16:4 Not tainted 6.13.0-ge128f627f404 #2
>       Hardware name: HW (DT)
>       Workqueue: bat_events batadv_send_outstanding_bcast_packet
>       Call trace:
>        show_stack+0x14/0x1c (C)
>        dump_stack_lvl+0x58/0x74
>        print_report+0x164/0x4c0
>        kasan_report+0xac/0xe8
>        __asan_report_load4_noabort+0x1c/0x24
>        ath11k_mac_op_tx+0x590/0x61c
>        ieee80211_handle_wake_tx_queue+0x12c/0x1c8
>        ieee80211_queue_skb+0xdcc/0x1b4c
>        ieee80211_tx+0x1ec/0x2bc
>        ieee80211_xmit+0x224/0x324
>        __ieee80211_subif_start_xmit+0x85c/0xcf8
>        ieee80211_subif_start_xmit+0xc0/0xec4
>        dev_hard_start_xmit+0xf4/0x28c
>        __dev_queue_xmit+0x6ac/0x318c
>        batadv_send_skb_packet+0x38c/0x4b0
>        batadv_send_outstanding_bcast_packet+0x110/0x328
>        process_one_work+0x578/0xc10
>        worker_thread+0x4bc/0xc7c
>        kthread+0x2f8/0x380
>        ret_from_fork+0x10/0x20
>     
>       Allocated by task 1906:
>        kasan_save_stack+0x28/0x4c
>        kasan_save_track+0x1c/0x40
>        kasan_save_alloc_info+0x3c/0x4c
>        __kasan_kmalloc+0xac/0xb0
>        __kmalloc_noprof+0x1b4/0x380
>        ieee80211_key_alloc+0x3c/0xb64
>        ieee80211_add_key+0x1b4/0x71c
>        nl80211_new_key+0x2b4/0x5d8
>        genl_family_rcv_msg_doit+0x198/0x240
>       <...>
>     
>       Freed by task 1494:
>        kasan_save_stack+0x28/0x4c
>        kasan_save_track+0x1c/0x40
>        kasan_save_free_info+0x48/0x94
>        __kasan_slab_free+0x48/0x60
>        kfree+0xc8/0x31c
>        kfree_sensitive+0x70/0x80
>        ieee80211_key_free_common+0x10c/0x174
>        ieee80211_free_keys+0x188/0x46c
>        ieee80211_stop_mesh+0x70/0x2cc
>        ieee80211_leave_mesh+0x1c/0x60
>        cfg80211_leave_mesh+0xe0/0x280
>        cfg80211_leave+0x1e0/0x244
>       <...>
>     
>     Reset SKB control block key before calling ieee80211_tx_h_select_key()
>     to avoid that.
>     
>     Fixes: bb42f2d13ffc ("mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue")
>     Signed-off-by: Remi Pommarel <repk@triplefau.lt>
>     Link: https://patch.msgid.link/06aa507b853ca385ceded81c18b0a6dd0f081bc8.1742833382.git.repk@triplefau.lt
>     Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>     Signed-off-by: Sasha Levin <sashal@kernel.org>
> 
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index a24636bda6793..0c6214f12ea39 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -3893,6 +3893,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
>  	 * The key can be removed while the packet was queued, so need to call
>  	 * this here to get the current key.
>  	 */
> +	info->control.hw_key = NULL;
>  	r = ieee80211_tx_h_select_key(&tx);
>  	if (r != TX_CONTINUE) {
>  		ieee80211_free_txskb(&local->hw, skb);