Skip to content

Commit f0f5b08

Browse files
author
Jiang Jiang Jian
committed
Merge branch 'bugfix/skip_memory_reordering_wpa2_semaphr_v5.1' into 'release/v5.1'
fix(esp_wifi): fixed stack corruption in WiFi tasks (v5.1) See merge request espressif/esp-idf!36939
2 parents e79c1ea + 29e8dc6 commit f0f5b08

File tree

4 files changed

+68
-106
lines changed

4 files changed

+68
-106
lines changed

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

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,11 @@ struct action_rx_param {
4040

4141
static int esp_dpp_post_evt(uint32_t evt_id, uint32_t data)
4242
{
43-
dpp_event_t *evt = os_zalloc(sizeof(dpp_event_t));
44-
int ret = ESP_OK;
43+
dpp_event_t evt;
44+
esp_err_t ret = ESP_OK;
4545

46-
if (evt == NULL) {
47-
ret = ESP_ERR_NO_MEM;
48-
goto end;
49-
}
50-
evt->id = evt_id;
51-
evt->data = data;
46+
evt.id = evt_id;
47+
evt.data = data;
5248
if (s_dpp_api_lock) {
5349
DPP_API_LOCK();
5450
} else {
@@ -67,10 +63,7 @@ static int esp_dpp_post_evt(uint32_t evt_id, uint32_t data)
6763

6864
return ret;
6965
end:
70-
if (evt) {
71-
os_free(evt);
72-
}
73-
wpa_printf(MSG_ERROR,"DPP: Failed to send event %d to DPP task", evt_id);
66+
wpa_printf(MSG_ERROR, "DPP: Failed to send event %d to DPP task", evt_id);
7467
return ret;
7568
}
7669

@@ -381,17 +374,16 @@ static void esp_dpp_rx_action(struct action_rx_param *rx_param)
381374

382375
static void esp_dpp_task(void *pvParameters )
383376
{
384-
dpp_event_t *evt;
377+
dpp_event_t evt;
385378
bool task_del = false;
386379

387380
for (;;) {
388381
if (os_queue_recv(s_dpp_evt_queue, &evt, OS_BLOCK) == TRUE) {
389-
if (evt->id >= SIG_DPP_MAX) {
390-
os_free(evt);
382+
if (evt.id >= SIG_DPP_MAX) {
391383
continue;
392384
}
393385

394-
switch (evt->id) {
386+
switch (evt.id) {
395387
case SIG_DPP_DEL_TASK:
396388
struct dpp_bootstrap_params_t *params = &s_dpp_ctx.bootstrap_params;
397389
eloop_cancel_timeout(esp_dpp_auth_conf_wait_timeout, NULL, NULL);
@@ -412,7 +404,7 @@ static void esp_dpp_task(void *pvParameters )
412404
break;
413405

414406
case SIG_DPP_BOOTSTRAP_GEN: {
415-
char *command = (char *)evt->data;
407+
char *command = (char *)evt.data;
416408
const char *uri;
417409

418410
s_dpp_ctx.id = dpp_bootstrap_gen(s_dpp_ctx.dpp_global, command);
@@ -424,7 +416,7 @@ static void esp_dpp_task(void *pvParameters )
424416
break;
425417

426418
case SIG_DPP_RX_ACTION: {
427-
esp_dpp_rx_action((struct action_rx_param *)evt->data);
419+
esp_dpp_rx_action((struct action_rx_param *)evt.data);
428420
}
429421
break;
430422

@@ -462,8 +454,6 @@ static void esp_dpp_task(void *pvParameters )
462454
break;
463455
}
464456

465-
os_free(evt);
466-
467457
if (task_del) {
468458
break;
469459
}

‎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
@@ -403,38 +403,32 @@ SemaphoreHandle_t g_wpa3_hostap_auth_api_lock = NULL;
403403

404404
int wpa3_hostap_post_evt(uint32_t evt_id, uint32_t data)
405405
{
406-
wpa3_hostap_auth_event_t *evt = os_zalloc(sizeof(wpa3_hostap_auth_event_t));
407-
if (evt == NULL) {
408-
return ESP_FAIL;
409-
}
410-
evt->id = evt_id;
411-
evt->data = data;
406+
wpa3_hostap_auth_event_t evt;
407+
408+
evt.id = evt_id;
409+
evt.data = data;
412410

413411
if (g_wpa3_hostap_auth_api_lock) {
414412
WPA3_HOSTAP_AUTH_API_LOCK();
415413
if (g_wpa3_hostap_evt_queue == NULL) {
416414
WPA3_HOSTAP_AUTH_API_UNLOCK();
417-
os_free(evt);
418415
wpa_printf(MSG_DEBUG, "hostap evt queue NULL");
419416
return ESP_FAIL;
420417
}
421418
} else {
422-
os_free(evt);
423419
wpa_printf(MSG_DEBUG, "g_wpa3_hostap_auth_api_lock not found");
424420
return ESP_FAIL;
425421
}
426-
if (evt->id == SIG_WPA3_RX_CONFIRM || evt->id == SIG_TASK_DEL) {
422+
if (evt.id == SIG_WPA3_RX_CONFIRM || evt.id == SIG_TASK_DEL) {
427423
/* prioritising confirm for completing handshake for committed sta */
428424
if (os_queue_send_to_front(g_wpa3_hostap_evt_queue, &evt, 0) != pdPASS) {
429425
WPA3_HOSTAP_AUTH_API_UNLOCK();
430426
wpa_printf(MSG_DEBUG, "failed to add msg to queue front");
431-
os_free(evt);
432427
return ESP_FAIL;
433428
}
434429
} else {
435430
if (os_queue_send(g_wpa3_hostap_evt_queue, &evt, 0) != pdPASS) {
436431
WPA3_HOSTAP_AUTH_API_UNLOCK();
437-
os_free(evt);
438432
wpa_printf(MSG_DEBUG, "failed to send msg to queue");
439433
return ESP_FAIL;
440434
}
@@ -543,18 +537,18 @@ static void wpa3_process_rx_confirm(wpa3_hostap_auth_event_t *evt)
543537

544538
static void esp_wpa3_hostap_task(void *pvParameters)
545539
{
546-
wpa3_hostap_auth_event_t *evt;
540+
wpa3_hostap_auth_event_t evt;
547541
bool task_del = false;
548542

549543
while (1) {
550544
if (os_queue_recv(g_wpa3_hostap_evt_queue, &evt, portMAX_DELAY) == pdTRUE) {
551-
switch (evt->id) {
545+
switch (evt.id) {
552546
case SIG_WPA3_RX_COMMIT: {
553-
wpa3_process_rx_commit(evt);
547+
wpa3_process_rx_commit(&evt);
554548
break;
555549
}
556550
case SIG_WPA3_RX_CONFIRM: {
557-
wpa3_process_rx_confirm(evt);
551+
wpa3_process_rx_confirm(&evt);
558552
break;
559553
}
560554
case SIG_TASK_DEL:
@@ -563,7 +557,6 @@ static void esp_wpa3_hostap_task(void *pvParameters)
563557
default:
564558
break;
565559
}
566-
os_free(evt);
567560

568561
if (task_del) {
569562
break;
@@ -574,10 +567,9 @@ static void esp_wpa3_hostap_task(void *pvParameters)
574567
while(items_in_queue--) {
575568
/* Free events posted to queue */
576569
os_queue_recv(g_wpa3_hostap_evt_queue, &evt, portMAX_DELAY);
577-
if (evt->id == SIG_WPA3_RX_CONFIRM) {
578-
os_free((void *)evt->data);
570+
if (evt.id == SIG_WPA3_RX_CONFIRM) {
571+
os_free((void *)evt.data);
579572
}
580-
os_free(evt);
581573
}
582574
os_queue_delete(g_wpa3_hostap_evt_queue);
583575
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)