From: "Arend van Spriel" <arend@broadcom.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: "Felix Fietkau" <nbd@openwrt.org>,
"Pavel Roskin" <proski@gnu.org>,
"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"johannes@sipsolutions.net" <johannes@sipsolutions.net>
Subject: Re: [PATCH] mac80211: do not call rate control .tx_status before .rate_init
Date: Thu, 9 Feb 2012 21:21:42 +0100 [thread overview]
Message-ID: <4F342AD6.8000205@broadcom.com> (raw)
In-Reply-To: <20120209201411.GB29948@tuxdriver.com>
On 02/09/2012 09:14 PM, John W. Linville wrote:
> On Thu, Feb 09, 2012 at 01:51:51AM +0100, Felix Fietkau wrote:
>> On 2012-02-09 12:01 AM, Pavel Roskin wrote:
>>> On Wed, 8 Feb 2012 14:44:54 -0500
>>> "John W. Linville" <linville@tuxdriver.com> wrote:
>>>
>>>> On Wed, Feb 08, 2012 at 08:38:00PM +0100, Felix Fietkau wrote:
>>>>> On 2012-02-08 8:25 PM, John W. Linville wrote:
>>>>>> On Wed, Feb 08, 2012 at 07:17:11PM +0100, Felix Fietkau wrote:
>>>>>>> Most rate control implementations assume .get_rate
>>>>>>> and .tx_status are only called once the per-station data has
>>>>>>> been fully initialized. minstrel_ht crashes if this assumption
>>>>>>> is violated.
>>>>>>>
>>>>>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>>>>>> Tested-by: Arend van Spriel <arend@broadcom.com>
>>>>>>> ---
>>>>>>> net/mac80211/rate.h | 2 +-
>>>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
>>>>>>> index 5fc3135..fbb1efd 100644
>>>>>>> --- a/net/mac80211/rate.h
>>>>>>> +++ b/net/mac80211/rate.h
>>>>>>> @@ -37,7 +37,7 @@ static inline void
>>>>>>> rate_control_tx_status(struct ieee80211_local *local, 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);
>>>>>>
>>>>>> Any reason not to apply this for 3.3? Or stable?
>>>>> I think 3.3 doesn't have that sta flag, the issue was probably
>>>>> introduced with the 3.4 changes.
>>>>> I don't remember something like this appearing in earlier versions.
>>>>
>>>> Cool, thanks.
>>>
>>> I believe 3.3 is affected. At least it looks like the Fedora bug 768639
>>> (https://bugzilla.redhat.com/show_bug.cgi?id=768639) is caused by
>>> calling .tx_status at a wrong time. Fedora kernels use
>>> compat-wireless-3.3. I'm going to test the bleeding edge
>>> compat-wireless with the patch by Felix to see if it fixes things.
>>>
>>> The lack of the WLAN_STA_RATE_CONTROL flag doesn't mean that the old
>>> behavior was correct. The flag was introduced to correct that behavior.
>>>
>>> The oldest report is dated 2011-12-17 and it's about Linux 3.2.0-rc5
>>> with compat-wireless-2011-12-01.
>> Only .get_rate and .tx_status are affected, wireless-testing commit
>> e1936e9407138b483e6d1332dd944afec8131f30 adds one of the checks, and my
>> commit adds the other. Maybe John could merge those two to 3.3.
>
> At least one of them will cause some merge issues. Can someone try
> the attached patches to verify that they actually fix a real problem
> in 3.3?
>
> Thanks!
>
> John
Hi John,
This patch fixes NULL deref issue I found and bisected in
wireless-testing earlier this week (see [1]). I don't think gives a
problem with 3.3 at the moment.
Gr. AvS
[1] http://www.spinics.net/lists/linux-wireless/msg84575.html
next prev parent reply other threads:[~2012-02-09 20:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-08 18:17 [PATCH] mac80211: do not call rate control .tx_status before .rate_init Felix Fietkau
2012-02-08 19:25 ` John W. Linville
2012-02-08 19:38 ` Felix Fietkau
2012-02-08 19:44 ` John W. Linville
2012-02-08 23:01 ` Pavel Roskin
2012-02-09 0:51 ` Felix Fietkau
2012-02-09 20:14 ` John W. Linville
2012-02-09 20:21 ` Arend van Spriel [this message]
2012-02-09 23:11 ` Pavel Roskin
2012-02-10 10:05 ` jpo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F342AD6.8000205@broadcom.com \
--to=arend@broadcom.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=nbd@openwrt.org \
--cc=proski@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).