Skip to content

Commit 7583cca

Browse files
committed
Ensure orderly shutdown of ssl socket
A crash would occur if an SSL socket was not shut down before `gc_deinit()`. I do not fully understand the root cause, but some object deinitialization / deallocation prior to `gc_deinit` leaves the SSL object in an inconsistent state. Rather than resolve the root cause, instead ensure that the closing of the user socket also closes the SSL socket. Closes: #6502
1 parent 082b0d1 commit 7583cca

File tree

4 files changed

+20
-9
lines changed

4 files changed

+20
-9
lines changed

ports/espressif/common-hal/socketpool/Socket.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
#include "py/mperrno.h"
3232
#include "py/runtime.h"
3333
#include "shared-bindings/socketpool/SocketPool.h"
34+
#include "shared-bindings/ssl/SSLSocket.h"
35+
#include "common-hal/ssl/SSLSocket.h"
3436
#include "supervisor/port.h"
3537
#include "supervisor/shared/tick.h"
3638
#include "supervisor/workflow.h"
@@ -44,7 +46,7 @@
4446
StackType_t socket_select_stack[2 * configMINIMAL_STACK_SIZE];
4547

4648
STATIC int open_socket_fds[CONFIG_LWIP_MAX_SOCKETS];
47-
STATIC bool user_socket[CONFIG_LWIP_MAX_SOCKETS];
49+
STATIC socketpool_socket_obj_t *user_socket[CONFIG_LWIP_MAX_SOCKETS];
4850
StaticTask_t socket_select_task_handle;
4951
STATIC int socket_change_fd = -1;
5052

@@ -117,7 +119,7 @@ void socket_user_reset(void) {
117119

118120
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
119121
open_socket_fds[i] = -1;
120-
user_socket[i] = false;
122+
user_socket[i] = NULL;
121123
}
122124
socket_change_fd = eventfd(0, 0);
123125
// Run this at the same priority as CP so that the web workflow background task can be
@@ -134,12 +136,13 @@ void socket_user_reset(void) {
134136

135137
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
136138
if (open_socket_fds[i] >= 0 && user_socket[i]) {
139+
common_hal_socketpool_socket_close(user_socket[i]);
137140
int num = open_socket_fds[i];
138141
// Close automatically clears socket handle
139142
lwip_shutdown(num, SHUT_RDWR);
140143
lwip_close(num);
141144
open_socket_fds[i] = -1;
142-
user_socket[i] = false;
145+
user_socket[i] = NULL;
143146
}
144147
}
145148
}
@@ -171,10 +174,10 @@ STATIC void unregister_open_socket(int fd) {
171174
}
172175
}
173176

174-
STATIC void mark_user_socket(int fd) {
177+
STATIC void mark_user_socket(int fd, socketpool_socket_obj_t *obj) {
175178
for (size_t i = 0; i < MP_ARRAY_SIZE(open_socket_fds); i++) {
176179
if (open_socket_fds[i] == fd) {
177-
user_socket[i] = true;
180+
user_socket[i] = obj;
178181
return;
179182
}
180183
}
@@ -236,7 +239,7 @@ socketpool_socket_obj_t *common_hal_socketpool_socket(socketpool_socketpool_obj_
236239
if (!socketpool_socket(self, family, type, sock)) {
237240
mp_raise_RuntimeError(translate("Out of sockets"));
238241
}
239-
mark_user_socket(sock->num);
242+
mark_user_socket(sock->num, sock);
240243
return sock;
241244
}
242245

@@ -292,12 +295,12 @@ int socketpool_socket_accept(socketpool_socket_obj_t *self, uint8_t *ip, uint32_
292295

293296
socketpool_socket_obj_t *common_hal_socketpool_socket_accept(socketpool_socket_obj_t *self,
294297
uint8_t *ip, uint32_t *port) {
298+
socketpool_socket_obj_t *sock = m_new_obj_with_finaliser(socketpool_socket_obj_t);
295299
int newsoc = socketpool_socket_accept(self, ip, port, NULL);
296300

297301
if (newsoc > 0) {
298-
mark_user_socket(newsoc);
299302
// Create the socket
300-
socketpool_socket_obj_t *sock = m_new_obj_with_finaliser(socketpool_socket_obj_t);
303+
mark_user_socket(newsoc, sock);
301304
sock->base.type = &socketpool_socket_type;
302305
sock->num = newsoc;
303306
sock->pool = self->pool;
@@ -338,6 +341,12 @@ bool common_hal_socketpool_socket_bind(socketpool_socket_obj_t *self,
338341
}
339342

340343
void socketpool_socket_close(socketpool_socket_obj_t *self) {
344+
if (self->ssl_socket) {
345+
ssl_sslsocket_obj_t *ssl_socket = self->ssl_socket;
346+
self->ssl_socket = NULL;
347+
common_hal_ssl_sslsocket_close(ssl_socket);
348+
return;
349+
}
341350
self->connected = false;
342351
if (self->num >= 0) {
343352
lwip_shutdown(self->num, SHUT_RDWR);

ports/espressif/common-hal/socketpool/Socket.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ typedef struct {
4242
int ipproto;
4343
bool connected;
4444
socketpool_socketpool_obj_t *pool;
45+
struct ssl_sslsocket_obj *ssl_socket;
4546
mp_uint_t timeout_ms;
4647
} socketpool_socket_obj_t;
4748

ports/espressif/common-hal/ssl/SSLContext.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ ssl_sslsocket_obj_t *common_hal_ssl_sslcontext_wrap_socket(ssl_sslcontext_obj_t
4848
sock->base.type = &ssl_sslsocket_type;
4949
sock->ssl_context = self;
5050
sock->sock = socket;
51+
socket->ssl_socket = sock;
5152

5253
// Create a copy of the ESP-TLS config object and store the server hostname
5354
// Note that ESP-TLS will use common_name for both SNI and verification

ports/espressif/common-hal/ssl/SSLSocket.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
#include "components/esp-tls/esp_tls.h"
3636

37-
typedef struct {
37+
typedef struct ssl_sslsocket_obj {
3838
mp_obj_base_t base;
3939
socketpool_socket_obj_t *sock;
4040
esp_tls_t *tls;

0 commit comments

Comments
 (0)