Skip to content

Commit 406356e

Browse files
committed
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 <damien@micropython.org>
1 parent b3d88cf commit 406356e

2 files changed

Lines changed: 19 additions & 4 deletions

File tree

extmod/modlwip.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ static void lwip_socket_free_incoming(lwip_socket_obj_t *socket, bool free_queue
384384
pbuf_free(socket->incoming.tcp.pbuf);
385385
socket->incoming.tcp.pbuf = NULL;
386386
}
387-
} else {
387+
} else if (socket->incoming.udp_raw.array != NULL) {
388388
for (size_t i = 0; i < LWIP_INCOMING_PACKET_QUEUE_LEN; ++i) {
389389
lwip_incoming_packet_t *slot = &socket->incoming.udp_raw.array[i];
390390
if (slot->pbuf != NULL) {
@@ -938,18 +938,29 @@ static void lwip_socket_print(const mp_print_t *print, mp_obj_t self_in, mp_prin
938938
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) {
939939
mp_arg_check_num(n_args, n_kw, 0, 4, false);
940940

941+
// Once the socket is allocated it must be in a valid state to be finalised:
942+
// - `incoming.udp_raw.array` is NULL or a valid heap pointer
943+
// - `pcb` is NULL or a valid lwIP PCB that has been fully initialised
941944
lwip_socket_obj_t *socket = mp_obj_malloc_with_finaliser(lwip_socket_obj_t, &lwip_socket_type);
945+
socket->pcb.tcp = NULL;
946+
socket->incoming.udp_raw.array = NULL;
942947
socket->timeout = -1;
943948
socket->recv_offset = 0;
944949
socket->domain = MOD_NETWORK_AF_INET;
945950
socket->type = MOD_NETWORK_SOCK_STREAM;
946951
socket->callback = MP_OBJ_NULL;
947952
socket->state = STATE_NEW;
948953

954+
// Parse given arguments.
955+
uint8_t socket_proto = 0;
956+
(void)socket_proto;
949957
if (n_args >= 1) {
950958
socket->domain = mp_obj_get_int(args[0]);
951959
if (n_args >= 2) {
952960
socket->type = mp_obj_get_int(args[1]);
961+
if (n_args >= 3) {
962+
socket_proto = mp_obj_get_int(args[2]);
963+
}
953964
}
954965
}
955966

@@ -963,18 +974,17 @@ static mp_obj_t lwip_socket_make_new(const mp_obj_type_t *type, size_t n_args, s
963974
#if MICROPY_PY_LWIP_SOCK_RAW
964975
case MOD_NETWORK_SOCK_RAW:
965976
#endif
977+
socket->incoming.udp_raw.array = m_new0(lwip_incoming_packet_t, LWIP_INCOMING_PACKET_QUEUE_LEN);
966978
if (socket->type == MOD_NETWORK_SOCK_DGRAM) {
967979
socket->pcb.udp = udp_new();
968980
}
969981
#if MICROPY_PY_LWIP_SOCK_RAW
970982
else {
971-
mp_int_t proto = n_args <= 2 ? 0 : mp_obj_get_int(args[2]);
972-
socket->pcb.raw = raw_new(proto);
983+
socket->pcb.raw = raw_new(socket_proto);
973984
}
974985
#endif
975986
socket->incoming.udp_raw.iget = 0;
976987
socket->incoming.udp_raw.iput = 0;
977-
socket->incoming.udp_raw.array = m_new0(lwip_incoming_packet_t, LWIP_INCOMING_PACKET_QUEUE_LEN);
978988
break;
979989
default:
980990
mp_raise_OSError(MP_EINVAL);

tests/extmod/socket_badconstructor.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
except TypeError:
1717
print("TypeError")
1818

19+
try:
20+
s = socket.socket(socket.AF_INET, 123456)
21+
except OSError:
22+
print("OSError")
23+
1924
try:
2025
s = socket.socket(socket.AF_INET, socket.SOCK_RAW, None)
2126
except TypeError:

0 commit comments

Comments
 (0)