From 16c70dd60e80f118dca7e97643847b6a64997caa Mon Sep 17 00:00:00 2001 From: sg Date: Tue, 5 Apr 2016 21:42:10 +0200 Subject: [PATCH] fixed bug# 43739 (Accept not reporting errors about aborted connections): netconn_accept() returns ERR_ABRT (sockets: ECONNABORTED) for aborted connections, ERR_CLSD (sockets: EINVAL) if the listening netconn is closed, which better seems to follow the standard --- CHANGELOG | 6 ++++++ src/api/api_lib.c | 20 ++++++++++++++------ src/api/api_msg.c | 27 ++++++++++++++++++++------- src/api/sockets.c | 6 ++++-- src/include/lwip/priv/api_msg.h | 4 ++++ 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 540690d8..ec60e129 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -317,6 +317,12 @@ HISTORY ++ Bugfixes: + 2016-04-05: Simon Goldschmidt + * netconn/socket API: fixed bug# 43739 (Accept not reporting errors about + aborted connections): netconn_accept() returns ERR_ABRT (sockets: ECONNABORTED) + for aborted connections, ERR_CLSD (sockets: EINVAL) if the listening netconn + is closed, which better seems to follow the standard. + 2016-03-23: Florent Matignon * dhcp.c: fixed bug #38203: DHCP options are not recorded in all DHCP ack messages diff --git a/src/api/api_lib.c b/src/api/api_lib.c index c8a0b736..7044c1fe 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -347,6 +347,7 @@ err_t netconn_accept(struct netconn *conn, struct netconn **new_conn) { #if LWIP_TCP + void *accept_ptr; struct netconn *newconn; err_t err; #if TCP_LISTEN_BACKLOG @@ -356,7 +357,6 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn) LWIP_ERROR("netconn_accept: invalid pointer", (new_conn != NULL), return ERR_ARG;); *new_conn = NULL; LWIP_ERROR("netconn_accept: invalid conn", (conn != NULL), return ERR_ARG;); - LWIP_ERROR("netconn_accept: invalid acceptmbox", sys_mbox_valid(&conn->acceptmbox), return ERR_ARG;); err = conn->last_err; if (ERR_IS_FATAL(err)) { @@ -364,24 +364,32 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn) waiting on acceptmbox forever! */ return err; } - + if (!sys_mbox_valid(&conn->acceptmbox)) { + return ERR_CLSD; + } #if LWIP_SO_RCVTIMEO - if (sys_arch_mbox_fetch(&conn->acceptmbox, (void **)&newconn, conn->recv_timeout) == SYS_ARCH_TIMEOUT) { + if (sys_arch_mbox_fetch(&conn->acceptmbox, &accept_ptr, conn->recv_timeout) == SYS_ARCH_TIMEOUT) { return ERR_TIMEOUT; } #else - sys_arch_mbox_fetch(&conn->acceptmbox, (void **)&newconn, 0); + sys_arch_mbox_fetch(&conn->acceptmbox, &accept_ptr, 0); #endif /* LWIP_SO_RCVTIMEO*/ + newconn = (struct netconn *)accept_ptr; /* Register event with callback */ API_EVENT(conn, NETCONN_EVT_RCVMINUS, 0); + if (accept_ptr == &netconn_aborted) { + /* a connection has been aborted: out of pcbs or out of netconns during accept */ + /* @todo: set netconn error, but this would be fatal and thus block further accepts */ + return ERR_ABRT; + } if (newconn == NULL) { /* connection has been aborted */ /* in this special case, we set the netconn error from application thread, as on a ready-to-accept listening netconn, there should not be anything running in tcpip_thread */ - NETCONN_SET_SAFE_ERR(conn, ERR_ABRT); - return ERR_ABRT; + NETCONN_SET_SAFE_ERR(conn, ERR_CLSD); + return ERR_CLSD; } #if TCP_LISTEN_BACKLOG /* Let the stack know that we have accepted the connection. */ diff --git a/src/api/api_msg.c b/src/api/api_msg.c index fe97c012..36efa790 100644 --- a/src/api/api_msg.c +++ b/src/api/api_msg.c @@ -83,6 +83,10 @@ static err_t lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_ #define TCPIP_APIMSG_ACK(m) do { NETCONN_SET_SAFE_ERR((m)->conn, (m)->err); sys_sem_signal(LWIP_API_MSG_SEM(m)); } while(0) #endif /* LWIP_TCPIP_CORE_LOCKING */ +#if LWIP_TCP +u8_t netconn_aborted; +#endif /* LWIP_TCP */ + #if LWIP_RAW /** * Receive callback function for RAW netconns. @@ -468,23 +472,32 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err) LWIP_DEBUGF(API_MSG_DEBUG, ("accept_function: newpcb->tate: %s\n", tcp_debug_state_str(newpcb->state))); - if ((err != ERR_OK) || (arg == NULL)) { + if (conn == NULL) { return ERR_VAL; } - if (newpcb == NULL) { - /* @todo: out-of-pcbs during connect: pass on this error to the application */ - return ERR_VAL; - } - if (!sys_mbox_valid(&conn->acceptmbox)) { LWIP_DEBUGF(API_MSG_DEBUG, ("accept_function: acceptmbox already deleted\n")); return ERR_VAL; } + if (newpcb == NULL) { + /* out-of-pcbs during connect: pass on this error to the application */ + if (sys_mbox_trypost(&conn->acceptmbox, &netconn_aborted) == ERR_OK) { + /* Register event with callback */ + API_EVENT(conn, NETCONN_EVT_RCVPLUS, 0); + } + return ERR_VAL; + } + /* We have to set the callback here even though - * the new socket is unknown. conn->socket is marked as -1. */ + * the new socket is unknown. newconn->socket is marked as -1. */ newconn = netconn_alloc(conn->type, conn->callback); if (newconn == NULL) { + /* outof netconns: pass on this error to the application */ + if (sys_mbox_trypost(&conn->acceptmbox, &netconn_aborted) == ERR_OK) { + /* Register event with callback */ + API_EVENT(conn, NETCONN_EVT_RCVPLUS, 0); + } return ERR_MEM; } newconn->pcb.tcp = newpcb; diff --git a/src/api/sockets.c b/src/api/sockets.c index 15878047..689f42df 100644 --- a/src/api/sockets.c +++ b/src/api/sockets.c @@ -514,9 +514,11 @@ lwip_accept(int s, struct sockaddr *addr, socklen_t *addrlen) LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_accept(%d): netconn_acept failed, err=%d\n", s, err)); if (NETCONNTYPE_GROUP(netconn_type(sock->conn)) != NETCONN_TCP) { sock_set_errno(sock, EOPNOTSUPP); - return -1; + } else if (err == ERR_CLSD) { + sock_set_errno(sock, EINVAL); + } else { + sock_set_errno(sock, err_to_errno(err)); } - sock_set_errno(sock, err_to_errno(err)); return -1; } LWIP_ASSERT("newconn != NULL", newconn != NULL); diff --git a/src/include/lwip/priv/api_msg.h b/src/include/lwip/priv/api_msg.h index 771e81ff..9c3b4750 100644 --- a/src/include/lwip/priv/api_msg.h +++ b/src/include/lwip/priv/api_msg.h @@ -176,6 +176,10 @@ struct dns_api_msg { }; #endif /* LWIP_DNS */ +#if LWIP_TCP +extern u8_t netconn_aborted; +#endif /* LWIP_TCP */ + void lwip_netconn_do_newconn (void *m); void lwip_netconn_do_delconn (void *m); void lwip_netconn_do_bind (void *m);