2022-05-19 22:04:37

by Johannes Berg

[permalink] [raw]
Subject: [PATCH v2 3/3] mac80211: refactor some key code

From: Johannes Berg <[email protected]>

There's some pretty close code here, with the exception
of RCU dereference vs. protected dereference. Refactor
this to just return a pointer and then do the deref in
the caller later.

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/cfg.c | 70 ++++++++++++++++++++++++----------------------
net/mac80211/key.h | 2 ++
2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index f3b10cee9299..5975889a338f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -533,33 +533,51 @@ static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
return err;
}

+static struct ieee80211_key *
+ieee80211_lookup_key(struct ieee80211_sub_if_data *sdata,
+ u8 key_idx, bool pairwise, const u8 *mac_addr)
+{
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta;
+
+ if (mac_addr) {
+ sta = sta_info_get_bss(sdata, mac_addr);
+ if (!sta)
+ return NULL;
+
+ if (pairwise && key_idx < NUM_DEFAULT_KEYS)
+ return rcu_dereference_check_key_mtx(local,
+ sta->ptk[key_idx]);
+
+ if (!pairwise &&
+ key_idx < NUM_DEFAULT_KEYS +
+ NUM_DEFAULT_MGMT_KEYS +
+ NUM_DEFAULT_BEACON_KEYS)
+ return rcu_dereference_check_key_mtx(local,
+ sta->deflink.gtk[key_idx]);
+
+ return NULL;
+ }
+
+ if (key_idx < NUM_DEFAULT_KEYS)
+ return rcu_dereference_check_key_mtx(local,
+ sdata->keys[key_idx]);
+
+ return NULL;
+}
+
static int ieee80211_del_key(struct wiphy *wiphy, struct net_device *dev,
u8 key_idx, bool pairwise, const u8 *mac_addr)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_local *local = sdata->local;
- struct sta_info *sta;
- struct ieee80211_key *key = NULL;
+ struct ieee80211_key *key;
int ret;

mutex_lock(&local->sta_mtx);
mutex_lock(&local->key_mtx);

- if (mac_addr) {
- ret = -ENOENT;
-
- sta = sta_info_get_bss(sdata, mac_addr);
- if (!sta)
- goto out_unlock;
-
- if (pairwise)
- key = key_mtx_dereference(local, sta->ptk[key_idx]);
- else
- key = key_mtx_dereference(local,
- sta->deflink.gtk[key_idx]);
- } else
- key = key_mtx_dereference(local, sdata->keys[key_idx]);
-
+ key = ieee80211_lookup_key(sdata, key_idx, pairwise, mac_addr);
if (!key) {
ret = -ENOENT;
goto out_unlock;
@@ -582,10 +600,9 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct net_device *dev,
struct key_params *params))
{
struct ieee80211_sub_if_data *sdata;
- struct sta_info *sta = NULL;
u8 seq[6] = {0};
struct key_params params;
- struct ieee80211_key *key = NULL;
+ struct ieee80211_key *key;
u64 pn64;
u32 iv32;
u16 iv16;
@@ -596,20 +613,7 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct net_device *dev,

rcu_read_lock();

- if (mac_addr) {
- sta = sta_info_get_bss(sdata, mac_addr);
- if (!sta)
- goto out;
-
- if (pairwise && key_idx < NUM_DEFAULT_KEYS)
- key = rcu_dereference(sta->ptk[key_idx]);
- else if (!pairwise &&
- key_idx < NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS +
- NUM_DEFAULT_BEACON_KEYS)
- key = rcu_dereference(sta->deflink.gtk[key_idx]);
- } else
- key = rcu_dereference(sdata->keys[key_idx]);
-
+ key = ieee80211_lookup_key(sdata, key_idx, pairwise, mac_addr);
if (!key)
goto out;

diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index b3fb41c0c77f..e994dcea1ce3 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -163,6 +163,8 @@ void ieee80211_reenable_keys(struct ieee80211_sub_if_data *sdata);

#define key_mtx_dereference(local, ref) \
rcu_dereference_protected(ref, lockdep_is_held(&((local)->key_mtx)))
+#define rcu_dereference_check_key_mtx(local, ref) \
+ rcu_dereference_check(ref, lockdep_is_held(&((local)->key_mtx)))

void ieee80211_delayed_tailroom_dec(struct work_struct *wk);

--
2.36.1



2022-05-26 09:15:50

by kernel test robot

[permalink] [raw]
Subject: [mac80211] 13d5c8ebd3: hwsim.dpp_pfs_errors.fail



Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: 13d5c8ebd318e7c7258a675a44068d3a750d801f ("[PATCH v2 3/3] mac80211: refactor some key code")
url: https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/mac80211_hwsim-call-ieee80211_tx_prepare_skb-under-RCU-protection/20220520-052940
base: https://git.kernel.org/cgit/linux/kernel/git/wireless/wireless-next.git main
patch link: https://lore.kernel.org/linux-wireless/20220519232721.6febb5d5b82b.I3e8b3aca8873a59da7b63e56d4349c652afc815c@changeid

in testcase: hwsim
version: hwsim-x86_64-717e5d7-1_20220411
with following parameters:

test: group-07
ucode: 0x21



on test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz with 16G memory

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):




If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


please be noted, besides test dpp_pfs_errors, we also found
test wpa2_ocv_sta_override_sa_query_req failed upon this commit but pass on
parent.


2022-05-25 14:27:44 ./run-tests.py dpp_pfs_errors
DEV: wlan0: 02:00:00:00:00:00
DEV: wlan1: 02:00:00:00:01:00
DEV: wlan2: 02:00:00:00:02:00
APDEV: wlan3
APDEV: wlan4
START dpp_pfs_errors 1/1
Test: DPP PFS error cases
Starting AP wlan3
Connect STA wlan0 to AP
Connection timed out
Traceback (most recent call last):
File "./run-tests.py", line 533, in main
t(dev, apdev)
File "/lkp/benchmarks/hwsim/tests/hwsim/test_dpp.py", line 6553, in test_dpp_pfs_errors
dpp_netaccesskey=params1_sta_netaccesskey)
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 1140, in connect
self.connect_network(id)
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 500, in connect_network
self.wait_connected(timeout=timeout)
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 1411, in wait_connected
raise Exception(error)
Exception: Connection timed out
FAIL dpp_pfs_errors 10.14996 2022-05-25 14:27:54.649706
passed 0 test case(s)
skipped 0 test case(s)
failed tests: dpp_pfs_errors

...

2022-05-25 14:28:40 ./run-tests.py wpa2_ocv_sta_override_sa_query_req
DEV: wlan0: 02:00:00:00:00:00
DEV: wlan1: 02:00:00:00:01:00
DEV: wlan2: 02:00:00:00:02:00
APDEV: wlan3
APDEV: wlan4
START wpa2_ocv_sta_override_sa_query_req 1/1
Test: OCV on 2.4 GHz and STA override SA Query Request
Starting AP wlan3
Connect STA wlan0 to AP
Connection timed out
Traceback (most recent call last):
File "./run-tests.py", line 533, in main
t(dev, apdev)
File "/lkp/benchmarks/hwsim/tests/hwsim/test_ocv.py", line 943, in test_wpa2_ocv_sta_override_sa_query_req
ieee80211w="2")
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 1140, in connect
self.connect_network(id)
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 500, in connect_network
self.wait_connected(timeout=timeout)
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 1411, in wait_connected
raise Exception(error)
Exception: Connection timed out
FAIL wpa2_ocv_sta_override_sa_query_req 10.129819 2022-05-25 14:28:50.410355
passed 0 test case(s)
skipped 0 test case(s)
failed tests: wpa2_ocv_sta_override_sa_query_req



To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



--
0-DAY CI Kernel Test Service
https://01.org/lkp



Attachments:
(No filename) (3.79 kB)
config-5.18.0-rc4-01383-g13d5c8ebd318 (168.63 kB)
job-script (5.93 kB)
dmesg.xz (81.99 kB)
hwsim (57.93 kB)
job.yaml (4.67 kB)
reproduce (3.51 kB)
Download all attachments