2016-09-14 12:45:10

by Bob Copeland

Subject: [PATCH] mwifiex: fix error handling in mwifiex_create_custom_regdomain

smatch reports:

sta_cmdresp.c:1053 mwifiex_create_custom_regdomain() warn: possible memory leak of 'regd'

Indeed, mwifiex_create_custom_regdomain() returns NULL in the
case that channel is missing in the TLV without freeing regd.

Moreover, some other error paths in this function return ERR_PTR
values which are assigned without checking to the regd field in
the mwifiex_adapter struct. The latter is only null-checked where
used.

Fix by freeing regd in the error path, and only update
priv->adapter->regd if the returned pointer is valid.

Cc: Amitkumar Karwar <[email protected]>
Cc: Nishant Sarmukadam <[email protected]>
Signed-off-by: Bob Copeland <[email protected]>

---
Note, compile-tested only. It might make sense to instead remove
the ERR_PTR stuff if there is no plan to report them further up the
call stack but I just went for the minimal patch here.
---
drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 3344a26..8548027 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -1049,8 +1049,10 @@ mwifiex_create_custom_regdomain(struct mwifiex_private *priv,
enum nl80211_band band;

chan = *buf++;
- if (!chan)
+ if (!chan) {
+ kfree(regd);
return NULL;
+ }
chflags = *buf++;
band = (chan <= 14) ? NL80211_BAND_2GHZ : NL80211_BAND_5GHZ;
freq = ieee80211_channel_to_frequency(chan, band);
@@ -1116,6 +1118,7 @@ static int mwifiex_ret_chan_region_cfg(struct mwifiex_private *priv,
u16 action = le16_to_cpu(reg->action);
u16 tlv, tlv_buf_len, tlv_buf_left;
struct mwifiex_ie_types_header *head;
+ struct ieee80211_regdomain *regd;
u8 *tlv_buf;

if (action != HostCmd_ACT_GEN_GET)
@@ -1137,10 +1140,10 @@ static int mwifiex_ret_chan_region_cfg(struct mwifiex_private *priv,
mwifiex_dbg_dump(priv->adapter, CMD_D, "CHAN:",
(u8 *)head + sizeof(*head),
tlv_buf_len);
- priv->adapter->regd =
- mwifiex_create_custom_regdomain(priv,
- (u8 *)head +
- sizeof(*head), tlv_buf_len);
+ regd = mwifiex_create_custom_regdomain(priv,
+ (u8 *)head + sizeof(*head), tlv_buf_len);
+ if (!IS_ERR(regd))
+ priv->adapter->regd = regd;
break;
}

--
2.9.0

2016-09-17 15:26:20

by Kalle Valo

Subject: Re: mwifiex: fix error handling in mwifiex_create_custom_regdomain

Bob Copeland <[email protected]> wrote:
> smatch reports:
>
> sta_cmdresp.c:1053 mwifiex_create_custom_regdomain() warn: possible memory leak of 'regd'
>
> Indeed, mwifiex_create_custom_regdomain() returns NULL in the
> case that channel is missing in the TLV without freeing regd.
>
> Moreover, some other error paths in this function return ERR_PTR
> values which are assigned without checking to the regd field in
> the mwifiex_adapter struct. The latter is only null-checked where
> used.
>
> Fix by freeing regd in the error path, and only update
> priv->adapter->regd if the returned pointer is valid.
>
> Cc: Amitkumar Karwar <[email protected]>
> Cc: Nishant Sarmukadam <[email protected]>
> Signed-off-by: Bob Copeland <[email protected]>

Thanks, 1 patch applied to wireless-drivers-next.git:

92ca4f92eca7 mwifiex: fix error handling in mwifiex_create_custom_regdomain

--
Sent by pwcli
https://patchwork.kernel.org/patch/9331337/