drivers/net/wireless/marvell/mwifiex/cfg80211.c | 32 ++++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-)
mwifiex_band_2ghz and mwifiex_band_5ghz are statically allocated, but
used and modified in driver instances. Duplicate them before using
them in driver instances so that different driver instances do not
influence each other.
This was observed on a board which has one PCIe and one SDIO mwifiex
adapter. It blew up in mwifiex_setup_ht_caps(). This was called with
the statically allocated struct which is modified in this function.
Cc: stable@vger.kernel.org
Fixes: d6bffe8bb520 ("mwifiex: support for creation of AP interface")
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 32 ++++++++++++++++++++-----
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b909a7665e9cc..d2e4153192032 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -4361,11 +4361,27 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
if (ISSUPP_ADHOC_ENABLED(adapter->fw_cap_info))
wiphy->interface_modes |= BIT(NL80211_IFTYPE_ADHOC);
- wiphy->bands[NL80211_BAND_2GHZ] = &mwifiex_band_2ghz;
- if (adapter->config_bands & BAND_A)
- wiphy->bands[NL80211_BAND_5GHZ] = &mwifiex_band_5ghz;
- else
+ wiphy->bands[NL80211_BAND_2GHZ] = devm_kmemdup(adapter->dev,
+ &mwifiex_band_2ghz,
+ sizeof(mwifiex_band_2ghz),
+ GFP_KERNEL);
+ if (!wiphy->bands[NL80211_BAND_2GHZ]) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ if (adapter->config_bands & BAND_A) {
+ wiphy->bands[NL80211_BAND_5GHZ] = devm_kmemdup(adapter->dev,
+ &mwifiex_band_5ghz,
+ sizeof(mwifiex_band_5ghz),
+ GFP_KERNEL);
+ if (!wiphy->bands[NL80211_BAND_5GHZ]) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ } else {
wiphy->bands[NL80211_BAND_5GHZ] = NULL;
+ }
if (adapter->drcs_enabled && ISSUPP_DRCS_ENABLED(adapter->fw_cap_info))
wiphy->iface_combinations = &mwifiex_iface_comb_ap_sta_drcs;
@@ -4459,8 +4475,7 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
if (ret < 0) {
mwifiex_dbg(adapter, ERROR,
"%s: wiphy_register failed: %d\n", __func__, ret);
- wiphy_free(wiphy);
- return ret;
+ goto err;
}
if (!adapter->regd) {
@@ -4502,4 +4517,9 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
adapter->wiphy = wiphy;
return ret;
+
+err:
+ wiphy_free(wiphy);
+
+ return ret;
}
---
base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd
change-id: 20240809-mwifiex-duplicate-static-structs-f6355e8da797
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>On Fri, Aug 09, 2024 at 10:11:33AM +0200, Sascha Hauer wrote:
> mwifiex_band_2ghz and mwifiex_band_5ghz are statically allocated, but
> used and modified in driver instances. Duplicate them before using
> them in driver instances so that different driver instances do not
> influence each other.
Ugh, I caught a few problems like this on the first several passes of
review, but I missed a few more. Thanks for the catches.
> This was observed on a board which has one PCIe and one SDIO mwifiex
> adapter. It blew up in mwifiex_setup_ht_caps(). This was called with
> the statically allocated struct which is modified in this function.
>
> Cc: stable@vger.kernel.org
> Fixes: d6bffe8bb520 ("mwifiex: support for creation of AP interface")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Brian Norris <briannorris@chromium.org>
On Fri, Aug 09, 2024 at 10:11:33AM +0200, Sascha Hauer wrote:
> mwifiex_band_2ghz and mwifiex_band_5ghz are statically allocated, but
> used and modified in driver instances. Duplicate them before using
> them in driver instances so that different driver instances do not
> influence each other.
>
> This was observed on a board which has one PCIe and one SDIO mwifiex
> adapter. It blew up in mwifiex_setup_ht_caps(). This was called with
> the statically allocated struct which is modified in this function.
>
> Cc: stable@vger.kernel.org
> Fixes: d6bffe8bb520 ("mwifiex: support for creation of AP interface")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Sascha Hauer <s.hauer@pengutronix.de> wrote: > + wiphy->bands[NL80211_BAND_2GHZ] = devm_kmemdup(adapter->dev, > + &mwifiex_band_2ghz, > + sizeof(mwifiex_band_2ghz), > + GFP_KERNEL); It seems like you forget to free the duplicate memory somewhere?
On Fri, Aug 09, 2024 at 08:49:32AM +0000, Ping-Ke Shih wrote: > Sascha Hauer <s.hauer@pengutronix.de> wrote: > > + wiphy->bands[NL80211_BAND_2GHZ] = devm_kmemdup(adapter->dev, > > + &mwifiex_band_2ghz, > > + sizeof(mwifiex_band_2ghz), > > + GFP_KERNEL); > > It seems like you forget to free the duplicate memory somewhere? It's freed automatically when adapter->dev is released, see the various devm_* functions Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Fri, Aug 09, 2024 at 08:49:32AM +0000, Ping-Ke Shih wrote: > > Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > + wiphy->bands[NL80211_BAND_2GHZ] = devm_kmemdup(adapter->dev, > > > + &mwifiex_band_2ghz, > > > + sizeof(mwifiex_band_2ghz), > > > + GFP_KERNEL); > > > > It seems like you forget to free the duplicate memory somewhere? > > It's freed automatically when adapter->dev is released, see the various > devm_* functions > Cool. Thanks for the info.
Sascha Hauer <s.hauer@pengutronix.de> writes:
> mwifiex_band_2ghz and mwifiex_band_5ghz are statically allocated, but
> used and modified in driver instances. Duplicate them before using
> them in driver instances so that different driver instances do not
> influence each other.
>
> This was observed on a board which has one PCIe and one SDIO mwifiex
> adapter. It blew up in mwifiex_setup_ht_caps(). This was called with
> the statically allocated struct which is modified in this function.
>
> Cc: stable@vger.kernel.org
> Fixes: d6bffe8bb520 ("mwifiex: support for creation of AP interface")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Should this go to wireless tree for v6.11?
"wifi:" missing in subject but I can add that, no need to resend because
of this.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Fri, Aug 09, 2024 at 11:14:36AM +0300, Kalle Valo wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
>
> > mwifiex_band_2ghz and mwifiex_band_5ghz are statically allocated, but
> > used and modified in driver instances. Duplicate them before using
> > them in driver instances so that different driver instances do not
> > influence each other.
> >
> > This was observed on a board which has one PCIe and one SDIO mwifiex
> > adapter. It blew up in mwifiex_setup_ht_caps(). This was called with
> > the statically allocated struct which is modified in this function.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: d6bffe8bb520 ("mwifiex: support for creation of AP interface")
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Should this go to wireless tree for v6.11?
Yes, that would be great.
>
> "wifi:" missing in subject but I can add that, no need to resend because
> of this.
Ok, thanks. I'll keep that in mind for the next patches.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
© 2016 - 2026 Red Hat, Inc.