netdev - Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must be NL80211_IFTYPE_AP when be in NL80211_IFTYPE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1385816013.4327.1.camel@jlt4.sipsolutions.net>
Date:	Sat, 30 Nov 2013 13:53:33 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	Chen Gang <gang.chen.5i5j@...il.com>
Cc:	"John W. Linville" <linville@...driver.com>,
	rkuo <rkuo@...eaurora.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	linux-wireless@...r.kernel.org, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' must
 be NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

On Sat, 2013-11-30 at 19:59 +0800, Chen Gang wrote:
> On 11/29/2013 11:38 PM, Johannes Berg wrote:
> > 
> >> +++ b/net/mac80211/tx.c
> >> @@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> >>  			break;
> >>  		/* fall through */
> >>  	case NL80211_IFTYPE_AP:
> >> -		if (sdata->vif.type == NL80211_IFTYPE_AP)
> >> -			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> >> +		if (sdata->vif.type != NL80211_IFTYPE_AP)
> >> +			goto fail_rcu;
> >> +		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> > 
> > This change is completely wrong.
> > 
> 
> Oh, it is.
> 
> Hmm... for me, this work flow still can be implemented with a little
> clearer way (at least it will avoid related warning):
> 
> -------------------------diff begin------------------------------
> 
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index c558b24..7076128 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>  		if (!chanctx_conf)
>  			goto fail_rcu;
>  		band = chanctx_conf->def.chan->band;
> -		if (sta)
> -			break;
> -		/* fall through */
> +		if (!sta)
> +			goto try_next;
> +		break;
>  	case NL80211_IFTYPE_AP:
> -		if (sdata->vif.type == NL80211_IFTYPE_AP)
> -			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> +		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>  		if (!chanctx_conf)
>  			goto fail_rcu;
> +try_next:

I don't think that's better than the (fairly obvious) fall-through, and
has a pretty odd goto. Also, depending on the compiler, it still knows
the previous case label and doesn't warn.

johannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html