From 406356ec8b289ae47bbad1bf9869171a7b13fd1d Mon Sep 17 00:00:00 2001 From: Damien George Date: Sat, 7 Feb 2026 23:11:16 +1100 Subject: [PATCH] extmod/modlwip: Ensure socket is finalisable if error during creation. Because socket objects have a finaliser they must be created carefully, in case an exception is raised during the population of their members, eg invalid input argument or out-of-memory when allocating additional arrays. Prior to the fix in this commit, the finaliser would crash due to `incoming.udp_raw.array` being an invalid pointer in the following cases: - if a SOCK_RAW was created with a proto argument that was not an integer - if a SOCK_DGRAM or SOCK_RAW was created where the allocation of `lwip_incoming_packet_t` failed - if an integer was passed in for the socket type but it was not one of SOCK_STREAM, SOCK_DGRAM or SOCK_RAW Furthermore, if the allocation of `lwip_incoming_packet_t` failed then it may have led to corruption within lwIP when freeing `socket->pcb.raw` because that PCB was not fully set up with its callbacks. This commit fixes all of these issues by ensuring: - `pcb.tcp` and `incoming.udp_raw.array` are initialised to NULL early on - the proto argument is parsed before allocating the PCB - the allocation of `lwip_incoming_packet_t` occurs befor allocating the PCB - `incoming.udp_raw.array` is checked for NULL in the finaliser code The corresponding test (which already checked most of these causes of failure) has been updated to include a previously-uncovered scenario. Signed-off-by: Damien George --- extmod/modlwip.c | 18 ++++++++++++++---- tests/extmod/socket_badconstructor.py | 5 +++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/extmod/modlwip.c b/extmod/modlwip.c index 4a72a05d84..9365bd492b 100644 --- a/extmod/modlwip.c +++ b/extmod/modlwip.c @@ -384,7 +384,7 @@ static void lwip_socket_free_incoming(lwip_socket_obj_t *socket, bool free_queue pbuf_free(socket->incoming.tcp.pbuf); socket->incoming.tcp.pbuf = NULL; } - } else { + } else if (socket->incoming.udp_raw.array != NULL) { for (size_t i = 0; i < LWIP_INCOMING_PACKET_QUEUE_LEN; ++i) { lwip_incoming_packet_t *slot = &socket->incoming.udp_raw.array[i]; if (slot->pbuf != NULL) { @@ -938,7 +938,12 @@ static void lwip_socket_print(const mp_print_t *print, mp_obj_t self_in, mp_prin static mp_obj_t lwip_socket_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { mp_arg_check_num(n_args, n_kw, 0, 4, false); + // Once the socket is allocated it must be in a valid state to be finalised: + // - `incoming.udp_raw.array` is NULL or a valid heap pointer + // - `pcb` is NULL or a valid lwIP PCB that has been fully initialised lwip_socket_obj_t *socket = mp_obj_malloc_with_finaliser(lwip_socket_obj_t, &lwip_socket_type); + socket->pcb.tcp = NULL; + socket->incoming.udp_raw.array = NULL; socket->timeout = -1; socket->recv_offset = 0; socket->domain = MOD_NETWORK_AF_INET; @@ -946,10 +951,16 @@ static mp_obj_t lwip_socket_make_new(const mp_obj_type_t *type, size_t n_args, s socket->callback = MP_OBJ_NULL; socket->state = STATE_NEW; + // Parse given arguments. + uint8_t socket_proto = 0; + (void)socket_proto; if (n_args >= 1) { socket->domain = mp_obj_get_int(args[0]); if (n_args >= 2) { socket->type = mp_obj_get_int(args[1]); + if (n_args >= 3) { + socket_proto = mp_obj_get_int(args[2]); + } } } @@ -963,18 +974,17 @@ static mp_obj_t lwip_socket_make_new(const mp_obj_type_t *type, size_t n_args, s #if MICROPY_PY_LWIP_SOCK_RAW case MOD_NETWORK_SOCK_RAW: #endif + socket->incoming.udp_raw.array = m_new0(lwip_incoming_packet_t, LWIP_INCOMING_PACKET_QUEUE_LEN); if (socket->type == MOD_NETWORK_SOCK_DGRAM) { socket->pcb.udp = udp_new(); } #if MICROPY_PY_LWIP_SOCK_RAW else { - mp_int_t proto = n_args <= 2 ? 0 : mp_obj_get_int(args[2]); - socket->pcb.raw = raw_new(proto); + socket->pcb.raw = raw_new(socket_proto); } #endif socket->incoming.udp_raw.iget = 0; socket->incoming.udp_raw.iput = 0; - socket->incoming.udp_raw.array = m_new0(lwip_incoming_packet_t, LWIP_INCOMING_PACKET_QUEUE_LEN); break; default: mp_raise_OSError(MP_EINVAL); diff --git a/tests/extmod/socket_badconstructor.py b/tests/extmod/socket_badconstructor.py index 4a9d2668c7..1ea5d750b3 100644 --- a/tests/extmod/socket_badconstructor.py +++ b/tests/extmod/socket_badconstructor.py @@ -16,6 +16,11 @@ try: except TypeError: print("TypeError") +try: + s = socket.socket(socket.AF_INET, 123456) +except OSError: + print("OSError") + try: s = socket.socket(socket.AF_INET, socket.SOCK_RAW, None) except TypeError: