Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <8a5845b49b6dc03b8d6f8fe9915034178be992ae.camel@sipsolutions.net>
Subject: Re: [PATCH] net: wireless: search and hold bss in
 cfg80211_connect_done
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Abhishek Kumar <kuabhs@chromium.org>
Cc:     netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org,
        briannorris@chromium.org, linux-wireless@vger.kernel.org
Date:   Tue, 16 Mar 2021 21:32:24 +0100
In-Reply-To: <20210316192919.1.I26d48d8a4d06ef9bd2b57f857c58ae681cc33783@changeid> (sfid-20210316_203101_193722_2D56E503)
References: <20210316192919.1.I26d48d8a4d06ef9bd2b57f857c58ae681cc33783@changeid>
         (sfid-20210316_203101_193722_2D56E503)
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2021-03-16 at 19:29 +0000, Abhishek Kumar wrote:
> If BSS instance is not provided in __cfg80211_connect_result then
> a get bss is performed. This can return NULL if the BSS for the
> given SSID is expired due to delayed scheduling of connect result event
> in rdev->event_work. This can cause WARN_ON(!cr->bss) in
> __cfg80211_connect_result to be triggered and cause cascading
> failures. To mitigate this, initiate a get bss call in
> cfg80211_connect_done itself and hold it to ensure that the BSS
> instance does not get expired.

I'm not sure I see the value in this.

You're basically picking a slightly earlier point in time where cfg80211
might know about the BSS entry still, so you're really just making the
problem window a few microseconds or perhaps milliseconds (whatever ends
up being the worker delay) shorter.

Compared to the 30s entry lifetime, that's nothing.

So what's the point? Please fix the driver instead to actually hold on
to it and report it back.

johannes