extmod/modlwip: Fix bug when polling listening socket with backlog=1.

The bug polling for readability was: if alloc==0 and tcp.item==NULL then
the code would incorrectly check tcp.array[iget] which is an invalid
dereference when alloc==0.  This patch refactors the code to use a helper
function lwip_socket_incoming_array() to return the correct pointer for the
incomming connection array.

Fixes issue #4511.
This commit is contained in:
Damien George 2019-02-18 14:23:35 +11:00
parent c65e5c88b8
commit 42c0e440b9
1 changed files with 16 additions and 25 deletions

View File

@ -3,7 +3,7 @@
* *
* The MIT License (MIT) * The MIT License (MIT)
* *
* Copyright (c) 2013, 2014 Damien P. George * Copyright (c) 2013-2019 Damien P. George
* Copyright (c) 2015 Galen Hazelwood * Copyright (c) 2015 Galen Hazelwood
* Copyright (c) 2015-2017 Paul Sokolovsky * Copyright (c) 2015-2017 Paul Sokolovsky
* *
@ -308,6 +308,14 @@ static inline void poll_sockets(void) {
#endif #endif
} }
STATIC struct tcp_pcb *volatile *lwip_socket_incoming_array(lwip_socket_obj_t *socket) {
if (socket->incoming.connection.alloc == 0) {
return &socket->incoming.connection.tcp.item;
} else {
return &socket->incoming.connection.tcp.array[0];
}
}
/*******************************************************************************/ /*******************************************************************************/
// Callback functions for the lwIP raw API. // Callback functions for the lwIP raw API.
@ -381,15 +389,10 @@ STATIC err_t _lwip_tcp_accept(void *arg, struct tcp_pcb *newpcb, err_t err) {
tcp_recv(newpcb, _lwip_tcp_recv_unaccepted); tcp_recv(newpcb, _lwip_tcp_recv_unaccepted);
// Search for an empty slot to store the new connection // Search for an empty slot to store the new connection
struct tcp_pcb *volatile *tcp_array; struct tcp_pcb *volatile *slot = &lwip_socket_incoming_array(socket)[socket->incoming.connection.iput];
if (socket->incoming.connection.alloc == 0) { if (*slot == NULL) {
tcp_array = &socket->incoming.connection.tcp.item;
} else {
tcp_array = socket->incoming.connection.tcp.array;
}
if (tcp_array[socket->incoming.connection.iput] == NULL) {
// Have an empty slot to store waiting connection // Have an empty slot to store waiting connection
tcp_array[socket->incoming.connection.iput] = newpcb; *slot = newpcb;
if (++socket->incoming.connection.iput >= socket->incoming.connection.alloc) { if (++socket->incoming.connection.iput >= socket->incoming.connection.alloc) {
socket->incoming.connection.iput = 0; socket->incoming.connection.iput = 0;
} }
@ -782,12 +785,7 @@ STATIC mp_obj_t lwip_socket_accept(mp_obj_t self_in) {
} }
// accept incoming connection // accept incoming connection
struct tcp_pcb *volatile *incoming_connection; struct tcp_pcb *volatile *incoming_connection = &lwip_socket_incoming_array(socket)[socket->incoming.connection.iget];
if (socket->incoming.connection.alloc == 0) {
incoming_connection = &socket->incoming.connection.tcp.item;
} else {
incoming_connection = &socket->incoming.connection.tcp.array[socket->incoming.connection.iget];
}
if (*incoming_connection == NULL) { if (*incoming_connection == NULL) {
if (socket->timeout == 0) { if (socket->timeout == 0) {
mp_raise_OSError(MP_EAGAIN); mp_raise_OSError(MP_EAGAIN);
@ -1218,10 +1216,8 @@ STATIC mp_uint_t lwip_socket_ioctl(mp_obj_t self_in, mp_uint_t request, uintptr_
if (flags & MP_STREAM_POLL_RD) { if (flags & MP_STREAM_POLL_RD) {
if (socket->state == STATE_LISTENING) { if (socket->state == STATE_LISTENING) {
// Listening TCP socket may have one or multiple connections waiting // Listening TCP socket may have one or multiple connections waiting
if ((socket->incoming.connection.alloc == 0 if (lwip_socket_incoming_array(socket)[socket->incoming.connection.iget] != NULL) {
&& socket->incoming.connection.tcp.item != NULL) ret |= MP_STREAM_POLL_RD;
|| socket->incoming.connection.tcp.array[socket->incoming.connection.iget] != NULL) {
ret |= MP_STREAM_POLL_RD;
} }
} else { } else {
// Otherwise there is just one slot for incoming data // Otherwise there is just one slot for incoming data
@ -1292,12 +1288,7 @@ STATIC mp_uint_t lwip_socket_ioctl(mp_obj_t self_in, mp_uint_t request, uintptr_
} }
} else { } else {
uint8_t alloc = socket->incoming.connection.alloc; uint8_t alloc = socket->incoming.connection.alloc;
struct tcp_pcb *volatile *tcp_array; struct tcp_pcb *volatile *tcp_array = lwip_socket_incoming_array(socket);
if (alloc == 0) {
tcp_array = &socket->incoming.connection.tcp.item;
} else {
tcp_array = socket->incoming.connection.tcp.array;
}
for (uint8_t i = 0; i < alloc; ++i) { for (uint8_t i = 0; i < alloc; ++i) {
// Deregister callback and abort // Deregister callback and abort
if (tcp_array[i] != NULL) { if (tcp_array[i] != NULL) {