From f2ad152e7e04608cbf57d756f6a4cebbf9976f6c Mon Sep 17 00:00:00 2001 From: Damien George Date: Fri, 23 Sep 2022 14:20:37 +1000 Subject: [PATCH] extmod/modbluetooth: Run BLE IRQ callback in protected NLR context. The call to invoke_irq_handler_run() always needs to run in a protected NLR context, to catch exceptions from the Python handler, and the m_new's (and also mp_local_alloc when PYSTACK is enabled). With MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS_WITH_INTERLOCK enabled there was already an explicit nlr_push, and that is now used in all cases. Without this change, on stm32 (for example), the callbacks from the BLE stack to invoke_irq_handler() were made via static scheduled nodes which do not have any NLR protection, and hence would lead to a hard fault (uncaught NLR) if an exception was raised in the Python BLE IRQ handler. This was a regression introduced by 8045ac07f599c0ccc447c88a0b778f704b497559, which is fixed by this commit. Signed-off-by: Damien George --- extmod/modbluetooth.c | 51 ++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/extmod/modbluetooth.c b/extmod/modbluetooth.c index acdbf8d85d..3a51fb8b23 100644 --- a/extmod/modbluetooth.c +++ b/extmod/modbluetooth.c @@ -1143,10 +1143,6 @@ STATIC mp_obj_t invoke_irq_handler_run(uint16_t event, const uint8_t *addr, const mp_obj_bluetooth_uuid_t *uuid, const uint8_t **data, uint16_t *data_len, size_t n_data) { - mp_obj_bluetooth_ble_t *o = MP_OBJ_TO_PTR(MP_STATE_VM(bluetooth)); - if (o->irq_handler == mp_const_none) { - return mp_const_none; - } mp_obj_array_t mv_addr; mp_obj_array_t mv_data[2]; @@ -1210,6 +1206,7 @@ STATIC mp_obj_t invoke_irq_handler_run(uint16_t event, assert(data_tuple->len <= MICROPY_PY_BLUETOOTH_MAX_EVENT_DATA_TUPLE_LEN); + mp_obj_bluetooth_ble_t *o = MP_OBJ_TO_PTR(MP_STATE_VM(bluetooth)); mp_obj_t result = mp_call_function_2(o->irq_handler, MP_OBJ_NEW_SMALL_INT(event), MP_OBJ_FROM_PTR(data_tuple)); #if MICROPY_PY_BLUETOOTH_USE_GATTC_EVENT_DATA_REASSEMBLY @@ -1223,6 +1220,34 @@ STATIC mp_obj_t invoke_irq_handler_run(uint16_t event, return result; } +STATIC mp_obj_t invoke_irq_handler_run_protected(uint16_t event, + const mp_int_t *numeric, size_t n_unsigned, size_t n_signed, + const uint8_t *addr, + const mp_obj_bluetooth_uuid_t *uuid, + const uint8_t **data, uint16_t *data_len, size_t n_data) { + + mp_obj_bluetooth_ble_t *o = MP_OBJ_TO_PTR(MP_STATE_VM(bluetooth)); + if (o->irq_handler == mp_const_none) { + return mp_const_none; + } + + mp_obj_t result = mp_const_none; + nlr_buf_t nlr; + if (nlr_push(&nlr) == 0) { + result = invoke_irq_handler_run(event, numeric, n_unsigned, n_signed, addr, uuid, data, data_len, n_data); + nlr_pop(); + } else { + // Uncaught exception, print it out. + mp_printf(MICROPY_ERROR_PRINTER, "Unhandled exception in IRQ callback handler\n"); + mp_obj_print_exception(MICROPY_ERROR_PRINTER, MP_OBJ_FROM_PTR(nlr.ret_val)); + + // Disable the BLE IRQ handler. + o->irq_handler = mp_const_none; + } + + return result; +} + #if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS_WITH_INTERLOCK // On some systems the BLE event callbacks may occur on a system thread which is not @@ -1256,19 +1281,9 @@ STATIC mp_obj_t invoke_irq_handler(uint16_t event, MP_THREAD_GIL_ENTER(); } - mp_obj_t result = mp_const_none; - nlr_buf_t nlr; - if (nlr_push(&nlr) == 0) { - mp_sched_lock(); - result = invoke_irq_handler_run(event, numeric, n_unsigned, n_signed, addr, uuid, data, data_len, n_data); - mp_sched_unlock(); - nlr_pop(); - } else { - // Uncaught exception, print it out. - mp_sched_unlock(); - mp_printf(MICROPY_ERROR_PRINTER, "Unhandled exception in IRQ callback handler\n"); - mp_obj_print_exception(MICROPY_ERROR_PRINTER, MP_OBJ_FROM_PTR(nlr.ret_val)); - } + mp_sched_lock(); + mp_obj_t result = invoke_irq_handler_run_protected(event, numeric, n_unsigned, n_signed, addr, uuid, data, data_len, n_data); + mp_sched_unlock(); if (ts_orig == NULL) { MP_THREAD_GIL_EXIT(); @@ -1288,7 +1303,7 @@ STATIC mp_obj_t invoke_irq_handler(uint16_t event, const uint8_t *addr, const mp_obj_bluetooth_uuid_t *uuid, const uint8_t **data, uint16_t *data_len, size_t n_data) { - return invoke_irq_handler_run(event, numeric, n_unsigned, n_signed, addr, uuid, data, data_len, n_data); + return invoke_irq_handler_run_protected(event, numeric, n_unsigned, n_signed, addr, uuid, data, data_len, n_data); } #endif