linux-kernel - RE: [PATCH 1/2] mac80211/wlcore: Add ieee80211_hw variable to get_expected

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: <F52F6CF13382174692D81838F3DBEF41136F0441@DFRE01.ent.ti.com>
Date:	Sun, 7 Aug 2016 13:42:56 +0000
From:	"Altshul, Maxim" <maxim.altshul@...com>
To:	Johannes Berg <johannes@...solutions.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	"john.stultz@...aro.org" <john.stultz@...aro.org>,
	Kalle Valo <kvalo@...eaurora.org>,
	Eliad Peller <eliad@...ery.com>,
	"Machani, Yaniv" <yanivma@...com>
Subject: RE: [PATCH 1/2] mac80211/wlcore: Add ieee80211_hw variable to
 get_expected_throughput

Hi Johaness,
I have prepared a patch for the issue and it is waiting for me to send it, but I feel that maybe I have not explained the previous issue well enough or I did not understand your request fully.
I would like to clarify about the previous patch (the one that you applied) again:

a. The bug occurred because I have added a member called wl to the structure wl_sta, but it turned to be NULL when the function drv_get_expected_throughput was called.
b. This member was NULL because it was initialized in the wrong place (sta_add instead of update_sta_state), and thus the regression has failed. 
c. Even so, wl_sta itself was not NULL at any point.
d. This is why I have created two patches:
First patch (the one that you have applied) made it easy for the driver to access hw->priv (the problematic access to hw->priv was the reason I added wl to wl_sta in the first place, which was a mistake).
Second patch reverted the addition of wl member to wl_sta.

2. From what I have seen, other ops that take ieee80211_sta as a parameter do not check for sta->uploaded, which is why it feels a little odd to do it in drv_get_expected_throughput and nowhere else.

Please tell me how to proceed. If you still think that a patch is needed, I will send it right away!

Max

-----Original Message-----
From: Johannes Berg [mailto:johannes@...solutions.net] 
Sent: Friday, August 05, 2016 6:34 PM
To: Altshul, Maxim; linux-kernel@...r.kernel.org
Cc: john.stultz@...aro.org; Kalle Valo; Eliad Peller; Machani, Yaniv
Subject: Re: [PATCH 1/2] mac80211/wlcore: Add ieee80211_hw variable to get_expected_throughput

On Fri, 2016-08-05 at 13:25 +0000, Altshul, Maxim wrote:
> Hi,
> 1) Sorry about the change log, I will try to be clearer next time.

Just mention something about how the bug happens please, at least.

> 2+3) The issue is not that the station is not known, it's that
> wl_sta->wl was null.
> wl member is now completely removed from wl_sta (PATCH 2/2) and hw is 
> sent directly from mac80211 to the driver (so it can get hw->priv).

Right, I understand that wl_sta->wl was NULL. But the driver must have some code to assign wl_sta->wl, right? And that would be called in add_sta or sta_state. Thus the reason for the crash would be that the station wasn't actually known to the driver yet.

Even if that wasn't quite the reason here, I think we need to take it into account and check sta->uploaded before calling the driver, so I'd like you to submit a patch for that.

johannes