From: Aloka Dixit <alokad@codeaurora.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [v13 2/3] mac80211: MBSSID and EMA beacon handling in AP mode
Date: Fri, 14 Jan 2022 11:23:23 -0800 [thread overview]
Message-ID: <d2c980b72af1488282f18e8b1814b56c@codeaurora.org> (raw)
In-Reply-To: <16a03353cee422340c8ac36240b1e088fd45802e.camel@sipsolutions.net>
On 2021-11-26 03:23, Johannes Berg wrote:
> On Tue, 2021-10-05 at 21:09 -0700, Aloka Dixit wrote:
>
>>
>> +static struct sk_buff *
>> +ieee80211_beacon_get_ap_mbssid(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + struct ieee80211_mutable_offsets *offs,
>> + bool is_template,
>> + struct beacon_data *beacon,
>> + struct ieee80211_chanctx_conf *chanctx_conf,
>> + int ema_index,
>> + struct list_head *ema_list)
>
> This function is called from ieee80211_beacon_get_ap(). That's called
> from __ieee80211_beacon_get(), under RCU read lock.
>
>> + for (i = 0; i < beacon->mbssid_ies->cnt; i++) {
>> + struct ieee80211_ema_bcns *bcn;
>> +
>> + bcn = kzalloc(sizeof(*bcn), GFP_KERNEL);
>
> Therefore, you really cannot GFP_KERNEL allocate anything. But I really
> only saw this because I went back to my comments on v12 where this was
> still more obvious.
>
Okay, I understand now that it is illegal because GFP_KERNEL is
blocking.
I thought of following:
lock rcu -> get mbssid count first -> unlock rcu -> allocate memory.
But in that case, will have again: lock -> dereference to get beacon
snapshot.
Beacon can change in between so new count might be wrong. In general
sounds complicated and wrong.
I read that GFP_ATOMIC should be used sparingly, mainly for interrupt
handlers etc.
Do you think this code path warrants its use?
Or should I look for some other function split?
Will add hwsim test cases before the next version but I genuinely did
not see any issue during testing with current code.
So can you tell me which debug flags should be enabled to make such
errors become obvious to someone like me who is new to these details in
kernel programming?
Thanks!!
next prev parent reply other threads:[~2022-01-14 19:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-06 4:09 [v13 0/3] MBSSID and EMA support in AP mode Aloka Dixit
2021-10-06 4:09 ` [v13 1/3] mac80211: split beacon retrieval functions Aloka Dixit
2021-10-06 20:20 ` Aloka Dixit
2021-10-06 4:09 ` [v13 2/3] mac80211: MBSSID and EMA beacon handling in AP mode Aloka Dixit
2021-11-26 11:23 ` Johannes Berg
2022-01-14 19:23 ` Aloka Dixit [this message]
2022-01-14 20:12 ` Johannes Berg
2022-01-14 20:34 ` Aloka Dixit
2022-01-14 20:50 ` Johannes Berg
2021-10-06 4:09 ` [v13 3/3] mac80211: MBSSID channel switch Aloka Dixit
2021-11-26 11:16 ` Johannes Berg
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=d2c980b72af1488282f18e8b1814b56c@codeaurora.org \
--to=alokad@codeaurora.org \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.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 read-only IMAP folder(s) and NNTP newsgroup(s).