Event group is used after vEventGroupDelete() when WiFi drops in station mode

[ft972]

Environment

• ESP-IDF v5.5.4, target esp32s3
• Board: Waveshare ESP32-S3-Touch-LCD-1.9
• App version 2.0.3.2
• CONFIG_ITEM_WIFI_MODE = station

Summary

In station mode, s_wifi_event_group is deleted at the end of wifi_init_sta()
but the handle is left dangling and the WiFi event handler stays registered. If
the connection is later lost and the retries are exhausted, the handler writes
to freed heap.

The path

wifi_init_sta() blocks until the connection either succeeds or fails, then
deletes the event group:

EventBits_t bits = xEventGroupWaitBits(s_wifi_event_group,
                                       WIFI_CONNECTED_BIT | WIFI_FAIL_BIT,
                                       pdFALSE, pdFALSE, portMAX_DELAY);
...
vEventGroupDelete(s_wifi_event_group);   // handle not cleared

vEventGroupDelete() really does release the memory — event_groups.c:693
calls vPortFree( pxEventBits ).

The event handler registered in the same function is never unregistered, and it
still refers to that handle:

else if ((event_base == WIFI_EVENT) && (event_id == WIFI_EVENT_STA_DISCONNECTED))
{
    if (s_retry_num < WIFI_STA_MAXIMUM_RETRY)
    {
        esp_wifi_connect();
        s_retry_num++;
    }
    else
    {
        xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT);   // freed memory
    }
    ...
}

So the sequence is:

1. Station connects. IP_EVENT_STA_GOT_IP resets s_retry_num to 0,
   wifi_init_sta() returns and deletes the event group.
2. Hours later the access point goes away. WIFI_EVENT_STA_DISCONNECTED fires,
   retries 1..5 run and call esp_wifi_connect() — fine so far.
3. The AP stays away, so the sixth disconnect takes the else branch and calls
   xEventGroupSetBits() on a freed block.

Why this is worse than a plain stale read

xEventGroupSetBits() does not just read the block. It dereferences it
immediately and then takes a lock inside it (event_groups.c:564-567):

pxList = &( pxEventBits->xTasksWaitingForBits );
pxListEnd = listGET_END_MARKER( pxList );
prvENTER_CRITICAL_OR_SUSPEND_ALL( &( pxEventBits->xEventGroupLock ) );

If the heap has reused the block, that is a spinlock taken on arbitrary data,
inside a critical section. Whether it faults, hangs or silently corrupts depends
on what the allocator put there — the same apparent randomness that made #445 so
hard to pin down.

The existing configASSERT( xEventGroup ) at event_groups.c:561 does not help:
it only catches NULL, and a dangling pointer is not NULL.

Suggested fix

The simplest is to not delete it at all — an event group is a few dozen bytes,
it is needed for as long as the handler is registered, and deleting it saves
nothing:

-    vEventGroupDelete(s_wifi_event_group);

Setting the bit later is then harmless, because nobody is waiting on it any more.

If you would rather keep the delete, it needs both halves:

     vEventGroupDelete(s_wifi_event_group);
+    s_wifi_event_group = NULL;

-        xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT);
+        if (s_wifi_event_group != NULL)
+        {
+            xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT);
+        }

Clearing the handle is worth doing either way: it turns an undefined heap access
into the clean configASSERT abort that is already there.

Honest note on evidence

This is derived from the sources, not from a captured crash. Reproducing it
needs an AP that disappears for good after a successful connection, and then the
symptom depends on heap reuse. I am reporting it because the code path is
unambiguous, not because I have a backtrace.

Related: what happens after those retries

Worth mentioning because a fix here could cover both. Once the else branch is
reached, esp_wifi_connect() is never called again — there is no timer, no
periodic retry, no fallback to AP mode. With WIFI_STA_MAXIMUM_RETRY at 5 (six
attempts including the one from WIFI_EVENT_STA_START) a controller that boots
while the router is still starting up gives up after a few seconds and stays
offline until it is power-cycled. The web server, mDNS and the locater broadcast
all start regardless, on an interface with no address.

If the retry counter were reset on a timer, or the handler simply kept retrying
at a slower rate, the dangling-handle branch would stop being reachable in
practice as well.