linux-kernel - Re: [PATCH v3 0/8] rtw88: prepare locking for SDIO support

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]

Date:   Mon, 31 Jan 2022 03:06:12 +0000
From:   Pkshih <pkshih@...ltek.com>
To:     "martin.blumenstingl@...glemail.com" 
        <martin.blumenstingl@...glemail.com>
CC:     "johannes@...solutions.net" <johannes@...solutions.net>,
        "kvalo@...eaurora.org" <kvalo@...eaurora.org>,
        "neojou@...il.com" <neojou@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "tony0620emma@...il.com" <tony0620emma@...il.com>,
        "jernej.skrabec@...il.com" <jernej.skrabec@...il.com>,
        "eswierk@...st" <eswierk@...st>
Subject: Re: [PATCH v3 0/8] rtw88: prepare locking for SDIO support

Hi,

On Sun, 2022-01-30 at 22:40 +0100, Martin Blumenstingl wrote:
> 
> On Fri, Jan 28, 2022 at 1:51 AM Pkshih <pkshih@...ltek.com> wrote:
> [...]
> > > > To avoid this, we can add a flag to struct rtw_vif, and set this flag
> > > > when ::remove_interface. Then, only collect vif without this flag into list
> > > > when we use iterate_actiom().
> > > > 
> > > > As well as ieee80211_sta can do similar fix.
> > > > 
> > 
> > I would prefer my method that adds a 'bool disabled' flag to struct rtw_vif/rtw_sta
> > and set it when ::remove_interface/::sta_remove. Then rtw_iterate_stas() can
> > check this flag to decide whether does thing or not.
> That would indeed be a very straight forward approach and easy to read.
> In net/mac80211/iface.c there's some cases where after
> drv_remove_interface() (which internally calls our .remove_interface
> op) will kfree the vif (sdata). Doesn't that then result in a
> use-after-free if we rely on a boolean within rtw_vif?

The rtw_vif is drv_priv of ieee80211_vif, and they will be freed at
the same time. We must set 'bool disabled' after holding rtwdev->mutex
lock, and check this flag in iterator of ieee80211_iterate_active_interfaces_atomic()
to contruct a list of vif.

That means we never access this flag out of rtwdev->mutx or iterator.
Does it make sense?

--
Ping-Ke