linux-kernel - RE: [PATCH rtw v4 2/4] wifi: rtw89: fix tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <08b25263c6874a089e4a271cb95a9cb7@realtek.com>
Date: Thu, 18 Sep 2025 05:47:37 +0000
From: Ping-Ke Shih <pkshih@...ltek.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>,
        Zong-Zhe Yang
	<kevin_yang@...ltek.com>
CC: Bitterblue Smith <rtl8821cerfe2@...il.com>,
        Bernie Huang
	<phhuang@...ltek.com>,
        "linux-wireless@...r.kernel.org"
	<linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "lvc-project@...uxtesting.org"
	<lvc-project@...uxtesting.org>,
        "stable@...r.kernel.org"
	<stable@...r.kernel.org>
Subject: RE: [PATCH rtw v4 2/4] wifi: rtw89: fix tx_wait initialization race

Fedor Pchelkin <pchelkin@...ras.ru> wrote:
> Now that nullfunc skbs are recycled in a separate work item in the driver,
> the following race during initialization and processing of those skbs
> might lead to noticeable bugs:
> 
>       Waiting thread                          Completing thread
> 
> rtw89_core_send_nullfunc()
>   rtw89_core_tx_write_link()
>     ...
>     rtw89_pci_txwd_submit()
>       skb_data->wait = NULL
>       /* add skb to the queue */
>       skb_queue_tail(&txwd->queue, skb)
>                                             rtw89_pci_napi_poll()
>                                             ...
>                                               rtw89_pci_release_txwd_skb()
>                                                 /* get skb from the queue */
>                                                 skb_unlink(skb, &txwd->queue)
>                                                 rtw89_pci_tx_status()
>                                                   rtw89_core_tx_wait_complete()
>                                                   /* use incorrect skb_data->wait */
>   rtw89_core_tx_kick_off_and_wait()
>   /* assign skb_data->wait but too late */

How will we receive tx completion before TX kick off? 
(see the original code below)

[...]

> diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> index 438930b65631..1efe4bb09262 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.c
> +++ b/drivers/net/wireless/realtek/rtw89/core.c
> @@ -1094,22 +1094,13 @@ int rtw89_core_tx_kick_off_and_wait(struct rtw89_dev *rtwdev, struct sk_buff *sk
>                                     int qsel, unsigned int timeout)
>  {
>         struct rtw89_tx_skb_data *skb_data = RTW89_TX_SKB_CB(skb);
> -       struct rtw89_tx_wait_info *wait;
> +       struct rtw89_tx_wait_info *wait = wiphy_dereference(rtwdev->hw->wiphy,
> +                                                           skb_data->wait);

Can't we just pass 'wait' by function argument?

>         unsigned long time_left;
>         int ret = 0;
> 
>         lockdep_assert_wiphy(rtwdev->hw->wiphy);
> 
> -       wait = kzalloc(sizeof(*wait), GFP_KERNEL);
> -       if (!wait) {
> -               rtw89_core_tx_kick_off(rtwdev, qsel);
> -               return 0;
> -       }
> -
> -       init_completion(&wait->completion);
> -       wait->skb = skb;
> -       rcu_assign_pointer(skb_data->wait, wait);
> -

Here, original code prepares completion before TX kick off. How it could
be a problem? Do I miss something?

>         rtw89_core_tx_kick_off(rtwdev, qsel);
>         time_left = wait_for_completion_timeout(&wait->completion,
>                                                 msecs_to_jiffies(timeout));