Skip to content

Commit 4fb3ef3

Browse files
krish2718rlubos
krish2718
authored and
rlubos
committed
drivers: wifi: Add VIF locking to public APIs
To fix crashes during interface de-init, add a lock + NULL check before de-referencing FMAC handle and invoking FMAC APIs. As this is expected during de-init only debug logs are used. Also, fix the NULL checks to return early before the mutex lock to keep the code clean (And add missing NULL checks too). Ideally a locking mechanism like RCU is perfect for this and low overhead, but we don't have that in Zephyr. Fixes SHEL-2436. Signed-off-by: Chaitanya Tata <[email protected]>
1 parent 9627faf commit 4fb3ef3

File tree

4 files changed

+460
-53
lines changed

4 files changed

+460
-53
lines changed

‎drivers/wifi/nrf700x/src/net_if.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,7 @@ int nrf_wifi_if_send(const struct device *dev,
199199
}
200200

201201
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
202-
203-
if (!rpu_ctx_zep->rpu_ctx) {
202+
if (!rpu_ctx_zep || !rpu_ctx_zep->rpu_ctx) {
204203
goto unlock;
205204
}
206205

‎drivers/wifi/nrf700x/src/wifi_mgmt.c‎

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,26 @@ int nrf_wifi_set_power_save(const struct device *dev,
3636

3737
if (!dev || !params) {
3838
LOG_ERR("%s: dev or params is NULL", __func__);
39-
goto out;
39+
return ret;
4040
}
4141

4242
vif_ctx_zep = dev->data;
4343

4444
if (!vif_ctx_zep) {
4545
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
46-
goto out;
46+
return ret;
4747
}
4848

4949
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
5050

5151
if (!rpu_ctx_zep) {
5252
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
53+
return ret;
54+
}
55+
56+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
57+
if (!rpu_ctx_zep->rpu_ctx) {
58+
LOG_DBG("%s: RPU context not initialized", __func__);
5359
goto out;
5460
}
5561

@@ -123,6 +129,7 @@ int nrf_wifi_set_power_save(const struct device *dev,
123129

124130
ret = 0;
125131
out:
132+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
126133
return ret;
127134
}
128135

@@ -137,14 +144,14 @@ int nrf_wifi_get_power_save_config(const struct device *dev,
137144
int count = 0;
138145

139146
if (!dev || !ps_config) {
140-
goto out;
147+
return ret;
141148
}
142149

143150
vif_ctx_zep = dev->data;
144151

145152
if (!vif_ctx_zep) {
146153
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
147-
goto out;
154+
return ret;
148155
}
149156

150157
if ((vif_ctx_zep->if_type != NRF_WIFI_IFTYPE_STATION)
@@ -154,10 +161,21 @@ int nrf_wifi_get_power_save_config(const struct device *dev,
154161
) {
155162
LOG_ERR("%s: Operation supported only in STA enabled mode",
156163
__func__);
157-
goto out;
164+
return ret;
158165
}
159166

160167
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
168+
if (!rpu_ctx_zep) {
169+
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
170+
return ret;
171+
}
172+
173+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
174+
if (!rpu_ctx_zep->rpu_ctx) {
175+
LOG_DBG("%s: RPU context not initialized", __func__);
176+
goto out;
177+
}
178+
161179
fmac_dev_ctx = rpu_ctx_zep->rpu_ctx;
162180

163181
if (!rpu_ctx_zep) {
@@ -194,6 +212,7 @@ int nrf_wifi_get_power_save_config(const struct device *dev,
194212

195213
ret = 0;
196214
out:
215+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
197216
return ret;
198217
}
199218

@@ -401,15 +420,19 @@ int nrf_wifi_twt_teardown_flows(struct nrf_wifi_vif_ctx_zep *vif_ctx_zep,
401420

402421
if (!vif_ctx_zep) {
403422
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
404-
ret = -1;
405-
goto out;
423+
return ret;
406424
}
407425

408426
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
409427

410428
if (!rpu_ctx_zep) {
411429
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
412-
ret = -1;
430+
return ret;
431+
}
432+
433+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
434+
if (!rpu_ctx_zep->rpu_ctx) {
435+
LOG_DBG("%s: RPU context not initialized", __func__);
413436
goto out;
414437
}
415438

@@ -437,6 +460,7 @@ int nrf_wifi_twt_teardown_flows(struct nrf_wifi_vif_ctx_zep *vif_ctx_zep,
437460
}
438461

439462
out:
463+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
440464
return ret;
441465
}
442466

@@ -450,20 +474,27 @@ int nrf_wifi_set_twt(const struct device *dev,
450474
int ret = -1;
451475

452476
if (!dev || !twt_params) {
453-
goto out;
477+
LOG_ERR("%s: dev or twt_params is NULL", __func__);
478+
return ret;
454479
}
455480

456481
vif_ctx_zep = dev->data;
457482

458483
if (!vif_ctx_zep) {
459484
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
460-
goto out;
485+
return ret;
461486
}
462487

463488
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
464489

465490
if (!rpu_ctx_zep) {
466491
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
492+
return ret;
493+
}
494+
495+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
496+
if (!rpu_ctx_zep->rpu_ctx) {
497+
LOG_DBG("%s: RPU context not initialized", __func__);
467498
goto out;
468499
}
469500

@@ -553,6 +584,7 @@ int nrf_wifi_set_twt(const struct device *dev,
553584

554585
ret = 0;
555586
out:
587+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
556588
return ret;
557589
}
558590

@@ -642,6 +674,12 @@ void nrf_wifi_event_proc_twt_sleep_zep(void *vif_ctx,
642674
return;
643675
}
644676

677+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
678+
if (!rpu_ctx_zep->rpu_ctx) {
679+
LOG_DBG("%s: RPU context not initialized", __func__);
680+
goto out;
681+
}
682+
645683
fmac_dev_ctx = rpu_ctx_zep->rpu_ctx;
646684
def_dev_ctx = wifi_dev_priv(fmac_dev_ctx);
647685
def_priv = wifi_fmac_priv(fmac_dev_ctx->fpriv);
@@ -684,6 +722,8 @@ void nrf_wifi_event_proc_twt_sleep_zep(void *vif_ctx,
684722
default:
685723
break;
686724
}
725+
out:
726+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
687727
}
688728

689729
#ifdef CONFIG_NRF700X_SYSTEM_MODE
@@ -699,16 +739,27 @@ int nrf_wifi_mode(const struct device *dev,
699739

700740
if (!dev || !mode) {
701741
LOG_ERR("%s: illegal input parameters", __func__);
702-
goto out;
742+
return ret;
703743
}
704744

705745
vif_ctx_zep = dev->data;
706746
if (!vif_ctx_zep) {
707747
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
708-
goto out;
748+
return ret;
709749
}
710750

711751
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
752+
if (!rpu_ctx_zep) {
753+
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
754+
return ret;
755+
}
756+
757+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
758+
if (!rpu_ctx_zep->rpu_ctx) {
759+
LOG_DBG("%s: RPU context not initialized", __func__);
760+
goto out;
761+
}
762+
712763
fmac_dev_ctx = rpu_ctx_zep->rpu_ctx;
713764
def_dev_ctx = wifi_dev_priv(fmac_dev_ctx);
714765

@@ -743,6 +794,7 @@ int nrf_wifi_mode(const struct device *dev,
743794
}
744795
ret = 0;
745796
out:
797+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
746798
return ret;
747799
}
748800
#endif
@@ -760,21 +812,32 @@ int nrf_wifi_channel(const struct device *dev,
760812

761813
if (!dev || !channel) {
762814
LOG_ERR("%s: illegal input parameters", __func__);
763-
goto out;
815+
return ret;
764816
}
765817

766818
vif_ctx_zep = dev->data;
767819
if (!vif_ctx_zep) {
768820
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
769-
goto out;
821+
return ret;
770822
}
771823

772824
if (vif_ctx_zep->authorized) {
773825
LOG_ERR("%s: Cannot change channel when in station connected mode", __func__);
774-
goto out;
826+
return ret;
775827
}
776828

777829
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
830+
if (!rpu_ctx_zep) {
831+
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
832+
return ret;
833+
}
834+
835+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
836+
if (!rpu_ctx_zep->rpu_ctx) {
837+
LOG_DBG("%s: RPU context not initialized", __func__);
838+
goto out;
839+
}
840+
778841
fmac_dev_ctx = rpu_ctx_zep->rpu_ctx;
779842
def_dev_ctx = wifi_dev_priv(fmac_dev_ctx);
780843

@@ -797,6 +860,7 @@ int nrf_wifi_channel(const struct device *dev,
797860
}
798861
ret = 0;
799862
out:
863+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
800864
return ret;
801865
}
802866
#endif /* CONFIG_NRF700X_RAW_DATA_TX */

‎drivers/wifi/nrf700x/src/wifi_mgmt_scan.c‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,23 @@ int nrf_wifi_disp_scan_zep(const struct device *dev, struct wifi_scan_params *pa
5656

5757
if (!vif_ctx_zep) {
5858
LOG_ERR("%s: vif_ctx_zep is NULL", __func__);
59-
goto out;
59+
return ret;
6060
}
6161

6262
if (vif_ctx_zep->if_op_state != NRF_WIFI_FMAC_IF_OP_STATE_UP) {
6363
LOG_ERR("%s: Interface not UP", __func__);
64-
goto out;
64+
return ret;
6565
}
6666

6767
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
6868
if (!rpu_ctx_zep) {
6969
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
70+
return ret;
71+
}
72+
73+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
74+
if (!rpu_ctx_zep->rpu_ctx) {
75+
LOG_DBG("%s: RPU context not initialized", __func__);
7076
goto out;
7177
}
7278

@@ -197,7 +203,7 @@ int nrf_wifi_disp_scan_zep(const struct device *dev, struct wifi_scan_params *pa
197203
if (scan_info) {
198204
k_free(scan_info);
199205
}
200-
206+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
201207
return ret;
202208
}
203209

@@ -207,6 +213,16 @@ enum nrf_wifi_status nrf_wifi_disp_scan_res_get_zep(struct nrf_wifi_vif_ctx_zep
207213
struct nrf_wifi_ctx_zep *rpu_ctx_zep = NULL;
208214

209215
rpu_ctx_zep = vif_ctx_zep->rpu_ctx_zep;
216+
if (!rpu_ctx_zep) {
217+
LOG_ERR("%s: rpu_ctx_zep is NULL", __func__);
218+
return NRF_WIFI_STATUS_FAIL;
219+
}
220+
221+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
222+
if (!rpu_ctx_zep->rpu_ctx) {
223+
LOG_DBG("%s: RPU context not initialized", __func__);
224+
goto out;
225+
}
210226

211227
status = nrf_wifi_fmac_scan_res_get(rpu_ctx_zep->rpu_ctx,
212228
vif_ctx_zep->vif_idx,
@@ -219,6 +235,7 @@ enum nrf_wifi_status nrf_wifi_disp_scan_res_get_zep(struct nrf_wifi_vif_ctx_zep
219235

220236
status = NRF_WIFI_STATUS_SUCCESS;
221237
out:
238+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
222239
return status;
223240
}
224241

@@ -368,6 +385,12 @@ void nrf_wifi_rx_bcn_prb_resp_frm(void *vif_ctx,
368385
return;
369386
}
370387

388+
k_mutex_lock(&vif_ctx_zep->vif_lock, K_FOREVER);
389+
if (!rpu_ctx_zep->rpu_ctx) {
390+
LOG_DBG("%s: RPU context not initialized", __func__);
391+
goto out;
392+
}
393+
371394
fmac_dev_ctx = rpu_ctx_zep->rpu_ctx;
372395

373396
frame_length = nrf_wifi_osal_nbuf_data_size(fmac_dev_ctx->fpriv->opriv,
@@ -397,5 +420,7 @@ void nrf_wifi_rx_bcn_prb_resp_frm(void *vif_ctx,
397420
wifi_mgmt_raise_raw_scan_result_event(vif_ctx_zep->zep_net_if_ctx,
398421
&bcn_prb_resp_info);
399422

423+
out:
424+
k_mutex_unlock(&vif_ctx_zep->vif_lock);
400425
}
401426
#endif /* CONFIG_WIFI_MGMT_RAW_SCAN_RESULTS */

0 commit comments

Comments
 (0)