| Message ID | 20240917110942.22077-1-nbd@nbd.name (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 34b69548108480ebb36cb6a067974a88ec745897 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: mt76: do not increase mcu skb refcount if retry is not supported | expand |
Felix Fietkau <nbd@nbd.name> writes: > If mcu_skb_prepare_msg is not implemented, incrementing skb refcount does not > work for mcu message retry. In some cases (e.g. on SDIO), shared skbs can trigger > a BUG_ON, crashing the system. > Fix this by only incrementing refcount if retry is actually supported. > > Fixes: 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") > Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> #KernelCI > Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > Signed-off-by: Felix Fietkau <nbd@nbd.name> I'll queue this to wireless tree and add: Closes: https://lore.kernel.org/r/d907b13a-f8be-4cb8-a0bb-560a21278041@notapiano/ Ok?
On 17.09.24 13:18, Kalle Valo wrote: > Felix Fietkau <nbd@nbd.name> writes: > >> If mcu_skb_prepare_msg is not implemented, incrementing skb refcount does not >> work for mcu message retry. In some cases (e.g. on SDIO), shared skbs can trigger >> a BUG_ON, crashing the system. >> Fix this by only incrementing refcount if retry is actually supported. >> >> Fixes: 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") >> Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> #KernelCI >> Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >> Signed-off-by: Felix Fietkau <nbd@nbd.name> > > I'll queue this to wireless tree and add: > > Closes: https://lore.kernel.org/r/d907b13a-f8be-4cb8-a0bb-560a21278041@notapiano/ > > Ok? Sure, thanks! - Felix
Felix Fietkau <nbd@nbd.name> wrote: > If mcu_skb_prepare_msg is not implemented, incrementing skb refcount does not > work for mcu message retry. In some cases (e.g. on SDIO), shared skbs can trigger > a BUG_ON, crashing the system. > Fix this by only incrementing refcount if retry is actually supported. > > Fixes: 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") > Closes: https://lore.kernel.org/r/d907b13a-f8be-4cb8-a0bb-560a21278041@notapiano/ > Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> #KernelCI > Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > Signed-off-by: Felix Fietkau <nbd@nbd.name> Patch applied to wireless.git, thanks. 34b695481084 wifi: mt76: do not increase mcu skb refcount if retry is not supported
On 9/17/24 4:09 PM, Felix Fietkau wrote: > If mcu_skb_prepare_msg is not implemented, incrementing skb refcount does not > work for mcu message retry. In some cases (e.g. on SDIO), shared skbs can trigger > a BUG_ON, crashing the system. > Fix this by only incrementing refcount if retry is actually supported. > > Fixes: 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") > Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> #KernelCI > Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > Signed-off-by: Felix Fietkau <nbd@nbd.name> > --- > drivers/net/wireless/mediatek/mt76/mcu.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mcu.c b/drivers/net/wireless/mediatek/mt76/mcu.c > index 98da82b74094..3353012e8542 100644 > --- a/drivers/net/wireless/mediatek/mt76/mcu.c > +++ b/drivers/net/wireless/mediatek/mt76/mcu.c > @@ -84,13 +84,16 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb, > mutex_lock(&dev->mcu.mutex); > > if (dev->mcu_ops->mcu_skb_prepare_msg) { > + orig_skb = skb; > ret = dev->mcu_ops->mcu_skb_prepare_msg(dev, skb, cmd, &seq); > if (ret < 0) > goto out; > } > > retry: > - orig_skb = skb_get(skb); > + /* orig skb might be needed for retry, mcu_skb_send_msg consumes it */ > + if (orig_skb) > + skb_get(orig_skb); > ret = dev->mcu_ops->mcu_skb_send_msg(dev, skb, cmd, &seq); > if (ret < 0) > goto out; > @@ -105,7 +108,7 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb, > do { > skb = mt76_mcu_get_response(dev, expires); > if (!skb && !test_bit(MT76_MCU_RESET, &dev->phy.state) && > - retry++ < dev->mcu_ops->max_retry) { > + orig_skb && retry++ < dev->mcu_ops->max_retry) { > dev_err(dev->dev, "Retry message %08x (seq %d)\n", > cmd, seq); > skb = orig_skb; This patch is in next from 5 weeks. As 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") is already in the mainline, why this fix hasn't been included in mainline? I thought fixes are included as soon as possible in mainline RCs. Am I missing something? Are we planning to include this fix in next release instead of current one?
Muhammad Usama Anjum <Usama.Anjum@collabora.com> writes: > On 9/17/24 4:09 PM, Felix Fietkau wrote: > >> If mcu_skb_prepare_msg is not implemented, incrementing skb refcount does not >> work for mcu message retry. In some cases (e.g. on SDIO), shared skbs can trigger >> a BUG_ON, crashing the system. >> Fix this by only incrementing refcount if retry is actually supported. >> >> Fixes: 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") >> Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> #KernelCI >> Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >> Signed-off-by: Felix Fietkau <nbd@nbd.name> >> --- >> drivers/net/wireless/mediatek/mt76/mcu.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/wireless/mediatek/mt76/mcu.c b/drivers/net/wireless/mediatek/mt76/mcu.c >> index 98da82b74094..3353012e8542 100644 >> --- a/drivers/net/wireless/mediatek/mt76/mcu.c >> +++ b/drivers/net/wireless/mediatek/mt76/mcu.c >> @@ -84,13 +84,16 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb, >> mutex_lock(&dev->mcu.mutex); >> >> if (dev->mcu_ops->mcu_skb_prepare_msg) { >> + orig_skb = skb; >> ret = dev->mcu_ops->mcu_skb_prepare_msg(dev, skb, cmd, &seq); >> if (ret < 0) >> goto out; >> } >> >> retry: >> - orig_skb = skb_get(skb); >> + /* orig skb might be needed for retry, mcu_skb_send_msg consumes it */ >> + if (orig_skb) >> + skb_get(orig_skb); >> ret = dev->mcu_ops->mcu_skb_send_msg(dev, skb, cmd, &seq); >> if (ret < 0) >> goto out; >> @@ -105,7 +108,7 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb, >> do { >> skb = mt76_mcu_get_response(dev, expires); >> if (!skb && !test_bit(MT76_MCU_RESET, &dev->phy.state) && >> - retry++ < dev->mcu_ops->max_retry) { >> + orig_skb && retry++ < dev->mcu_ops->max_retry) { >> dev_err(dev->dev, "Retry message %08x (seq %d)\n", >> cmd, seq); >> skb = orig_skb; > This patch is in next from 5 weeks. As 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") is > already in the mainline, why this fix hasn't been included in mainline? I thought fixes are included > as soon as possible in mainline RCs. Am I missing something? > > Are we planning to include this fix in next release instead of current one? The commit is now in wireless tree: 34b695481084 wifi: mt76: do not increase mcu skb refcount if retry is not supported With good luck it will be in v6.12-rc5 but no guarantees.
diff --git a/drivers/net/wireless/mediatek/mt76/mcu.c b/drivers/net/wireless/mediatek/mt76/mcu.c index 98da82b74094..3353012e8542 100644 --- a/drivers/net/wireless/mediatek/mt76/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mcu.c @@ -84,13 +84,16 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb, mutex_lock(&dev->mcu.mutex); if (dev->mcu_ops->mcu_skb_prepare_msg) { + orig_skb = skb; ret = dev->mcu_ops->mcu_skb_prepare_msg(dev, skb, cmd, &seq); if (ret < 0) goto out; } retry: - orig_skb = skb_get(skb); + /* orig skb might be needed for retry, mcu_skb_send_msg consumes it */ + if (orig_skb) + skb_get(orig_skb); ret = dev->mcu_ops->mcu_skb_send_msg(dev, skb, cmd, &seq); if (ret < 0) goto out; @@ -105,7 +108,7 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb, do { skb = mt76_mcu_get_response(dev, expires); if (!skb && !test_bit(MT76_MCU_RESET, &dev->phy.state) && - retry++ < dev->mcu_ops->max_retry) { + orig_skb && retry++ < dev->mcu_ops->max_retry) { dev_err(dev->dev, "Retry message %08x (seq %d)\n", cmd, seq); skb = orig_skb;
If mcu_skb_prepare_msg is not implemented, incrementing skb refcount does not work for mcu message retry. In some cases (e.g. on SDIO), shared skbs can trigger a BUG_ON, crashing the system. Fix this by only incrementing refcount if retry is actually supported. Fixes: 3688c18b65ae ("wifi: mt76: mt7915: retry mcu messages") Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> #KernelCI Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> Signed-off-by: Felix Fietkau <nbd@nbd.name> --- drivers/net/wireless/mediatek/mt76/mcu.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)