[PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Arend van Spriel]

This patch fixes the following NULL pointer dereference:

> I rerun the test on a kernel with some more lock checking and got lucky.
>
> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
> with 98:fc:11:8e:94:57
> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
> 98:fc:11:8e:94:57 (try 1/3)
> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
> kernel NULL pointer dereference at 00000004
> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
> minstrel_tx_status+0x48/0xe0 [mac80211]

static inline int
rix_to_ndx(struct minstrel_sta_info *mi, int rix)
{
        int i = rix;
        for (i = rix; i >= 0; i--)
                if (mi->r[i].rix == rix)
  48:   3b 58 04                cmp    0x4(%eax),%ebx
  4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
  4d:   83 c2 01                add    $0x1,%edx

It fails because of mi->r[i] being a NULL pointer. It is allocated in
minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.

struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
struct minstrel_sta_info. During authenticate the bool is_ht is false so
minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
is not set until rate_init which is after assoc.

Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
John,

This one with tab indent instead of spaces. Noticed just after sending
the first patch.

Gr. AvS
---
 net/mac80211/rc80211_minstrel_ht.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index ff5f7b8..a58c348 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -813,6 +813,9 @@ minstrel_ht_alloc_sta(void *priv, struct ieee80211_sta *sta, gfp_t gfp)
 	if (!msp->sample_table)
 		goto error1;
 
+	msp->legacy.r = msp->ratelist;
+	msp->legacy.sample_table = msp->sample_table;
+
 	return msp;
 
 error1:
-- 
1.7.5.4

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Larry Finger]

On 02/08/2012 06:44 AM, Arend van Spriel wrote:
> This patch fixes the following NULL pointer dereference:
>
>> I rerun the test on a kernel with some more lock checking and got lucky.
>>
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>> with 98:fc:11:8e:94:57
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>> 98:fc:11:8e:94:57 (try 1/3)
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>> kernel NULL pointer dereference at 00000004
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>> minstrel_tx_status+0x48/0xe0 [mac80211]
>
> static inline int
> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
> {
>          int i = rix;
>          for (i = rix; i>= 0; i--)
>                  if (mi->r[i].rix == rix)
>    48:   3b 58 04                cmp    0x4(%eax),%ebx
>    4b:   74 4b                   je     98<minstrel_tx_status+0x98>
>    4d:   83 c2 01                add    $0x1,%edx
>
> It fails because of mi->r[i] being a NULL pointer. It is allocated in
> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>
> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
> struct minstrel_sta_info. During authenticate the bool is_ht is false so
> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
> is not set until rate_init which is after assoc.
>
> Cc: Johannes Berg<johannes@sipsolutions.net>
> Cc: Felix Fietkau<nbd@openwrt.org>
> Signed-off-by: Arend van Spriel<arend@broadcom.com>
> ---
> John,
>
> This one with tab indent instead of spaces. Noticed just after sending
> the first patch.
>
> Gr. AvS
> ---
>   net/mac80211/rc80211_minstrel_ht.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
> index ff5f7b8..a58c348 100644
> --- a/net/mac80211/rc80211_minstrel_ht.c
> +++ b/net/mac80211/rc80211_minstrel_ht.c
> @@ -813,6 +813,9 @@ minstrel_ht_alloc_sta(void *priv, struct ieee80211_sta *sta, gfp_t gfp)
>   	if (!msp->sample_table)
>   		goto error1;
>
> +	msp->legacy.r = msp->ratelist;
> +	msp->legacy.sample_table = msp->sample_table;
> +
>   	return msp;
>
>   error1:

This patch fixes the problem I was having with minstrel_tx_status with p54usb.

John: If you want, you may add a

Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

Arend: Thanks for finding this problem and fixing it.

Larry

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Felix Fietkau]

On 2012-02-08 1:44 PM, Arend van Spriel wrote:
> This patch fixes the following NULL pointer dereference:
> 
>> I rerun the test on a kernel with some more lock checking and got lucky.
>>
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>> with 98:fc:11:8e:94:57
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>> 98:fc:11:8e:94:57 (try 1/3)
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>> kernel NULL pointer dereference at 00000004
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>> minstrel_tx_status+0x48/0xe0 [mac80211]
> 
> static inline int
> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
> {
>         int i = rix;
>         for (i = rix; i >= 0; i--)
>                 if (mi->r[i].rix == rix)
>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>   4d:   83 c2 01                add    $0x1,%edx
> 
> It fails because of mi->r[i] being a NULL pointer. It is allocated in
> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
> 
> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
> struct minstrel_sta_info. During authenticate the bool is_ht is false so
> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
> is not set until rate_init which is after assoc.
I'm not sure this is the right fix. If I understand the issue correctly,
mi->r is only NULL because .rate_init has not been called yet, which is
a bug that other rate control modules might trip over as well, maybe in
more subtle ways. If this is the case, then I believe this bug should be
fixed in mac80211 instead.

Maybe it would even be a good idea to set a flag after calling
rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
before that.

- Felix

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Arend van Spriel]

On 02/08/2012 05:08 PM, Felix Fietkau wrote:
> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>> This patch fixes the following NULL pointer dereference:
>>
>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>
>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>> with 98:fc:11:8e:94:57
>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>> 98:fc:11:8e:94:57 (try 1/3)
>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>> kernel NULL pointer dereference at 00000004
>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>
>> static inline int
>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>> {
>>         int i = rix;
>>         for (i = rix; i >= 0; i--)
>>                 if (mi->r[i].rix == rix)
>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>   4d:   83 c2 01                add    $0x1,%edx
>>
>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>
>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>> is not set until rate_init which is after assoc.
> I'm not sure this is the right fix. If I understand the issue correctly,
> mi->r is only NULL because .rate_init has not been called yet, which is
> a bug that other rate control modules might trip over as well, maybe in
> more subtle ways. If this is the case, then I believe this bug should be
> fixed in mac80211 instead.
> 
> Maybe it would even be a good idea to set a flag after calling
> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
> before that.
> 
> - Felix
> 

Hi Felix,

I am not sure I can agree. minstrel_ht allocates the struct
minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
default and zero'ing the legacy field. When setting is_ht to false
consequently the contents of the legacy field should be properly set as
is done in minstrel_ht_update_caps (at use_legacy label).

Gr. AvS

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Larry Finger]

On 02/08/2012 10:27 AM, Arend van Spriel wrote:
> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>> This patch fixes the following NULL pointer dereference:
>>>
>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>> with 98:fc:11:8e:94:57
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>> kernel NULL pointer dereference at 00000004
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>
>>> static inline int
>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>> {
>>>          int i = rix;
>>>          for (i = rix; i>= 0; i--)
>>>                  if (mi->r[i].rix == rix)
>>>    48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>    4b:   74 4b                   je     98<minstrel_tx_status+0x98>
>>>    4d:   83 c2 01                add    $0x1,%edx
>>>
>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>
>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>> is not set until rate_init which is after assoc.
>> I'm not sure this is the right fix. If I understand the issue correctly,
>> mi->r is only NULL because .rate_init has not been called yet, which is
>> a bug that other rate control modules might trip over as well, maybe in
>> more subtle ways. If this is the case, then I believe this bug should be
>> fixed in mac80211 instead.
>>
>> Maybe it would even be a good idea to set a flag after calling
>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>> before that.
>>
>> - Felix
>>
>
> Hi Felix,
>
> I am not sure I can agree. minstrel_ht allocates the struct
> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
> default and zero'ing the legacy field. When setting is_ht to false
> consequently the contents of the legacy field should be properly set as
> is done in minstrel_ht_update_caps (at use_legacy label).

Once you settle on a "final" patch, I can test it as I can force the crash 
nearly instantly.

Larry

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Felix Fietkau]

On 2012-02-08 5:27 PM, Arend van Spriel wrote:
> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>> This patch fixes the following NULL pointer dereference:
>>>
>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>> with 98:fc:11:8e:94:57
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>> kernel NULL pointer dereference at 00000004
>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>
>>> static inline int
>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>> {
>>>         int i = rix;
>>>         for (i = rix; i >= 0; i--)
>>>                 if (mi->r[i].rix == rix)
>>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>>   4d:   83 c2 01                add    $0x1,%edx
>>>
>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>
>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>> is not set until rate_init which is after assoc.
>> I'm not sure this is the right fix. If I understand the issue correctly,
>> mi->r is only NULL because .rate_init has not been called yet, which is
>> a bug that other rate control modules might trip over as well, maybe in
>> more subtle ways. If this is the case, then I believe this bug should be
>> fixed in mac80211 instead.
>> 
>> Maybe it would even be a good idea to set a flag after calling
>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>> before that.
>> 
>> - Felix
>> 
> 
> Hi Felix,
> 
> I am not sure I can agree. minstrel_ht allocates the struct
> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
> default and zero'ing the legacy field. When setting is_ht to false
> consequently the contents of the legacy field should be properly set as
> is done in minstrel_ht_update_caps (at use_legacy label).
Right, minstrel_ht_update_caps initializes that field, and it's always
called from .rate_init.

While the non-HT minstrel initializes that field earlier (because it is
not stored anywhere else), it still also assumes that it isn't asked to
do tx until .rate_init is called.

It doesn't matter whether it crashes in non-HT minstrel or not, the main
issue is that assumptions of both minstrel and minstrel_ht (and possibly
other rate control implementations) are being violated by mac80211.

Before we can address that problem, we need to decide whether we want to
change the code to no longer require .rate_init before tx (which I think
is a bad idea, since there's no list of supported rates available yet at
that point in time), or if we change mac80211 to no longer violate those
assumptions. In either case, this patch is insufficient.

- Felix

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Felix Fietkau]

On 2012-02-08 6:06 PM, Felix Fietkau wrote:
> On 2012-02-08 5:27 PM, Arend van Spriel wrote:
>> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>>> This patch fixes the following NULL pointer dereference:
>>>>
>>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>>
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>>> with 98:fc:11:8e:94:57
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>>> kernel NULL pointer dereference at 00000004
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>>
>>>> static inline int
>>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>>> {
>>>>         int i = rix;
>>>>         for (i = rix; i >= 0; i--)
>>>>                 if (mi->r[i].rix == rix)
>>>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>>>   4d:   83 c2 01                add    $0x1,%edx
>>>>
>>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>>
>>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>>> is not set until rate_init which is after assoc.
>>> I'm not sure this is the right fix. If I understand the issue correctly,
>>> mi->r is only NULL because .rate_init has not been called yet, which is
>>> a bug that other rate control modules might trip over as well, maybe in
>>> more subtle ways. If this is the case, then I believe this bug should be
>>> fixed in mac80211 instead.
>>> 
>>> Maybe it would even be a good idea to set a flag after calling
>>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>>> before that.
>>> 
>>> - Felix
>>> 
>> 
>> Hi Felix,
>> 
>> I am not sure I can agree. minstrel_ht allocates the struct
>> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
>> default and zero'ing the legacy field. When setting is_ht to false
>> consequently the contents of the legacy field should be properly set as
>> is done in minstrel_ht_update_caps (at use_legacy label).
> Right, minstrel_ht_update_caps initializes that field, and it's always
> called from .rate_init.
> 
> While the non-HT minstrel initializes that field earlier (because it is
> not stored anywhere else), it still also assumes that it isn't asked to
> do tx until .rate_init is called.
> 
> It doesn't matter whether it crashes in non-HT minstrel or not, the main
> issue is that assumptions of both minstrel and minstrel_ht (and possibly
> other rate control implementations) are being violated by mac80211.
> 
> Before we can address that problem, we need to decide whether we want to
> change the code to no longer require .rate_init before tx (which I think
> is a bad idea, since there's no list of supported rates available yet at
> that point in time), or if we change mac80211 to no longer violate those
> assumptions. In either case, this patch is insufficient.
Actually, I took a quick look at the code and it seems to be easier
than I thought. mac80211 sets a flag after rate_control_rate_init
already, only the tx status path is lacking a check for it. This simple
patch should take care of it, please test:

---
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -37,7 +37,7 @@ static inline void rate_control_tx_statu
 	struct ieee80211_sta *ista = &sta->sta;
 	void *priv_sta = sta->rate_ctrl_priv;
 
-	if (!ref)
+	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
 		return;
 
 	ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Arend van Spriel]

On 02/08/2012 06:29 PM, Felix Fietkau wrote:
> On 2012-02-08 6:06 PM, Felix Fietkau wrote:
>> On 2012-02-08 5:27 PM, Arend van Spriel wrote:
>>> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>>>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>>>> This patch fixes the following NULL pointer dereference:
>>>>>
>>>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>>>
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>>>> with 98:fc:11:8e:94:57
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>>>> kernel NULL pointer dereference at 00000004
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>>>
>>>>> static inline int
>>>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>>>> {
>>>>>         int i = rix;
>>>>>         for (i = rix; i >= 0; i--)
>>>>>                 if (mi->r[i].rix == rix)
>>>>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>>>>   4d:   83 c2 01                add    $0x1,%edx
>>>>>
>>>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>>>
>>>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>>>> is not set until rate_init which is after assoc.
>>>> I'm not sure this is the right fix. If I understand the issue correctly,
>>>> mi->r is only NULL because .rate_init has not been called yet, which is
>>>> a bug that other rate control modules might trip over as well, maybe in
>>>> more subtle ways. If this is the case, then I believe this bug should be
>>>> fixed in mac80211 instead.
>>>>
>>>> Maybe it would even be a good idea to set a flag after calling
>>>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>>>> before that.
>>>>
>>>> - Felix
>>>>
>>>
>>> Hi Felix,
>>>
>>> I am not sure I can agree. minstrel_ht allocates the struct
>>> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
>>> default and zero'ing the legacy field. When setting is_ht to false
>>> consequently the contents of the legacy field should be properly set as
>>> is done in minstrel_ht_update_caps (at use_legacy label).
>> Right, minstrel_ht_update_caps initializes that field, and it's always
>> called from .rate_init.
>>
>> While the non-HT minstrel initializes that field earlier (because it is
>> not stored anywhere else), it still also assumes that it isn't asked to
>> do tx until .rate_init is called.
>>
>> It doesn't matter whether it crashes in non-HT minstrel or not, the main
>> issue is that assumptions of both minstrel and minstrel_ht (and possibly
>> other rate control implementations) are being violated by mac80211.
>>
>> Before we can address that problem, we need to decide whether we want to
>> change the code to no longer require .rate_init before tx (which I think
>> is a bad idea, since there's no list of supported rates available yet at
>> that point in time), or if we change mac80211 to no longer violate those
>> assumptions. In either case, this patch is insufficient.
> Actually, I took a quick look at the code and it seems to be easier
> than I thought. mac80211 sets a flag after rate_control_rate_init
> already, only the tx status path is lacking a check for it. This simple
> patch should take care of it, please test:
> 

I will test it. So basically we don't want to do rate control for
management frames like AUTH and ASSOC?

Gr. AvS

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Felix Fietkau]

On 2012-02-08 6:35 PM, Arend van Spriel wrote:
> On 02/08/2012 06:29 PM, Felix Fietkau wrote:
>> On 2012-02-08 6:06 PM, Felix Fietkau wrote:
>>> On 2012-02-08 5:27 PM, Arend van Spriel wrote:
>>>> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>>>>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>>>>> This patch fixes the following NULL pointer dereference:
>>>>>>
>>>>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>>>>
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>>>>> with 98:fc:11:8e:94:57
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>>>>> kernel NULL pointer dereference at 00000004
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>>>>
>>>>>> static inline int
>>>>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>>>>> {
>>>>>>         int i = rix;
>>>>>>         for (i = rix; i >= 0; i--)
>>>>>>                 if (mi->r[i].rix == rix)
>>>>>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>>>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>>>>>   4d:   83 c2 01                add    $0x1,%edx
>>>>>>
>>>>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>>>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>>>>
>>>>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>>>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>>>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>>>>> is not set until rate_init which is after assoc.
>>>>> I'm not sure this is the right fix. If I understand the issue correctly,
>>>>> mi->r is only NULL because .rate_init has not been called yet, which is
>>>>> a bug that other rate control modules might trip over as well, maybe in
>>>>> more subtle ways. If this is the case, then I believe this bug should be
>>>>> fixed in mac80211 instead.
>>>>>
>>>>> Maybe it would even be a good idea to set a flag after calling
>>>>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>>>>> before that.
>>>>>
>>>>> - Felix
>>>>>
>>>>
>>>> Hi Felix,
>>>>
>>>> I am not sure I can agree. minstrel_ht allocates the struct
>>>> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
>>>> default and zero'ing the legacy field. When setting is_ht to false
>>>> consequently the contents of the legacy field should be properly set as
>>>> is done in minstrel_ht_update_caps (at use_legacy label).
>>> Right, minstrel_ht_update_caps initializes that field, and it's always
>>> called from .rate_init.
>>>
>>> While the non-HT minstrel initializes that field earlier (because it is
>>> not stored anywhere else), it still also assumes that it isn't asked to
>>> do tx until .rate_init is called.
>>>
>>> It doesn't matter whether it crashes in non-HT minstrel or not, the main
>>> issue is that assumptions of both minstrel and minstrel_ht (and possibly
>>> other rate control implementations) are being violated by mac80211.
>>>
>>> Before we can address that problem, we need to decide whether we want to
>>> change the code to no longer require .rate_init before tx (which I think
>>> is a bad idea, since there's no list of supported rates available yet at
>>> that point in time), or if we change mac80211 to no longer violate those
>>> assumptions. In either case, this patch is insufficient.
>> Actually, I took a quick look at the code and it seems to be easier
>> than I thought. mac80211 sets a flag after rate_control_rate_init
>> already, only the tx status path is lacking a check for it. This simple
>> patch should take care of it, please test:
>> 
> 
> I will test it. So basically we don't want to do rate control for
> management frames like AUTH and ASSOC?
Right. That's already handled like this by the current code, only the tx
status path was left out (probably accidentally).

- Felix

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Johannes Berg]

On 2/8/2012 6:40 PM, Felix Fietkau wrote:
>> I will test it. So basically we don't want to do rate control for
>> management frames like AUTH and ASSOC?
> Right. That's already handled like this by the current code, only the tx
> status path was left out (probably accidentally).

Yes, my mistake, I forgot about TX status path. Thanks everyone for 
taking care of this!

johannes

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Arend van Spriel]

On 02/08/2012 06:29 PM, Felix Fietkau wrote:
> On 2012-02-08 6:06 PM, Felix Fietkau wrote:
>> On 2012-02-08 5:27 PM, Arend van Spriel wrote:
>>> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>>>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>>>> This patch fixes the following NULL pointer dereference:
>>>>>
>>>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>>>
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>>>> with 98:fc:11:8e:94:57
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>>>> kernel NULL pointer dereference at 00000004
>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>>>
>>>>> static inline int
>>>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>>>> {
>>>>>         int i = rix;
>>>>>         for (i = rix; i >= 0; i--)
>>>>>                 if (mi->r[i].rix == rix)
>>>>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>>>>   4d:   83 c2 01                add    $0x1,%edx
>>>>>
>>>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>>>
>>>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>>>> is not set until rate_init which is after assoc.
>>>> I'm not sure this is the right fix. If I understand the issue correctly,
>>>> mi->r is only NULL because .rate_init has not been called yet, which is
>>>> a bug that other rate control modules might trip over as well, maybe in
>>>> more subtle ways. If this is the case, then I believe this bug should be
>>>> fixed in mac80211 instead.
>>>>
>>>> Maybe it would even be a good idea to set a flag after calling
>>>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>>>> before that.
>>>>
>>>> - Felix
>>>>
>>>
>>> Hi Felix,
>>>
>>> I am not sure I can agree. minstrel_ht allocates the struct
>>> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
>>> default and zero'ing the legacy field. When setting is_ht to false
>>> consequently the contents of the legacy field should be properly set as
>>> is done in minstrel_ht_update_caps (at use_legacy label).
>> Right, minstrel_ht_update_caps initializes that field, and it's always
>> called from .rate_init.
>>
>> While the non-HT minstrel initializes that field earlier (because it is
>> not stored anywhere else), it still also assumes that it isn't asked to
>> do tx until .rate_init is called.
>>
>> It doesn't matter whether it crashes in non-HT minstrel or not, the main
>> issue is that assumptions of both minstrel and minstrel_ht (and possibly
>> other rate control implementations) are being violated by mac80211.
>>
>> Before we can address that problem, we need to decide whether we want to
>> change the code to no longer require .rate_init before tx (which I think
>> is a bad idea, since there's no list of supported rates available yet at
>> that point in time), or if we change mac80211 to no longer violate those
>> assumptions. In either case, this patch is insufficient.
> Actually, I took a quick look at the code and it seems to be easier
> than I thought. mac80211 sets a flag after rate_control_rate_init
> already, only the tx status path is lacking a check for it. This simple
> patch should take care of it, please test:
> 
> ---
> --- a/net/mac80211/rate.h
> +++ b/net/mac80211/rate.h
> @@ -37,7 +37,7 @@ static inline void rate_control_tx_statu
>  	struct ieee80211_sta *ista = &sta->sta;
>  	void *priv_sta = sta->rate_ctrl_priv;
>  
> -	if (!ref)
> +	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
>  		return;
>  
>  	ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
> 
> 

Works for me so you can sent this patch with
Tested-by: Arend van Spriel <arend@broadcom.com>

Gr. AvS

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Larry Finger]

On 02/08/2012 12:04 PM, Arend van Spriel wrote:
> On 02/08/2012 06:29 PM, Felix Fietkau wrote:
>> On 2012-02-08 6:06 PM, Felix Fietkau wrote:
>>> On 2012-02-08 5:27 PM, Arend van Spriel wrote:
>>>> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>>>>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>>>>> This patch fixes the following NULL pointer dereference:
>>>>>>
>>>>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>>>>
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>>>>> with 98:fc:11:8e:94:57
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>>>>> kernel NULL pointer dereference at 00000004
>>>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>>>>
>>>>>> static inline int
>>>>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>>>>> {
>>>>>>          int i = rix;
>>>>>>          for (i = rix; i>= 0; i--)
>>>>>>                  if (mi->r[i].rix == rix)
>>>>>>    48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>>>>    4b:   74 4b                   je     98<minstrel_tx_status+0x98>
>>>>>>    4d:   83 c2 01                add    $0x1,%edx
>>>>>>
>>>>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>>>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>>>>
>>>>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>>>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>>>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>>>>> is not set until rate_init which is after assoc.
>>>>> I'm not sure this is the right fix. If I understand the issue correctly,
>>>>> mi->r is only NULL because .rate_init has not been called yet, which is
>>>>> a bug that other rate control modules might trip over as well, maybe in
>>>>> more subtle ways. If this is the case, then I believe this bug should be
>>>>> fixed in mac80211 instead.
>>>>>
>>>>> Maybe it would even be a good idea to set a flag after calling
>>>>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>>>>> before that.
>>>>>
>>>>> - Felix
>>>>>
>>>>
>>>> Hi Felix,
>>>>
>>>> I am not sure I can agree. minstrel_ht allocates the struct
>>>> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
>>>> default and zero'ing the legacy field. When setting is_ht to false
>>>> consequently the contents of the legacy field should be properly set as
>>>> is done in minstrel_ht_update_caps (at use_legacy label).
>>> Right, minstrel_ht_update_caps initializes that field, and it's always
>>> called from .rate_init.
>>>
>>> While the non-HT minstrel initializes that field earlier (because it is
>>> not stored anywhere else), it still also assumes that it isn't asked to
>>> do tx until .rate_init is called.
>>>
>>> It doesn't matter whether it crashes in non-HT minstrel or not, the main
>>> issue is that assumptions of both minstrel and minstrel_ht (and possibly
>>> other rate control implementations) are being violated by mac80211.
>>>
>>> Before we can address that problem, we need to decide whether we want to
>>> change the code to no longer require .rate_init before tx (which I think
>>> is a bad idea, since there's no list of supported rates available yet at
>>> that point in time), or if we change mac80211 to no longer violate those
>>> assumptions. In either case, this patch is insufficient.
>> Actually, I took a quick look at the code and it seems to be easier
>> than I thought. mac80211 sets a flag after rate_control_rate_init
>> already, only the tx status path is lacking a check for it. This simple
>> patch should take care of it, please test:
>>
>> ---
>> --- a/net/mac80211/rate.h
>> +++ b/net/mac80211/rate.h
>> @@ -37,7 +37,7 @@ static inline void rate_control_tx_statu
>>   	struct ieee80211_sta *ista =&sta->sta;
>>   	void *priv_sta = sta->rate_ctrl_priv;
>>
>> -	if (!ref)
>> +	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
>>   		return;
>>
>>   	ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
>>
>>
>
> Works for me so you can sent this patch with
> Tested-by: Arend van Spriel<arend@broadcom.com>

It works for me as well.

Larry

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Arend van Spriel]

On 02/08/2012 06:14 PM, Daniel Halperin wrote:
> On Wed, Feb 8, 2012 at 9:06 AM, Felix Fietkau <nbd@openwrt.org<mailto:nbd@openwrt.org>> wrote:
> On 2012-02-08 5:27 PM, Arend van Spriel wrote:
>> On 02/08/2012 05:08 PM, Felix Fietkau wrote:
>>> On 2012-02-08 1:44 PM, Arend van Spriel wrote:
>>>> This patch fixes the following NULL pointer dereference:
>>>>
>>>>> I rerun the test on a kernel with some more lock checking and got lucky.
>>>>>
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>>>>> with 98:fc:11:8e:94:57
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>>>>> 98:fc:11:8e:94:57 (try 1/3)
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>>>>> kernel NULL pointer dereference at 00000004
>>>>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>>>>> minstrel_tx_status+0x48/0xe0 [mac80211]
>>>>
>>>> static inline int
>>>> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>>> {
>>>>         int i = rix;
>>>>         for (i = rix; i >= 0; i--)
>>>>                 if (mi->r[i].rix == rix)
>>>>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>>>>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>>>>   4d:   83 c2 01                add    $0x1,%edx
>>>>
>>>> It fails because of mi->r[i] being a NULL pointer. It is allocated in
>>>> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
>>>>
>>>> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
>>>> struct minstrel_sta_info. During authenticate the bool is_ht is false so
>>>> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
>>>> is not set until rate_init which is after assoc.
>>> I'm not sure this is the right fix. If I understand the issue correctly,
>>> mi->r is only NULL because .rate_init has not been called yet, which is
>>> a bug that other rate control modules might trip over as well, maybe in
>>> more subtle ways. If this is the case, then I believe this bug should be
>>> fixed in mac80211 instead.
>>>
>>> Maybe it would even be a good idea to set a flag after calling
>>> rate_control_rate_init, and calling WARN_ON or even BUG_ON when doing tx
>>> before that.
>>>
>>> - Felix
>>>
>>
>> Hi Felix,
>>
>> I am not sure I can agree. minstrel_ht allocates the struct
>> minstrel_ht_sta_priv initializing is_ht to false thereby choosing a
>> default and zero'ing the legacy field. When setting is_ht to false
>> consequently the contents of the legacy field should be properly set as
>> is done in minstrel_ht_update_caps (at use_legacy label).
> Right, minstrel_ht_update_caps initializes that field, and it's always
> called from .rate_init.
> 
> While the non-HT minstrel initializes that field earlier (because it is
> not stored anywhere else), it still also assumes that it isn't asked to
> do tx until .rate_init is called.
> 
> It doesn't matter whether it crashes in non-HT minstrel or not, the main
> issue is that assumptions of both minstrel and minstrel_ht (and possibly
> other rate control implementations) are being violated by mac80211.
> 
> Before we can address that problem, we need to decide whether we want to
> change the code to no longer require .rate_init before tx (which I think
> is a bad idea, since there's no list of supported rates available yet at
> that point in time), or if we change mac80211 to no longer violate those
> assumptions. In either case, this patch is insufficient.
> 
> My two cents: Felix is right---this whole process is a bit confusing and hard to work around. I know that iwlwifi has had to implement a bunch of weird things because of the strange order in which mac80211 tells rate modules about clients. Taking the time work out a sane order, that's clearly specified, would help a lot in this space. We can patch minstrel_ht but it doesn't make it easier for other modules.
> 
> Dan
> 

Ok,

I think we are discussing the rate_control interface here and what ops
is called when.

The .alloc_sta is being called when a sta is created. The .rate_init
callback is called at different points in mac80211 depending on the mode
(STA, P2P, TDLS, etc.). Commonality seems to be that .rate_init is being
called when the connection has been established, ie. when the actual
rates to be used over that connection are known/negotiated. Before this
point the supported rates available are basically the ones supported by
the local device and these are known upon calling .alloc_sta. The
negotiated rates are always the supported rates or a subset.

Gr. AvS

Re: [PATCH v2] mac80211: minstrel_ht: fix NULL pointer dereference during auth/assoc

[Arend van Spriel]

On 02/08/2012 01:44 PM, Arend van Spriel wrote:
> This patch fixes the following NULL pointer dereference:
> 
>> I rerun the test on a kernel with some more lock checking and got lucky.
>>
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512283] wlan0: authenticate
>> with 98:fc:11:8e:94:57
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.512515] wlan0: send auth to
>> 98:fc:11:8e:94:57 (try 1/3)
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514184] BUG: unable to handle
>> kernel NULL pointer dereference at 00000004
>> Feb  8 08:40:17 lb-bun-10 kernel: [  514.514233] IP: [<f8648f08>]
>> minstrel_tx_status+0x48/0xe0 [mac80211]
> 
> static inline int
> rix_to_ndx(struct minstrel_sta_info *mi, int rix)
> {
>         int i = rix;
>         for (i = rix; i >= 0; i--)
>                 if (mi->r[i].rix == rix)
>   48:   3b 58 04                cmp    0x4(%eax),%ebx
>   4b:   74 4b                   je     98 <minstrel_tx_status+0x98>
>   4d:   83 c2 01                add    $0x1,%edx
> 
> It fails because of mi->r[i] being a NULL pointer. It is allocated in
> minstel_alloc_sta, but minstrel_ht_alloc_sta does not call that function.
> 
> struct minstrel_ht_sta_priv holds a union with struct minstrel_ht_sta and
> struct minstrel_sta_info. During authenticate the bool is_ht is false so
> minstrel_ht_tx_status calls minstrel_tx_status, but minstrel_sta_info::r
> is not set until rate_init which is after assoc.
> 
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
> John,
> 
> This one with tab indent instead of spaces. Noticed just after sending
> the first patch.
> 
> Gr. AvS
> ---
>  net/mac80211/rc80211_minstrel_ht.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
> index ff5f7b8..a58c348 100644
> --- a/net/mac80211/rc80211_minstrel_ht.c
> +++ b/net/mac80211/rc80211_minstrel_ht.c
> @@ -813,6 +813,9 @@ minstrel_ht_alloc_sta(void *priv, struct ieee80211_sta *sta, gfp_t gfp)
>  	if (!msp->sample_table)
>  		goto error1;
>  
> +	msp->legacy.r = msp->ratelist;
> +	msp->legacy.sample_table = msp->sample_table;
> +
>  	return msp;
>  
>  error1:

Hi John,

As Felix submitted a replacement patch, please drop this one.

Gr. AvS