Message-ID: <e870c4f0c0c04b3e4c95252b2d4722bf5dcba064.camel@sipsolutions.net>
Subject: Re: [PATCH v4] nl80211: reset regdom when reloading regdb
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Finn Behrens <me@kloenk.dev>
Cc:     linux-wireless@vger.kernel.org
Date:   Fri, 26 Nov 2021 13:08:49 +0100
In-Reply-To: <20210526112418.cdwkn7ed4twctimc@imap.mailbox.org>
References: <20210526112418.cdwkn7ed4twctimc@imap.mailbox.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.1 (3.42.1-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

> Sorry for taking so long for a new version. The mail provider my company
> is using had some major problems connecting to the LKML.
> This is why I now are using my personal mail server.


Did you resend this? Did I lose it before? Or maybe the list lost it and
I had a copy? Confusing ...

But anyway:

> @@ -1113,6 +1116,26 @@ int reg_reload_regdb(void)
>  	regdb = db;
>  	rtnl_unlock();
>  
> +	/* reset regulatory domain */
> +	current_regdomain = get_cfg80211_regdom();


This really is just

	rcu_dereference_rtnl(cfg80211_regdomain);

so it cannot be right that you just unlocked the RTNL and are not under
RCU protection here?

I guess we can just keep the RTNL, since ...


> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
> +	if (!request) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	request->wiphy_idx = WIPHY_IDX_INVALID;
> +	request->alpha2[0] = current_regdomain->alpha2[0];
> +	request->alpha2[1] = current_regdomain->alpha2[1];
> +	request->initiator = NL80211_USER_REG_HINT_USER;
> +	request->user_reg_hint_type = NL80211_USER_REG_HINT_USER;
> +	request->reload = true;
> +
> +	rtnl_lock();

we acquire it again here anyway? Just need a new

> +	reg_process_hint(request);

out_unlock:
> +	rtnl_unlock();

label there.

> 
> @@ -2683,7 +2706,8 @@ reg_process_hint_user(struct regulatory_request *user_request)
>  
>  	treatment = __reg_process_hint_user(user_request);
>  	if (treatment == REG_REQ_IGNORE ||
> -	    treatment == REG_REQ_ALREADY_SET)
> +	    (treatment == REG_REQ_ALREADY_SET &&
> +				!user_request->reload))
>  		return REG_REQ_IGNORE;

That could use some nicer indentation :)

johannes