| Message ID | 20180930105300.30797-1-stefan.seyfried@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | cfg80211: fix wext-compat memory leak | expand |
Hi Stefan, Thanks for the investigation & fix. I've changed the fix to use cfg80211_sinfo_release_content(), take care of the error path in cfg80211_wext_giwrate() and added a Fixes tag :-) Applied with those changes. johannes On Sun, 2018-09-30 at 12:53 +0200, stefan.seyfried@googlemail.com wrote: > From: Stefan Seyfried <seife+kernel@b1-systems.com> > > cfg80211_wext_giwrate and sinfo.pertid might allocate sinfo.pertid via > rdev_get_station(), but never release it > > Signed-off-by: Stefan Seyfried <seife+kernel@b1-systems.com> > --- > net/wireless/wext-compat.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c > index 167f7025ac98..f462336aac1c 100644 > --- a/net/wireless/wext-compat.c > +++ b/net/wireless/wext-compat.c > @@ -1277,12 +1277,16 @@ static int cfg80211_wext_giwrate(struct net_device *dev, > err = rdev_get_station(rdev, dev, addr, &sinfo); > if (err) > return err; > - > if (!(sinfo.filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE))) > return -EOPNOTSUPP; > > rate->value = 100000 * cfg80211_calculate_bitrate(&sinfo.txrate); > > + /* sta_set_sinfo(), called from ieee80211_get_station(), called from > + * rdev_get_station via rdev->ops->get_station, allocates pertid struct > + * which we do not use here. */ > + kfree(sinfo.pertid); > + > return 0; > } > > @@ -1293,7 +1297,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev) > struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy); > /* we are under RTNL - globally locked - so can use static structs */ > static struct iw_statistics wstats; > - static struct station_info sinfo; > + static struct station_info sinfo = {}; > u8 bssid[ETH_ALEN]; > > if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION) > @@ -1352,6 +1356,9 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev) > if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_TX_FAILED)) > wstats.discard.retries = sinfo.tx_failed; > > + /* see cfg80211_wext_giwrate() above */ > + kfree(sinfo.pertid); > + > return &wstats; > } >
Hi Johannes, Am 01.10.18 um 09:12 schrieb Johannes Berg: > Hi Stefan, > > Thanks for the investigation & fix. I've changed the fix to use > cfg80211_sinfo_release_content(), Being totally unfamiliar with the code, I was not sure about that :-) > take care of the error path in > cfg80211_wext_giwrate() and added a Fixes tag :-) That's certainly much better. Thanks! (kudos to Vlastimil Babka who pointed out trace-cmd to me on the opensuse-kernel malinglist, which made the investigation quite easy)
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c index 167f7025ac98..f462336aac1c 100644 --- a/net/wireless/wext-compat.c +++ b/net/wireless/wext-compat.c @@ -1277,12 +1277,16 @@ static int cfg80211_wext_giwrate(struct net_device *dev, err = rdev_get_station(rdev, dev, addr, &sinfo); if (err) return err; - if (!(sinfo.filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE))) return -EOPNOTSUPP; rate->value = 100000 * cfg80211_calculate_bitrate(&sinfo.txrate); + /* sta_set_sinfo(), called from ieee80211_get_station(), called from + * rdev_get_station via rdev->ops->get_station, allocates pertid struct + * which we do not use here. */ + kfree(sinfo.pertid); + return 0; } @@ -1293,7 +1297,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev) struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy); /* we are under RTNL - globally locked - so can use static structs */ static struct iw_statistics wstats; - static struct station_info sinfo; + static struct station_info sinfo = {}; u8 bssid[ETH_ALEN]; if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION) @@ -1352,6 +1356,9 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev) if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_TX_FAILED)) wstats.discard.retries = sinfo.tx_failed; + /* see cfg80211_wext_giwrate() above */ + kfree(sinfo.pertid); + return &wstats; }