[PATCH 0/1] mac80211: Avoid accessing uninitialized stack data

[PATCH 0/1] mac80211: Avoid accessing uninitialized stack data

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

Working on a wifi driver I hit the case where
ieee80211_determine_chantype() could end up accessing uninitialized
stack data, if a driver advertised it's supported bands as having HT
support, but only 20MHz channel support.

This patch should take care of the issue by copying over *chandef into
vht_chandef. I don't think it would be correct to set
IEEE80211_STA_DISABLE_VHT here since I believe it is valid for a
driver to support VHT without 40+MHz support? Please correct me if I
am wrong.

This is against the current staging tree, but should be valid for 3.18
and probably also the stable kernels.

Cheers,
Jes


Jes Sorensen (1):
  wireless: mac80211: Avoid using uninitialized stack data

 net/mac80211/mlme.c | 1 +
 1 file changed, 1 insertion(+)

-- 
1.9.3

[PATCH 1/1] wireless: mac80211: Avoid using uninitialized stack data

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Avoid case where we would access uninitialized stack data if a driver
advertises HT support without 40MHz channel support.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 net/mac80211/mlme.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 93af0f1..da1f639 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -174,6 +174,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (!(ht_cap->cap_info &
 	      cpu_to_le16(IEEE80211_HT_CAP_SUP_WIDTH_20_40))) {
 		ret = IEEE80211_STA_DISABLE_40MHZ;
+		vht_chandef = *chandef;
 		goto out;
 	}
 
-- 
1.9.3

Re: [PATCH 1/1] wireless: mac80211: Avoid using uninitialized stack data

[Johannes Berg]

On Wed, 2014-12-10 at 14:14 -0500, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Avoid case where we would access uninitialized stack data if a driver
> advertises HT support without 40MHz channel support.

>  		ret = IEEE80211_STA_DISABLE_40MHZ;
> +		vht_chandef = *chandef;

I think we can and should disable VHT in this case, since VHT requires
40 and 80 MHz support.

johannes

[PATCH 1/1] wireless: mac80211: Avoid using uninitialized stack data

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Disable VHT in case driver advertises HT support without 40MHz+
channel support, in order to avoid accessing uninitialized stack data.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 net/mac80211/mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 93af0f1..d7fe2fe 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -173,7 +173,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 
 	if (!(ht_cap->cap_info &
 	      cpu_to_le16(IEEE80211_HT_CAP_SUP_WIDTH_20_40))) {
-		ret = IEEE80211_STA_DISABLE_40MHZ;
+		ret = IEEE80211_STA_DISABLE_40MHZ | IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
 
-- 
1.9.3

Re: [PATCH 1/1] wireless: mac80211: Avoid using uninitialized stack data

[Johannes Berg]

On Wed, 2014-12-10 at 14:14 -0500, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Avoid case where we would access uninitialized stack data if a driver
> advertises HT support without 40MHz channel support.

I've fixed the commit message (it's actually in the check for the *AP*,
not the driver!)

Also, this is complicated. We originally had the DISABLE_VHT, but then
found APs that were doing it wrong - see commit f3000e1b43f1 ("mac80211:
fix broken use of VHT/20Mhz with some APs"). That fix introduced the bug
here, going back now to the DISABLE_VHT as I'd suggested would break the
fix again ... I'm thus taking this version to just put the right data on
the stack, with the correct Fixes/Cc stable tags.

johannes

Re: [PATCH 1/1] wireless: mac80211: Avoid using uninitialized stack data

[Jes Sorensen]

Johannes Berg <johannes@sipsolutions.net> writes:
> On Wed, 2014-12-10 at 14:14 -0500, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> Avoid case where we would access uninitialized stack data if a driver
>> advertises HT support without 40MHz channel support.
>
> I've fixed the commit message (it's actually in the check for the *AP*,
> not the driver!)
>
> Also, this is complicated. We originally had the DISABLE_VHT, but then
> found APs that were doing it wrong - see commit f3000e1b43f1 ("mac80211:
> fix broken use of VHT/20Mhz with some APs"). That fix introduced the bug
> here, going back now to the DISABLE_VHT as I'd suggested would break the
> fix again ... I'm thus taking this version to just put the right data on
> the stack, with the correct Fixes/Cc stable tags.

Either patch works for me, so I'm all good! Thanks for fixing this up!

Cheers,
Jes