linux-kernel - Re: [PATCH v2] wifi: mac80211: re-order unassigning channel in activate links

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] [day] [month] [year] [list]

Message-ID: <8d9fdf00-8c04-430d-b5a5-edcdccbd21e7@quicinc.com>
Date: Mon, 9 Dec 2024 10:42:10 +0530
From: Aditya Kumar Singh <quic_adisi@...cinc.com>
To: Johannes Berg <johannes@...solutions.net>
CC: <linux-wireless@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] wifi: mac80211: re-order unassigning channel in
 activate links

On 12/6/24 15:37, Johannes Berg wrote:
> On Thu, 2024-12-05 at 20:43 +0530, Aditya Kumar Singh wrote:
>> On 12/5/24 18:30, Johannes Berg wrote:
>>> On Thu, 2024-12-05 at 12:43 +0100, Johannes Berg wrote:
>>>>>
>>>>> Therefore, re-order the logic so that stations are handled first and then
>>>>> channel is unassigned.
>>>>>
>>>>
>>>> This causes memory leaks in my tests with iwlwifi.
>>>>
>>>
>>> And also firmware crashes because the station is removed while it's
>>> still being used.
>>>
>>
>> So is this exposing some underlying issue with iwlwifi?
> 
> I don't think so?
> 
>> Or this change
>> will break drivers which does not group multiple hardware into single
>> wiphy?
> 
> Not necessarily, but it breaks iwlwifi because of the changed order of
> operations, and what it does with the firmware.
> 
> I think the issue here is that we treat link active == link has channel
> context in iwlwifi, and an active link in client mode requires a station
> in firmware. Otherwise you cannot even deactivate a link, since that
> requires sending an NDP to the AP, but if you don't have the AP STA you
> can't do that ...

Fair enough ...

> 
> I guess the driver could be changed to treat station links as active
> when they have the AP STA entry, but that seems ... difficult and
> strange, it would make it different between AP and client modes?
> 
> Looking at your commit message more, I wonder if it really even makes
> sense to *delete* the link when the channel context is unassigned,
> rather than (similarly to iwlwifi) deactivating it and deleting it later
> when it's actually removed (change_vif_links)? You do know which
> hardware it is/was on, after all. And these two operations can *never*
> be atomic. Removing the STAs first might be something that's appropriate
> for AP mode, but I guess I'm more with iwlwifi here in that it doesn't
> seem quite right for client mode?

I see your point. I need to experiment and see whether this way works or 
not for ath12k. Let me try that out.

> 
>> Also, how about non-ML scenario in iwlwifi? There, first station is
>> removed and then the interface goes down right?
> 
> It's not so much about the interface but the link, it seems.
> 

Sure..


-- 
Aditya