diff mbox

[RFC,3/6] mac80211: Check for NULL in get-ethtool-stats logic.

Message ID 1355349295-30960-3-git-send-email-greearb@candelatech.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Ben Greear Dec. 12, 2012, 9:54 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

In an earlier version of this code, testing found that sometimes
either sta wasn't found or sta->sdata was NULL.  This caused
crashes of course.  So, add checks with WARN_ON_ONCE to
catch these.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/cfg.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Comments

Johannes Berg Dec. 12, 2012, 10:02 p.m. UTC | #1 
On Wed, 2012-12-12 at 13:54 -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> In an earlier version of this code, testing found that sometimes
> either sta wasn't found or sta->sdata was NULL.  This caused
> crashes of course.  So, add checks with WARN_ON_ONCE to
> catch these.

I don't know what you tested, but there's absolutely no way that
sta->sdata is NULL... *Especially* not after sta_info_get_bss(), I mean,
look at that function. But once it's on the list it can't be NULL, so
I'm not going to apply this patch.

In the future, I'd appreciate if you could avoid avoid sending patches
for drivers and mac80211 etc. in one series, it gets confusing with who
applies what etc.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Greear Dec. 12, 2012, 10:11 p.m. UTC | #2 
On 12/12/2012 02:02 PM, Johannes Berg wrote:
> On Wed, 2012-12-12 at 13:54 -0800, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> In an earlier version of this code, testing found that sometimes
>> either sta wasn't found or sta->sdata was NULL.  This caused
>> crashes of course.  So, add checks with WARN_ON_ONCE to
>> catch these.
>
> I don't know what you tested, but there's absolutely no way that
> sta->sdata is NULL... *Especially* not after sta_info_get_bss(), I mean,
> look at that function. But once it's on the list it can't be NULL, so
> I'm not going to apply this patch.
>
> In the future, I'd appreciate if you could avoid avoid sending patches
> for drivers and mac80211 etc. in one series, it gets confusing with who
> applies what etc.

Ok, it was quite a while ago..maybe I was hitting other upstream bugs or
bugs of my own.

For what it's worth, I haven't noticed my WARN_ON hitting.

I'll drop this patch from my tree.

Thanks,
Ben

>
> johannes
>
diff mbox

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5c61677..1e6dcc3 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -599,7 +599,8 @@  static void ieee80211_get_et_stats(struct wiphy *wiphy,
 	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
 		sta = sta_info_get_bss(sdata, sdata->u.mgd.bssid);
 
-		if (!(sta && !WARN_ON(sta->sdata->dev != dev)))
+		if (sta == NULL || WARN_ON_ONCE(sta->sdata == NULL)
+		    || WARN_ON(sta->sdata->dev != dev))
 			goto do_survey;
 
 		i = 0;
@@ -625,7 +626,8 @@  static void ieee80211_get_et_stats(struct wiphy *wiphy,
 	} else {
 		list_for_each_entry(sta, &local->sta_list, list) {
 			/* Make sure this station belongs to the proper dev */
-			if (sta->sdata->dev != dev)
+			if (WARN_ON_ONCE(sta->sdata == NULL) ||
+			    sta->sdata->dev != dev)
 				continue;
 
 			i = 0;