linux-kernel - RE: [PATCH rtw v4 1/4] wifi: rtw89: fix use-after-free in rtw89_core_tx_kick_off_and

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <e4153922734d4c8f8b394b3542b9cf58@realtek.com>
Date: Fri, 19 Sep 2025 00:27:33 +0000
From: Ping-Ke Shih <pkshih@...ltek.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>
CC: Zong-Zhe Yang <kevin_yang@...ltek.com>,
        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 1/4] wifi: rtw89: fix use-after-free in rtw89_core_tx_kick_off_and_wait()

Fedor Pchelkin <pchelkin@...ras.ru> wrote:
> On Thu, 18. Sep 05:23, Ping-Ke Shih wrote:
> > Zong-Zhe Yang <kevin_yang@...ltek.com> wrote:
> > > Ping-Ke Shih <pkshih@...ltek.com> wrote:
> > > >
> > > > Fedor Pchelkin <pchelkin@...ras.ru> wrote:
> > > >
> > > > [...]
> > > >
> > > > > @@ -6181,6 +6187,27 @@ rtw89_assoc_link_rcu_dereference(struct rtw89_dev *rtwdev,
> > > > u8 macid)
> > > > >         list_first_entry_or_null(&p->dlink_pool,
> > > > > typeof(*p->links_inst), dlink_schd); \
> > > > >  })
> > > > >
> > > > > +static inline void rtw89_tx_wait_release(struct rtw89_tx_wait_info
> > > > > +*wait) {
> > > > > +       dev_kfree_skb_any(wait->skb);
> > > > > +       kfree_rcu(wait, rcu_head);
> > > > > +}
> > > > > +
> > > > > +static inline void rtw89_tx_wait_list_clear(struct rtw89_dev *rtwdev)
> > > > > +{
> > > > > +       struct rtw89_tx_wait_info *wait, *tmp;
> > > > > +
> > > > > +       lockdep_assert_wiphy(rtwdev->hw->wiphy);
> > > > > +
> > > > > +       list_for_each_entry_safe(wait, tmp, &rtwdev->tx_waits, list) {
> > > > > +               if (!wait_for_completion_timeout(&wait->completion,
> > > > > +
> > > > RTW89_TX_WAIT_DEFAULT_TIMEOUT))
> > > > > +                       continue;
> > > >
> > > >
> > > > Why should we wait 10ms? Just try_wait_for_completion()?
> > > >
> > > > Since TX completion might be missing (rtw89_core_stop(), for example), shouldn't we
> > > > unconditionally free all in wait list for that case?
> > > >
> > >
> > > In hci reset (when we release pending skb), the condition will become true.
> > > So, all left will be freed at that time. Before that, there is no logic to ensure no
> > > more completing side, so it cannot be unconditionally freed unless we don't
> > > want to double check if those, which timed out, are done at some moment.
> > >
> > > (e.g. core stop will do hci reset)
> >
> > Thanks for the explanation.
> >
> > Just consider try_wait_for_completion() then.
> 
> OK.  completion_done() looks appropriate here as well.
> 
> >
> > By the way, if want a delay for timeout case, use delayed work for tx_wait_work
> > instead.
> 
> That makes sense, thanks.  So the next time I'll go with delayed
> tx_wait_work performing completion_done(): work delay 500 ms, looks
> neither too small nor too big for freeing potentially timed out items.

Both look reasonable to me.