From e4e1bd076e883bf597c5abb790f7a3d7cdbc59b7 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Thu, 22 Oct 2026 06:21:56 -0700 Subject: [PATCH] wifi.Monitor: track deinit per object, not globally deinited() checked whether promiscuous mode was on, which is global. An old, deinited Monitor whose __del__ ran while a newer one was live saw "not deinited", turned promiscuous mode off and passed its NULL queue to FreeRTOS, which hard faults. packet() and queued() on the old object did the same. deinited() now takes self, like every other deinited(), and checks the object's own queue. deinit() clears the singleton, because the root pointer outlives the VM heap and would otherwise be read in the next VM. The rx callback skips packets while there is no singleton, which also fixes a NULL dereference before the first Monitor is set and a payload that was malloc'd for a queueless Monitor and never freed. Fixes #11650 Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/common-hal/wifi/Monitor.c | 28 ++++++++++++--------- ports/espressif/common-hal/wifi/__init__.c | 4 ++- ports/raspberrypi/common-hal/wifi/Monitor.c | 2 +- ports/zephyr-cp/common-hal/wifi/Monitor.c | 4 +-- shared-bindings/wifi/Monitor.c | 6 ++--- shared-bindings/wifi/Monitor.h | 2 +- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/ports/espressif/common-hal/wifi/Monitor.c b/ports/espressif/common-hal/wifi/Monitor.c index f6d0e6e4d8f..8bb4eb98e23 100644 --- a/ports/espressif/common-hal/wifi/Monitor.c +++ b/ports/espressif/common-hal/wifi/Monitor.c @@ -30,6 +30,12 @@ static const char *TAG = "monitor"; static void wifi_monitor_cb(void *recv_buf, wifi_promiscuous_pkt_type_t type) { wifi_promiscuous_pkt_t *pkt = (wifi_promiscuous_pkt_t *)recv_buf; + // Construct enables promiscuous mode before make_new sets the singleton. + wifi_monitor_obj_t *self = MP_STATE_VM(wifi_monitor_singleton); + if (self == NULL || common_hal_wifi_monitor_deinited(self)) { + return; + } + // prepare packet monitor_packet_t packet = { .channel = pkt->rx_ctrl.channel, @@ -43,14 +49,11 @@ static void wifi_monitor_cb(void *recv_buf, wifi_promiscuous_pkt_type_t type) { packet.payload = malloc(packet.length); if (packet.payload) { memcpy(packet.payload, pkt->payload, packet.length); - wifi_monitor_obj_t *self = MP_STATE_VM(wifi_monitor_singleton); - if (self->queue) { - // send packet - if (xQueueSendFromISR(self->queue, &packet, NULL) != pdTRUE) { - self->lost++; - free(packet.payload); - ESP_LOGE(TAG, "packet queue full"); - } + // send packet + if (xQueueSendFromISR(self->queue, &packet, NULL) != pdTRUE) { + self->lost++; + free(packet.payload); + ESP_LOGE(TAG, "packet queue full"); } } else { ESP_LOGE(TAG, "not enough memory for packet"); @@ -97,13 +100,12 @@ void common_hal_wifi_monitor_construct(wifi_monitor_obj_t *self, uint8_t channel self->queue_length = queue; } -bool common_hal_wifi_monitor_deinited(void) { - bool enabled; - return (esp_wifi_get_promiscuous(&enabled) == ESP_ERR_WIFI_NOT_INIT) ? true : !enabled; +bool common_hal_wifi_monitor_deinited(wifi_monitor_obj_t *self) { + return self->queue == NULL; } void common_hal_wifi_monitor_deinit(wifi_monitor_obj_t *self) { - if (common_hal_wifi_monitor_deinited()) { + if (common_hal_wifi_monitor_deinited(self)) { return; } @@ -119,6 +121,8 @@ void common_hal_wifi_monitor_deinit(wifi_monitor_obj_t *self) { } vQueueDelete(self->queue); self->queue = NULL; + // Root pointers outlive the VM heap. + MP_STATE_VM(wifi_monitor_singleton) = NULL; } void common_hal_wifi_monitor_set_channel(wifi_monitor_obj_t *self, uint8_t channel) { diff --git a/ports/espressif/common-hal/wifi/__init__.c b/ports/espressif/common-hal/wifi/__init__.c index 59172b17851..5895638e68c 100644 --- a/ports/espressif/common-hal/wifi/__init__.c +++ b/ports/espressif/common-hal/wifi/__init__.c @@ -576,7 +576,9 @@ void wifi_reset(void) { if (!wifi_inited) { return; } - common_hal_wifi_monitor_deinit(MP_STATE_VM(wifi_monitor_singleton)); + if (MP_STATE_VM(wifi_monitor_singleton) != NULL) { + common_hal_wifi_monitor_deinit(MP_STATE_VM(wifi_monitor_singleton)); + } wifi_radio_obj_t *radio = &common_hal_wifi_radio_obj; common_hal_wifi_radio_set_enabled(radio, false); #ifndef CONFIG_IDF_TARGET_ESP32 diff --git a/ports/raspberrypi/common-hal/wifi/Monitor.c b/ports/raspberrypi/common-hal/wifi/Monitor.c index 8469c60ad56..58c49ab7916 100644 --- a/ports/raspberrypi/common-hal/wifi/Monitor.c +++ b/ports/raspberrypi/common-hal/wifi/Monitor.c @@ -23,7 +23,7 @@ void common_hal_wifi_monitor_construct(wifi_monitor_obj_t *self, uint8_t channel mp_raise_NotImplementedError(MP_ERROR_TEXT("wifi.Monitor not available")); } -bool common_hal_wifi_monitor_deinited(void) { +bool common_hal_wifi_monitor_deinited(wifi_monitor_obj_t *self) { return true; } diff --git a/ports/zephyr-cp/common-hal/wifi/Monitor.c b/ports/zephyr-cp/common-hal/wifi/Monitor.c index 7cacb8b520a..ca1f920a4c6 100644 --- a/ports/zephyr-cp/common-hal/wifi/Monitor.c +++ b/ports/zephyr-cp/common-hal/wifi/Monitor.c @@ -76,14 +76,14 @@ void common_hal_wifi_monitor_construct(wifi_monitor_obj_t *self, uint8_t channel // self->queue_length = queue; } -bool common_hal_wifi_monitor_deinited(void) { +bool common_hal_wifi_monitor_deinited(wifi_monitor_obj_t *self) { // bool enabled; // return (esp_wifi_get_promiscuous(&enabled) == ESP_ERR_WIFI_NOT_INIT) ? true : !enabled; return true; } void common_hal_wifi_monitor_deinit(wifi_monitor_obj_t *self) { - if (common_hal_wifi_monitor_deinited()) { + if (common_hal_wifi_monitor_deinited(self)) { return; } diff --git a/shared-bindings/wifi/Monitor.c b/shared-bindings/wifi/Monitor.c index 55a786c5428..fc321614d31 100644 --- a/shared-bindings/wifi/Monitor.c +++ b/shared-bindings/wifi/Monitor.c @@ -45,7 +45,7 @@ static mp_obj_t wifi_monitor_make_new(const mp_obj_type_t *type, size_t n_args, mp_int_t queue = mp_arg_validate_int_min(args[ARG_queue].u_int, 0, MP_QSTR_queue); wifi_monitor_obj_t *self = MP_STATE_VM(wifi_monitor_singleton); - if (common_hal_wifi_monitor_deinited()) { + if (self == NULL || common_hal_wifi_monitor_deinited(self)) { self = mp_obj_malloc_with_finaliser(wifi_monitor_obj_t, &wifi_monitor_type); common_hal_wifi_monitor_construct(self, channel, queue); MP_STATE_VM(wifi_monitor_singleton) = self; @@ -109,7 +109,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_monitor_lost_obj, wifi_monitor_obj_get_lost); //| ... //| static mp_obj_t wifi_monitor_obj_get_queued(mp_obj_t self_in) { - if (common_hal_wifi_monitor_deinited()) { + if (common_hal_wifi_monitor_deinited(self_in)) { return mp_obj_new_int_from_uint(0); } return common_hal_wifi_monitor_get_queued(self_in); @@ -122,7 +122,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_monitor_queued_obj, wifi_monitor_obj_get_queued); //| //| static mp_obj_t wifi_monitor_obj_get_packet(mp_obj_t self_in) { - if (common_hal_wifi_monitor_deinited()) { + if (common_hal_wifi_monitor_deinited(self_in)) { raise_deinited_error(); } return common_hal_wifi_monitor_get_packet(self_in); diff --git a/shared-bindings/wifi/Monitor.h b/shared-bindings/wifi/Monitor.h index c6a4ae6ecdf..cef14303522 100644 --- a/shared-bindings/wifi/Monitor.h +++ b/shared-bindings/wifi/Monitor.h @@ -13,7 +13,7 @@ extern const mp_obj_type_t wifi_monitor_type; void common_hal_wifi_monitor_construct(wifi_monitor_obj_t *self, uint8_t channel, size_t queue); void common_hal_wifi_monitor_deinit(wifi_monitor_obj_t *self); -bool common_hal_wifi_monitor_deinited(void); +bool common_hal_wifi_monitor_deinited(wifi_monitor_obj_t *self); void common_hal_wifi_monitor_set_channel(wifi_monitor_obj_t *self, uint8_t channel); mp_obj_t common_hal_wifi_monitor_get_channel(wifi_monitor_obj_t *self);