linux-kernel - Re: [PATCH 04/13] wifi: mt76: mt7921: add mutex protection in critical paths

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: <CAOFcj8S4vBGpgwamW_b9mGaHhL78O44_CdM++qqGJc89nEancA@mail.gmail.com>
Date: Wed, 28 Jan 2026 22:19:49 -0800
From: Zac Bowling <zac@...bowling.com>
To: Felix Fietkau <nbd@....name>
Cc: sean.wang@...nel.org, deren.wu@...iatek.com, kvalo@...nel.org, 
	linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org, 
	linux-wireless@...r.kernel.org, linux@...me.work, lorenzo@...nel.org, 
	ryder.lee@...iatek.com, sean.wang@...iatek.com
Subject: Re: [PATCH 04/13] wifi: mt76: mt7921: add mutex protection in
 critical paths

You are right. I caught that too. After reordering, when I went from
the 24-something patch version of this series earlier this month to
this smaller 11-patch series, to make it easier to follow again, that
happened. It's already gone in my new v7 series. We lock somewhere
else up in the stack now.

I'm cleaning up this whole stack again, dropping the ROC_ABORT back
off hack, because I think actually the solution isn't at this layer at
all, but possibly in the mac80211 layer.

Zac Bowling

On Tue, Jan 27, 2026 at 2:59 AM Felix Fietkau <nbd@....name> wrote:
>
> On 20.01.26 21:10, Zac wrote:
> > From: Zac Bowling <zac@...bowling.com>
> >
> > Add proper mutex protection for mt7921 driver operations that access
> > hardware state without proper synchronization. This fixes multiple race
> > conditions that can cause system instability.
> >
> > Fixes added:
> >
> > 1. mac.c: mt7921_mac_reset_work()
> >     - Wrap ieee80211_iterate_active_interfaces() with mt792x_mutex
> >     - The vif_connect_iter callback accesses hw_encap state
> >
> > 2. main.c: mt7921_remain_on_channel()
> >     - Remove mt792x_mutex_acquire/release around mt7925_set_channel_state()
> >     - The function is already called with mutex held from mac80211
> >     - This was causing double-lock deadlock
> >
> > 3. main.c: mt7921_cancel_remain_on_channel()
> >     - Remove mt792x_mutex_acquire/release
> >     - Function is called from mac80211 with mutex already held
> >
> > 4. pci.c: mt7921_pci_pm_complete()
> >     - Remove mt792x_mutex_acquire/release around ieee80211_iterate_active_interfaces
> >     - This was causing deadlock as the vif connect iteration tries
> >       to acquire the mutex again
> >
> > 5. usb.c: mt7921_usb_pm_complete()
> >     - Same fix as pci.c for USB driver path
> Changelog should be below "---" after the commit description, so it
> doesn't get picked up.
>
> > These changes prevent both missing mutex protection and mutex deadlocks
> > in the mt7921 driver.
> >
> > Fixes: 5c14a5f944b9 ("wifi: mt76: mt7921: introduce remain_on_channel support")
> > Signed-off-by: Zac Bowling <zac@...bowling.com>
>
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > index 5fae9a6e273c..196fcb1e2e94 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > @@ -373,6 +373,11 @@ void mt7921_roc_abort_sync(struct mt792x_dev *dev)
> >
> >       timer_delete_sync(&phy->roc_timer);
> >       cancel_work_sync(&phy->roc_work);
> > +     /* Note: caller must hold mutex if ieee80211_iterate_interfaces is
> > +      * needed for ROC cleanup. Some call sites (like mt7921_mac_sta_remove)
> > +      * already hold the mutex via mt76_sta_remove(). For suspend paths,
> > +      * the mutex should be acquired before calling this function.
> > +      */
> >       if (test_and_clear_bit(MT76_STATE_ROC, &phy->mt76->state))
> >               ieee80211_iterate_interfaces(mt76_hw(dev),
> >                                            IEEE80211_IFACE_ITER_RESUME_ALL,
> > @@ -619,6 +624,7 @@ void mt7921_set_runtime_pm(struct mt792x_dev *dev)
> >       bool monitor = !!(hw->conf.flags & IEEE80211_CONF_MONITOR);
> >
> >       pm->enable = pm->enable_user && !monitor;
> > +     /* Note: caller (debugfs) must hold mutex before calling this function */
> >       ieee80211_iterate_active_interfaces(hw,
> >                                           IEEE80211_IFACE_ITER_RESUME_ALL,
> >                                           mt7921_pm_interface_iter, dev);
> > @@ -765,6 +771,9 @@ mt7921_regd_set_6ghz_power_type(struct ieee80211_vif *vif, bool is_add)
> >       struct mt792x_dev *dev = phy->dev;
> >       u32 valid_vif_num = 0;
> >
> > +     /* Note: caller (mt7921_mac_sta_add/remove via mt76_sta_add/remove)
> > +      * already holds dev->mt76.mutex, so we must not acquire it here.
> > +      */
> >       ieee80211_iterate_active_interfaces(mt76_hw(dev),
> >                                           IEEE80211_IFACE_ITER_RESUME_ALL,
> >                                           mt7921_calc_vif_num, &valid_vif_num);
>
> It looks like these comments should be replaced with
> lockdep_assert_held, so that these assumptions can be verified
> automatically instead of doing so by hand.
>
>
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> > index ec9686183251..9f76b334b93d 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> > @@ -426,7 +426,9 @@ static int mt7921_pci_suspend(struct device *device)
> >       cancel_delayed_work_sync(&pm->ps_work);
> >       cancel_work_sync(&pm->wake_work);
> >
> > +     mt792x_mutex_acquire(dev);
> >       mt7921_roc_abort_sync(dev);
> > +     mt792x_mutex_release(dev);
> The next patch is removing those...
>
> - Felix