All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiajia Liu <liujiajia@kylinos.cn>
To: Sean Wang <sean.wang@kernel.org>
Cc: Felix Fietkau <nbd@nbd.name>,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	Ryder Lee <ryder.lee@mediatek.com>,
	Shayne Chen <shayne.chen@mediatek.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Ming Yen Hsieh <mingyen.hsieh@mediatek.com>,
	Michael Lo <michael.lo@mediatek.com>,
	Leon Yen <leon.yen@mediatek.com>,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] wifi: mt76: mt7925: add wcid publish check in mt76_sta_add
Date: Wed, 27 May 2026 18:00:17 +0800	[thread overview]
Message-ID: <ahbAsYwFCjsvpxEC@nature> (raw)
In-Reply-To: <CAGp9LzqzddmyDHMNsaqigYpVEdo_Pmzwbeh5Ri5_Gr87cVL6Dg@mail.gmail.com>

On Tue, May 26, 2026 at 04:52:32PM -0500, Sean Wang wrote:
> Hi,
> 
> On Tue, May 26, 2026 at 1:09 AM Jiajia Liu <liujiajia@kylinos.cn> wrote:
> >
> > Since mt7925_mac_sta_add publishes wcid, add publish check in mt76_sta_add
> > to avoid reinitializing the wcid->poll_list for mt7925.
> >
> > Found dev->sta_poll_list corruption when using mt7925 and 7.0-rc4.
> > According to the corruption information, prev->next was changed to itself.
> >
> > wlan0: disconnect from AP 90:fb:5d:94:8b:e3 for new auth to 90:fb:5d:94:8b:e2
> > wlan0: authenticate with 90:fb:5d:94:8b:e2 (local address=84:9e:56:9c:7e:6b)
> > wlan0: send auth to 90:fb:5d:94:8b:e2 (try 1/3)
> >  slab kmalloc-8k start ffff8c80958a6000 pointer offset 4160 size 8192
> > list_add corruption. prev->next should be next (ffff8c808a7488f8), but was ffff8c80958a7040. (prev=ffff8c80958a7040).
> >
> >  mt76_wcid_add_poll+0x95/0xd0 [mt76]
> >  mt7925_mac_add_txs.part.0+0xa5/0xe0 [mt7925_common]
> >  mt7925_rx_check+0xa7/0xc0 [mt7925_common]
> >  mt76_dma_rx_poll+0x50d/0x790 [mt76]
> >  mt792x_poll_rx+0x52/0xe0 [mt792x_lib]
> >
> > Signed-off-by: Jiajia Liu <liujiajia@kylinos.cn>
> > ---
> >
> > Reproduced and tested using the script below over ssh. Roam between two
> > bssids with the same SSID on a router.
> >
> > #!/bin/bash
> >
> > set -ex
> >
> > while :; do
> >         num=$(sudo iw wlan0 scan | grep Polaris | wc -l)
> >         if [ $num -eq 2 ]; then
> >                 break
> >         fi
> > done
> >
> > for i in $(seq 1 500); do
> >
> > echo "index $i"
> > wpa_cli -i wlan0 roam 90:fb:5d:94:8b:e3
> > sleep 5
> > wpa_cli -i wlan0 roam 90:fb:5d:94:8b:e2
> > sleep 5
> >
> > done
> >
> > ---
> >  drivers/net/wireless/mediatek/mt76/mac80211.c    | 11 ++++++++---
> >  drivers/net/wireless/mediatek/mt76/mt76.h        |  1 +
> >  drivers/net/wireless/mediatek/mt76/mt7925/main.c |  3 +++
> >  3 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
> > index 4ae5e4715a9c..83f4f941b890 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
> > @@ -1595,11 +1595,16 @@ mt76_sta_add(struct mt76_phy *phy, struct ieee80211_vif *vif,
> >                 mtxq->wcid = wcid->idx;
> >         }
> >
> > -       ewma_signal_init(&wcid->rssi);
> > -       rcu_assign_pointer(dev->wcid[wcid->idx], wcid);
> > +       if (!test_bit(MT_WCID_FLAG_DRV_PUBLISH, &wcid->flags)) {
> > +               ewma_signal_init(&wcid->rssi);
> > +               rcu_assign_pointer(dev->wcid[wcid->idx], wcid);
> > +               mt76_wcid_init(wcid, phy->band_idx);
> > +       } else {
> > +               wcid->phy_idx = phy->band_idx;
> > +       }
> > +
> >         phy->num_sta++;
> >
> 
> Thanks for spotting the roaming issue.
> 
> I think we can avoid adding MT_WCID_FLAG_DRV_PUBLISH and instead use the
> WCID table itself for the publish check.
> 
> dev->wcid[] already encodes whether a WCID has been published, so checking
> it directly avoids adding a second mirror state. MT_WCID_FLAG_* is also
> better kept for WCID features that affect WTBL setup or data-path handling,
> rather than common bookkeeping state.
> 
> Something like:
> 
> @@ -1620,6 +1620,7 @@ mt76_sta_add(struct mt76_phy *phy, struct
> ieee80211_vif *vif,
> {
>   struct mt76_wcid *wcid = (struct mt76_wcid *)sta->drv_priv;
>   struct mt76_dev *dev = phy->dev;
> +       struct mt76_wcid *published;
>   int ret;
>   int i;
> 
> @@ -1639,7 +1640,10 @@ mt76_sta_add(struct mt76_phy *phy, struct
> ieee80211_vif *vif,
>           mtxq->wcid = wcid->idx;
>   }
> 
> -       if (!test_bit(MT_WCID_FLAG_DRV_PUBLISH, &wcid->flags)) {
> +       published = rcu_dereference_protected(dev->wcid[wcid->idx],
> +                                             lockdep_is_held(&dev->mutex));
> +       if (published != wcid) {
> +               WARN_ON_ONCE(published);
>                  ewma_signal_init(&wcid->rssi);
>                  rcu_assign_pointer(dev->wcid[wcid->idx], wcid);
>                  mt76_wcid_init(wcid, phy->band_idx);
> 
>    ....
> 

Thanks for the suggestion. Will update in v2.

> 
> > -       mt76_wcid_init(wcid, phy->band_idx);
> >  out:
> >         mutex_unlock(&dev->mutex);
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> > index 527bef97e122..8bfce686bff7 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > @@ -361,6 +361,7 @@ enum mt76_wcid_flags {
> >         MT_WCID_FLAG_PS,
> >         MT_WCID_FLAG_4ADDR,
> >         MT_WCID_FLAG_HDR_TRANS,
> > +       MT_WCID_FLAG_DRV_PUBLISH,
> >  };
> >
> >  #define MT76_N_WCIDS 1088
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/main.c b/drivers/net/wireless/mediatek/mt76/mt7925/main.c
> > index 73d3722739d0..35b5c718475c 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7925/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7925/main.c
> > @@ -1102,6 +1102,9 @@ int mt7925_mac_sta_add(struct mt76_dev *mdev, struct ieee80211_vif *vif,
> >                                               &msta->deflink);
> >         }
> >
> > +       if (!err)
> > +               set_bit(MT_WCID_FLAG_DRV_PUBLISH, &msta->deflink.wcid.flags);
> > +
> >         return err;
> >  }
> >  EXPORT_SYMBOL_GPL(mt7925_mac_sta_add);
> > --
> > 2.53.0
> >
> >

      reply	other threads:[~2026-05-27 10:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-26  6:08 [PATCH] wifi: mt76: mt7925: add wcid publish check in mt76_sta_add Jiajia Liu
2026-05-26 21:52 ` Sean Wang
2026-05-27 10:00   ` Jiajia Liu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ahbAsYwFCjsvpxEC@nature \
    --to=liujiajia@kylinos.cn \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=leon.yen@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=michael.lo@mediatek.com \
    --cc=mingyen.hsieh@mediatek.com \
    --cc=nbd@nbd.name \
    --cc=ryder.lee@mediatek.com \
    --cc=sean.wang@kernel.org \
    --cc=sean.wang@mediatek.com \
    --cc=shayne.chen@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.