From: Johannes Berg <[email protected]>
This fixes memory leaks in the case where we just have the
station info on the stack for internal usage without sending
it to cfg80211.
Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/cfg.c | 4 ++--
net/mac80211/ethtool.c | 4 ++--
net/mac80211/sta_info.c | 7 ++++---
net/mac80211/sta_info.h | 3 ++-
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5ce9d121af2b..bdf6fa78d0d2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -695,7 +695,7 @@ static int ieee80211_dump_station(struct wiphy *wiphy, struct net_device *dev,
if (sta) {
ret = 0;
memcpy(mac, sta->sta.addr, ETH_ALEN);
- sta_set_sinfo(sta, sinfo);
+ sta_set_sinfo(sta, sinfo, true);
}
mutex_unlock(&local->sta_mtx);
@@ -724,7 +724,7 @@ static int ieee80211_get_station(struct wiphy *wiphy, struct net_device *dev,
sta = sta_info_get_bss(sdata, mac);
if (sta) {
ret = 0;
- sta_set_sinfo(sta, sinfo);
+ sta_set_sinfo(sta, sinfo, true);
}
mutex_unlock(&local->sta_mtx);
diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
index 2ba5686cbcab..690c142a7a44 100644
--- a/net/mac80211/ethtool.c
+++ b/net/mac80211/ethtool.c
@@ -108,7 +108,7 @@ static void ieee80211_get_stats(struct net_device *dev,
goto do_survey;
memset(&sinfo, 0, sizeof(sinfo));
- sta_set_sinfo(sta, &sinfo);
+ sta_set_sinfo(sta, &sinfo, false);
i = 0;
ADD_STA_STATS(sta);
@@ -135,7 +135,7 @@ static void ieee80211_get_stats(struct net_device *dev,
continue;
memset(&sinfo, 0, sizeof(sinfo));
- sta_set_sinfo(sta, &sinfo);
+ sta_set_sinfo(sta, &sinfo, false);
i = 0;
ADD_STA_STATS(sta);
}
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 04d47689b557..6428f1ac37b6 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1008,7 +1008,7 @@ static void __sta_info_destroy_part2(struct sta_info *sta)
sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
if (sinfo)
- sta_set_sinfo(sta, sinfo);
+ sta_set_sinfo(sta, sinfo, true);
cfg80211_del_sta_sinfo(sdata->dev, sta->sta.addr, sinfo, GFP_KERNEL);
kfree(sinfo);
@@ -2079,7 +2079,8 @@ static inline u64 sta_get_stats_bytes(struct ieee80211_sta_rx_stats *rxstats)
return value;
}
-void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
+void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
+ bool tidstats)
{
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct ieee80211_local *local = sdata->local;
@@ -2233,7 +2234,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
sinfo->filled |= BIT(NL80211_STA_INFO_RX_BITRATE);
}
- if (!cfg80211_sinfo_alloc_tid_stats(sinfo, GFP_KERNEL)) {
+ if (tidstats && !cfg80211_sinfo_alloc_tid_stats(sinfo, GFP_KERNEL)) {
for (i = 0; i < IEEE80211_NUM_TIDS + 1; i++) {
struct cfg80211_tid_stats *tidstats = &sinfo->pertid[i];
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index d79bd6eeb549..81b35f623792 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -744,7 +744,8 @@ static inline int sta_info_flush(struct ieee80211_sub_if_data *sdata)
void sta_set_rate_info_tx(struct sta_info *sta,
const struct ieee80211_tx_rate *rate,
struct rate_info *rinfo);
-void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo);
+void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
+ bool tidstats);
u32 sta_get_expected_throughput(struct sta_info *sta);
--
2.14.3
On Fri, 2018-05-18 at 12:25 +0200, Arend van Spriel wrote:
> On 5/18/2018 11:47 AM, Johannes Berg wrote:
> > From: Johannes Berg <[email protected]>
> >
> > This fixes memory leaks in cases where we got the station
> > info but failed sending it out properly.
>
> Reviewed-by: Arend van Spriel <[email protected]>
> > Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")
> > Signed-off-by: Johannes Berg <[email protected]>
> > ---
> > include/net/cfg80211.h | 13 +++++++++++++
> > net/wireless/nl80211.c | 11 ++++++++---
> > 2 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> > index 8984d24d68b7..1c6364591856 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -5710,6 +5710,19 @@ void cfg80211_remain_on_channel_expired(struct wireless_dev *wdev, u64 cookie,
> > */
> > int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
> >
> > +/**
> > + * cfg80211_sinfo_release_sinfo - release contents of station info
>
> Maybe drop one '_sinfo' from the function name? Or
> cfg80211_sinfo_release_contents?
Heh. I guess I wasn't paying attention, I like _content()
johannes
From: Johannes Berg <[email protected]>
This fixes memory leaks in cases where we got the station
info but failed sending it out properly.
Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/cfg80211.h | 13 +++++++++++++
net/wireless/nl80211.c | 11 ++++++++---
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8984d24d68b7..1c6364591856 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5710,6 +5710,19 @@ void cfg80211_remain_on_channel_expired(struct wireless_dev *wdev, u64 cookie,
*/
int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
+/**
+ * cfg80211_sinfo_release_sinfo - release contents of station info
+ * @sinfo: the station information
+ *
+ * Releases any potentially allocated sub-information of the station
+ * information, but not the struct itself (since it's typically on
+ * the stack.)
+ */
+static inline void cfg80211_sinfo_release_sinfo(struct station_info *sinfo)
+{
+ kfree(sinfo->pertid);
+}
+
/**
* cfg80211_new_sta - notify userspace about station
*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7daceb1f253d..0996fdc002e0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4702,7 +4702,6 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
}
nla_nest_end(msg, tidsattr);
- kfree(sinfo->pertid);
}
nla_nest_end(msg, sinfoattr);
@@ -4712,10 +4711,12 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
sinfo->assoc_req_ies))
goto nla_put_failure;
+ cfg80211_sinfo_release_sinfo(sinfo);
genlmsg_end(msg, hdr);
return 0;
nla_put_failure:
+ cfg80211_sinfo_release_sinfo(sinfo);
genlmsg_cancel(msg, hdr);
return -EMSGSIZE;
}
@@ -4797,8 +4798,10 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
return err;
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
- if (!msg)
+ if (!msg) {
+ cfg80211_sinfo_release_sinfo(sinfo);
return -ENOMEM;
+ }
if (nl80211_send_station(msg, NL80211_CMD_NEW_STATION,
info->snd_portid, info->snd_seq, 0,
@@ -14624,8 +14627,10 @@ void cfg80211_del_sta_sinfo(struct net_device *dev, const u8 *mac_addr,
trace_cfg80211_del_sta(dev, mac_addr);
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
- if (!msg)
+ if (!msg) {
+ cfg80211_sinfo_release_sinfo(sinfo);
return;
+ }
if (nl80211_send_station(msg, NL80211_CMD_DEL_STATION, 0, 0, 0,
rdev, dev, mac_addr, sinfo) < 0) {
--
2.14.3
On 5/18/2018 11:47 AM, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> This fixes memory leaks in cases where we got the station
> info but failed sending it out properly.
Reviewed-by: Arend van Spriel <[email protected]>
> Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> include/net/cfg80211.h | 13 +++++++++++++
> net/wireless/nl80211.c | 11 ++++++++---
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 8984d24d68b7..1c6364591856 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -5710,6 +5710,19 @@ void cfg80211_remain_on_channel_expired(struct wireless_dev *wdev, u64 cookie,
> */
> int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
>
> +/**
> + * cfg80211_sinfo_release_sinfo - release contents of station info
Maybe drop one '_sinfo' from the function name? Or
cfg80211_sinfo_release_contents?
Regards,
Arend