Skip to content

Commit 403cc24

Browse files
author
Jiang Jiang Jian
committed
Merge branch 'bugfix/skip_memory_reordering_wpa2_semaphr' into 'master'
fix(esp_wifi): fixed stack corruption in WiFi tasks Closes IDFGH-14617 See merge request espressif/esp-idf!36905
2 parents e3e49d7 + b5eadb5 commit 403cc24

File tree

4 files changed

+67
-105
lines changed

4 files changed

+67
-105
lines changed

‎components/wpa_supplicant/esp_supplicant/src/esp_dpp.c‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,11 @@ struct action_rx_param {
4242

4343
esp_err_t esp_dpp_post_evt(uint32_t evt_id, uint32_t data)
4444
{
45-
dpp_event_t *evt = os_zalloc(sizeof(dpp_event_t));
45+
dpp_event_t evt;
4646
esp_err_t ret = ESP_OK;
4747

48-
if (evt == NULL) {
49-
ret = ESP_ERR_NO_MEM;
50-
goto end;
51-
}
52-
evt->id = evt_id;
53-
evt->data = data;
48+
evt.id = evt_id;
49+
evt.data = data;
5450
if (s_dpp_api_lock) {
5551
DPP_API_LOCK();
5652
} else {
@@ -69,9 +65,6 @@ esp_err_t esp_dpp_post_evt(uint32_t evt_id, uint32_t data)
6965

7066
return ret;
7167
end:
72-
if (evt) {
73-
os_free(evt);
74-
}
7568
wpa_printf(MSG_ERROR, "DPP: Failed to send event %d to DPP task", evt_id);
7669
return ret;
7770
}
@@ -518,17 +511,16 @@ static esp_err_t esp_dpp_rx_action(struct action_rx_param *rx_param)
518511

519512
static void esp_dpp_task(void *pvParameters)
520513
{
521-
dpp_event_t *evt;
514+
dpp_event_t evt;
522515
bool task_del = false;
523516

524517
for (;;) {
525518
if (os_queue_recv(s_dpp_evt_queue, &evt, OS_BLOCK) == TRUE) {
526-
if (evt->id >= SIG_DPP_MAX) {
527-
os_free(evt);
519+
if (evt.id >= SIG_DPP_MAX) {
528520
continue;
529521
}
530522

531-
switch (evt->id) {
523+
switch (evt.id) {
532524
case SIG_DPP_DEL_TASK:
533525
struct dpp_bootstrap_params_t *params = &s_dpp_ctx.bootstrap_params;
534526
eloop_cancel_timeout(esp_dpp_auth_conf_wait_timeout, NULL, NULL);
@@ -549,7 +541,7 @@ static void esp_dpp_task(void *pvParameters)
549541
break;
550542

551543
case SIG_DPP_BOOTSTRAP_GEN: {
552-
char *command = (char *)evt->data;
544+
char *command = (char *)evt.data;
553545
const char *uri;
554546

555547
s_dpp_ctx.id = dpp_bootstrap_gen(s_dpp_ctx.dpp_global, command);
@@ -561,7 +553,7 @@ static void esp_dpp_task(void *pvParameters)
561553
break;
562554

563555
case SIG_DPP_RX_ACTION: {
564-
esp_dpp_rx_action((struct action_rx_param *)evt->data);
556+
esp_dpp_rx_action((struct action_rx_param *)evt.data);
565557
}
566558
break;
567559

@@ -588,7 +580,7 @@ static void esp_dpp_task(void *pvParameters)
588580
break;
589581

590582
case SIG_DPP_START_NET_INTRO: {
591-
esp_dpp_start_net_intro_protocol((uint8_t*)evt->data);
583+
esp_dpp_start_net_intro_protocol((uint8_t*)evt.data);
592584
}
593585
break;
594586

@@ -605,8 +597,6 @@ static void esp_dpp_task(void *pvParameters)
605597
break;
606598
}
607599

608-
os_free(evt);
609-
610600
if (task_del) {
611601
break;
612602
}

‎components/wpa_supplicant/esp_supplicant/src/esp_eap_client.c‎

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ static void wpa2_rxq_deinit(void)
196196

197197
void wpa2_task(void *pvParameters)
198198
{
199-
ETSEvent *e;
199+
ETSEvent e;
200200
struct eap_sm *sm = gEapSm;
201201
bool task_del = false;
202202

@@ -206,16 +206,16 @@ void wpa2_task(void *pvParameters)
206206

207207
for (;;) {
208208
if (TRUE == os_queue_recv(s_wpa2_queue, &e, OS_BLOCK)) {
209-
if (e->sig < SIG_WPA2_MAX) {
209+
if (e.sig < SIG_WPA2_MAX) {
210210
DATA_MUTEX_TAKE();
211-
if (sm->wpa2_sig_cnt[e->sig]) {
212-
sm->wpa2_sig_cnt[e->sig]--;
211+
if (sm->wpa2_sig_cnt[e.sig]) {
212+
sm->wpa2_sig_cnt[e.sig]--;
213213
} else {
214-
wpa_printf(MSG_ERROR, "wpa2_task: invalid sig cnt, sig=%" PRId32 " cnt=%d", e->sig, sm->wpa2_sig_cnt[e->sig]);
214+
wpa_printf(MSG_ERROR, "wpa2_task: invalid sig cnt, sig=%" PRId32 " cnt=%d", e.sig, sm->wpa2_sig_cnt[e.sig]);
215215
}
216216
DATA_MUTEX_GIVE();
217217
}
218-
switch (e->sig) {
218+
switch (e.sig) {
219219
case SIG_WPA2_TASK_DEL:
220220
task_del = true;
221221
break;
@@ -235,12 +235,9 @@ void wpa2_task(void *pvParameters)
235235
default:
236236
break;
237237
}
238-
os_free(e);
239-
}
240-
241-
if (task_del) {
242-
break;
243-
} else {
238+
if (task_del) {
239+
break;
240+
}
244241
if (s_wifi_wpa2_sync_sem) {
245242
wpa_printf(MSG_DEBUG, "EAP: wifi->EAP api completed");
246243
os_semphr_give(s_wifi_wpa2_sync_sem);
@@ -268,6 +265,7 @@ void wpa2_task(void *pvParameters)
268265
int wpa2_post(uint32_t sig, uint32_t par)
269266
{
270267
struct eap_sm *sm = gEapSm;
268+
ETSEvent evt;
271269

272270
if (!sm) {
273271
return ESP_FAIL;
@@ -277,28 +275,20 @@ int wpa2_post(uint32_t sig, uint32_t par)
277275
if (sm->wpa2_sig_cnt[sig]) {
278276
DATA_MUTEX_GIVE();
279277
return ESP_OK;
278+
}
279+
sm->wpa2_sig_cnt[sig]++;
280+
DATA_MUTEX_GIVE();
281+
evt.sig = sig;
282+
evt.par = par;
283+
if (os_queue_send(s_wpa2_queue, &evt, os_task_ms_to_tick(10)) != TRUE) {
284+
wpa_printf(MSG_ERROR, "EAP: Q S E");
285+
return ESP_FAIL;
286+
}
287+
if (s_wifi_wpa2_sync_sem) {
288+
os_semphr_take(s_wifi_wpa2_sync_sem, OS_BLOCK);
289+
wpa_printf(MSG_DEBUG, "EAP: EAP api return, sm->state(%d)", sm->finish_state);
280290
} else {
281-
ETSEvent *evt = (ETSEvent *)os_malloc(sizeof(ETSEvent));
282-
if (evt == NULL) {
283-
wpa_printf(MSG_ERROR, "EAP: E N M");
284-
DATA_MUTEX_GIVE();
285-
return ESP_FAIL;
286-
}
287-
sm->wpa2_sig_cnt[sig]++;
288-
DATA_MUTEX_GIVE();
289-
evt->sig = sig;
290-
evt->par = par;
291-
if (os_queue_send(s_wpa2_queue, &evt, os_task_ms_to_tick(10)) != TRUE) {
292-
wpa_printf(MSG_ERROR, "EAP: Q S E");
293-
return ESP_FAIL;
294-
} else {
295-
if (s_wifi_wpa2_sync_sem) {
296-
os_semphr_take(s_wifi_wpa2_sync_sem, OS_BLOCK);
297-
wpa_printf(MSG_DEBUG, "EAP: EAP api return, sm->state(%d)", sm->finish_state);
298-
} else {
299-
wpa_printf(MSG_ERROR, "EAP: null wifi->EAP sync sem");
300-
}
301-
}
291+
wpa_printf(MSG_ERROR, "EAP: null wifi->EAP sync sem");
302292
}
303293
return ESP_OK;
304294
}

‎components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -412,38 +412,32 @@ SemaphoreHandle_t g_wpa3_hostap_auth_api_lock = NULL;
412412

413413
int wpa3_hostap_post_evt(uint32_t evt_id, uint32_t data)
414414
{
415-
wpa3_hostap_auth_event_t *evt = os_zalloc(sizeof(wpa3_hostap_auth_event_t));
416-
if (evt == NULL) {
417-
return ESP_FAIL;
418-
}
419-
evt->id = evt_id;
420-
evt->data = data;
415+
wpa3_hostap_auth_event_t evt;
416+
417+
evt.id = evt_id;
418+
evt.data = data;
421419

422420
if (g_wpa3_hostap_auth_api_lock) {
423421
WPA3_HOSTAP_AUTH_API_LOCK();
424422
if (g_wpa3_hostap_evt_queue == NULL) {
425423
WPA3_HOSTAP_AUTH_API_UNLOCK();
426-
os_free(evt);
427424
wpa_printf(MSG_DEBUG, "hostap evt queue NULL");
428425
return ESP_FAIL;
429426
}
430427
} else {
431-
os_free(evt);
432428
wpa_printf(MSG_DEBUG, "g_wpa3_hostap_auth_api_lock not found");
433429
return ESP_FAIL;
434430
}
435-
if (evt->id == SIG_WPA3_RX_CONFIRM || evt->id == SIG_TASK_DEL) {
431+
if (evt.id == SIG_WPA3_RX_CONFIRM || evt.id == SIG_TASK_DEL) {
436432
/* prioritising confirm for completing handshake for committed sta */
437433
if (os_queue_send_to_front(g_wpa3_hostap_evt_queue, &evt, 0) != pdPASS) {
438434
WPA3_HOSTAP_AUTH_API_UNLOCK();
439435
wpa_printf(MSG_DEBUG, "failed to add msg to queue front");
440-
os_free(evt);
441436
return ESP_FAIL;
442437
}
443438
} else {
444439
if (os_queue_send(g_wpa3_hostap_evt_queue, &evt, 0) != pdPASS) {
445440
WPA3_HOSTAP_AUTH_API_UNLOCK();
446-
os_free(evt);
447441
wpa_printf(MSG_DEBUG, "failed to send msg to queue");
448442
return ESP_FAIL;
449443
}
@@ -562,18 +556,18 @@ static void wpa3_process_rx_confirm(wpa3_hostap_auth_event_t *evt)
562556

563557
static void esp_wpa3_hostap_task(void *pvParameters)
564558
{
565-
wpa3_hostap_auth_event_t *evt;
559+
wpa3_hostap_auth_event_t evt;
566560
bool task_del = false;
567561

568562
while (1) {
569563
if (os_queue_recv(g_wpa3_hostap_evt_queue, &evt, portMAX_DELAY) == pdTRUE) {
570-
switch (evt->id) {
564+
switch (evt.id) {
571565
case SIG_WPA3_RX_COMMIT: {
572-
wpa3_process_rx_commit(evt);
566+
wpa3_process_rx_commit(&evt);
573567
break;
574568
}
575569
case SIG_WPA3_RX_CONFIRM: {
576-
wpa3_process_rx_confirm(evt);
570+
wpa3_process_rx_confirm(&evt);
577571
break;
578572
}
579573
case SIG_TASK_DEL:
@@ -582,7 +576,6 @@ static void esp_wpa3_hostap_task(void *pvParameters)
582576
default:
583577
break;
584578
}
585-
os_free(evt);
586579

587580
if (task_del) {
588581
break;
@@ -593,10 +586,9 @@ static void esp_wpa3_hostap_task(void *pvParameters)
593586
while (items_in_queue--) {
594587
/* Free events posted to queue */
595588
os_queue_recv(g_wpa3_hostap_evt_queue, &evt, portMAX_DELAY);
596-
if (evt->id == SIG_WPA3_RX_CONFIRM) {
597-
os_free((void *)evt->data);
589+
if (evt.id == SIG_WPA3_RX_CONFIRM) {
590+
os_free((void *)evt.data);
598591
}
599-
os_free(evt);
600592
}
601593
os_queue_delete(g_wpa3_hostap_evt_queue);
602594
g_wpa3_hostap_evt_queue = NULL;

‎components/wpa_supplicant/esp_supplicant/src/esp_wps.c‎

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ static void wps_rxq_deinit(void)
119119
#ifdef USE_WPS_TASK
120120
void wps_task(void *pvParameters)
121121
{
122-
ETSEvent *e;
122+
ETSEvent e;
123123
wps_ioctl_param_t *param;
124124
bool del_task = false;
125125

@@ -129,32 +129,32 @@ void wps_task(void *pvParameters)
129129
for (;;) {
130130
if (TRUE == os_queue_recv(s_wps_queue, &e, OS_BLOCK)) {
131131

132-
if ((e->sig >= SIG_WPS_ENABLE) && (e->sig < SIG_WPS_NUM)) {
132+
if ((e.sig >= SIG_WPS_ENABLE) && (e.sig < SIG_WPS_NUM)) {
133133
DATA_MUTEX_TAKE();
134-
if (s_wps_sig_cnt[e->sig]) {
135-
s_wps_sig_cnt[e->sig]--;
134+
if (s_wps_sig_cnt[e.sig]) {
135+
s_wps_sig_cnt[e.sig]--;
136136
} else {
137-
wpa_printf(MSG_ERROR, "wpsT: invalid sig cnt, sig=%" PRId32 " cnt=%d", e->sig, s_wps_sig_cnt[e->sig]);
137+
wpa_printf(MSG_ERROR, "wpsT: invalid sig cnt, sig=%" PRId32 " cnt=%d", e.sig, s_wps_sig_cnt[e.sig]);
138138
}
139139
DATA_MUTEX_GIVE();
140140
}
141141

142-
wpa_printf(MSG_DEBUG, "wpsT: rx sig=%" PRId32 "", e->sig);
142+
wpa_printf(MSG_DEBUG, "wpsT: rx sig=%" PRId32 "", e.sig);
143143

144-
switch (e->sig) {
144+
switch (e.sig) {
145145
case SIG_WPS_ENABLE:
146146
case SIG_WPS_DISABLE:
147147
case SIG_WPS_START:
148-
param = (wps_ioctl_param_t *)e->par;
148+
param = (wps_ioctl_param_t *)e.par;
149149
if (!param) {
150-
wpa_printf(MSG_ERROR, "wpsT: invalid param sig=%" PRId32 "", e->sig);
150+
wpa_printf(MSG_ERROR, "wpsT: invalid param sig=%" PRId32 "", e.sig);
151151
os_semphr_give(s_wps_api_sem);
152152
break;
153153
}
154154

155-
if (e->sig == SIG_WPS_ENABLE) {
155+
if (e.sig == SIG_WPS_ENABLE) {
156156
param->ret = wifi_wps_enable_internal((esp_wps_config_t *)(param->arg));
157-
} else if (e->sig == SIG_WPS_DISABLE) {
157+
} else if (e.sig == SIG_WPS_DISABLE) {
158158
DATA_MUTEX_TAKE();
159159
param->ret = wifi_wps_disable_internal();
160160
del_task = true;
@@ -198,10 +198,9 @@ void wps_task(void *pvParameters)
198198
break;
199199

200200
default:
201-
wpa_printf(MSG_ERROR, "wpsT: invalid sig=%" PRId32 "", e->sig);
201+
wpa_printf(MSG_ERROR, "wpsT: invalid sig=%" PRId32 "", e.sig);
202202
break;
203203
}
204-
os_free(e);
205204

206205
if (del_task) {
207206
wpa_printf(MSG_DEBUG, "wpsT: delete task");
@@ -218,39 +217,30 @@ void wps_task(void *pvParameters)
218217
int wps_post(uint32_t sig, uint32_t par)
219218
{
220219
wpa_printf(MSG_DEBUG, "wps post: sig=%" PRId32 " cnt=%d", sig, s_wps_sig_cnt[sig]);
221-
222-
DATA_MUTEX_TAKE();
220+
ETSEvent evt;
223221

224222
if (!s_wps_task_hdl) {
225223
wpa_printf(MSG_DEBUG, "wps post: sig=%" PRId32 " failed as wps task has been deinited", sig);
226-
DATA_MUTEX_GIVE();
227224
return ESP_FAIL;
228225
}
226+
DATA_MUTEX_TAKE();
229227
if (s_wps_sig_cnt[sig]) {
230228
wpa_printf(MSG_DEBUG, "wps post: sig=%" PRId32 " processing", sig);
231229
DATA_MUTEX_GIVE();
232230
return ESP_OK;
233-
} else {
234-
ETSEvent *evt = (ETSEvent *)os_malloc(sizeof(ETSEvent));
231+
}
235232

236-
if (evt == NULL) {
237-
wpa_printf(MSG_ERROR, "WPS: E N M");
238-
DATA_MUTEX_GIVE();
239-
return ESP_FAIL;
240-
}
233+
s_wps_sig_cnt[sig]++;
234+
evt.sig = sig;
235+
evt.par = par;
236+
DATA_MUTEX_GIVE();
241237

242-
s_wps_sig_cnt[sig]++;
243-
evt->sig = sig;
244-
evt->par = par;
238+
if (os_queue_send(s_wps_queue, &evt, os_task_ms_to_tick(10)) != TRUE) {
239+
wpa_printf(MSG_ERROR, "WPS: Q S E");
240+
DATA_MUTEX_TAKE();
241+
s_wps_sig_cnt[sig]--;
245242
DATA_MUTEX_GIVE();
246-
247-
if (os_queue_send(s_wps_queue, &evt, os_task_ms_to_tick(10)) != TRUE) {
248-
wpa_printf(MSG_ERROR, "WPS: Q S E");
249-
DATA_MUTEX_TAKE();
250-
s_wps_sig_cnt[sig]--;
251-
DATA_MUTEX_GIVE();
252-
return ESP_FAIL;
253-
}
243+
return ESP_FAIL;
254244
}
255245
return ESP_OK;
256246
}

0 commit comments

Comments
 (0)