From 47f55b02bf9f889eb38379a68de9d765c163b89a Mon Sep 17 00:00:00 2001 From: goldsimon Date: Thu, 3 Aug 2017 22:28:22 +0200 Subject: [PATCH] Finally fix bug #50088 (socket/netconn: data before RST should be readable) and added a unit test for it --- src/api/api_lib.c | 24 +++--- test/unit/api/test_sockets.c | 145 +++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 9 deletions(-) diff --git a/src/api/api_lib.c b/src/api/api_lib.c index aced5aab..f93a9b78 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -524,25 +524,30 @@ netconn_recv_data(struct netconn *conn, void **new_buf, u8_t apiflags) { void *buf = NULL; u16_t len; - err_t err; LWIP_ERROR("netconn_recv: invalid pointer", (new_buf != NULL), return ERR_ARG;); *new_buf = NULL; LWIP_ERROR("netconn_recv: invalid conn", (conn != NULL), return ERR_ARG;); - err = netconn_err(conn); - if (err != ERR_OK) { - /* return pending error */ - return err; - } if (!sys_mbox_valid(&conn->recvmbox)) { - return ERR_CLSD; + err_t err = netconn_err(conn); + if (err != ERR_OK) { + /* return pending error */ + return err; + } + return ERR_CONN; } - if (netconn_is_nonblocking(conn) || (apiflags & NETCONN_DONTBLOCK) || (conn->flags & NETCONN_FLAG_MBOXCLOSED)) { + if (netconn_is_nonblocking(conn) || (apiflags & NETCONN_DONTBLOCK) || + (conn->flags & NETCONN_FLAG_MBOXCLOSED) || (conn->pending_err != ERR_OK)) { if (sys_arch_mbox_tryfetch(&conn->recvmbox, &buf) == SYS_ARCH_TIMEOUT) { + err_t err = netconn_err(conn); + if (err != ERR_OK) { + /* return pending error */ + return err; + } if (conn->flags & NETCONN_FLAG_MBOXCLOSED) { - return ERR_CLSD; + return ERR_CONN; } return ERR_WOULDBLOCK; } @@ -561,6 +566,7 @@ netconn_recv_data(struct netconn *conn, void **new_buf, u8_t apiflags) if (NETCONNTYPE_GROUP(conn->type) == NETCONN_TCP) #endif /* (LWIP_UDP || LWIP_RAW) */ { + err_t err; /* Check if this is an error message or a pbuf */ if (lwip_netconn_is_err_msg(buf, &err)) { /* new_buf has been zeroed above already */ diff --git a/test/unit/api/test_sockets.c b/test/unit/api/test_sockets.c index 4972f28a..f8bfa9f8 100644 --- a/test/unit/api/test_sockets.c +++ b/test/unit/api/test_sockets.c @@ -8,6 +8,7 @@ #include "lwip/tcpip.h" #include "lwip/priv/tcp_priv.h" +#include "lwip/api.h" static int @@ -641,6 +642,149 @@ START_TEST(test_sockets_select) } END_TEST +START_TEST(test_sockets_recv_after_rst) +{ + int sl, sact; + int spass = -1; + int ret; + struct sockaddr_in sa_listen; + const u16_t port = 1234; + int arg; + const char txbuf[] = "something"; + char rxbuf[16]; + struct lwip_sock *sact_sock; + int err; + LWIP_UNUSED_ARG(_i); + + fail_unless(test_sockets_get_used_count() == 0); + + memset(&sa_listen, 0, sizeof(sa_listen)); + sa_listen.sin_family = AF_INET; + sa_listen.sin_port = PP_HTONS(port); + sa_listen.sin_addr.s_addr = PP_HTONL(INADDR_LOOPBACK); + + /* set up the listener */ + sl = lwip_socket(AF_INET, SOCK_STREAM, 0); + fail_unless(sl >= 0); + fail_unless(test_sockets_get_used_count() == 0); + + ret = lwip_bind(sl, (struct sockaddr *)&sa_listen, sizeof(sa_listen)); + fail_unless(ret == 0); + ret = lwip_listen(sl, 0); + fail_unless(ret == 0); + + /* set up the client */ + sact = lwip_socket(AF_INET, SOCK_STREAM, 0); + fail_unless(sact >= 0); + fail_unless(test_sockets_get_used_count() == 0); + /* set the client to nonblocking to simplify this test */ + arg = 1; + ret = lwip_ioctl(sact, FIONBIO, &arg); + fail_unless(ret == 0); + /* connect */ + do { + ret = lwip_connect(sact, (struct sockaddr *)&sa_listen, sizeof(sa_listen)); + err = errno; + fail_unless((ret == 0) || (ret == -1)); + if (ret != 0) { + if (err == EISCONN) { + /* Although this is not valid, use EISCONN as an indicator for successful connection. + This marks us as "connect phase is done". On error, we would either have a different + errno code or "send" fails later... -> good enough for this test. */ + ret = 0; + } else { + fail_unless(err == EINPROGRESS); + if (err != EINPROGRESS) { + goto cleanup; + } + /* we're in progress: little side check: test for EALREADY */ + ret = lwip_connect(sact, (struct sockaddr *)&sa_listen, sizeof(sa_listen)); + err = errno; + fail_unless(ret == -1); + fail_unless(err == EALREADY); + if ((ret != -1) || (err != EALREADY)) { + goto cleanup; + } + } + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + } + } while (ret != 0); + fail_unless(ret == 0); + + /* accept the server connection part */ + spass = lwip_accept(sl, NULL, NULL); + fail_unless(spass >= 0); + + /* write data from client */ + ret = lwip_send(sact, txbuf, sizeof(txbuf), 0); + fail_unless(ret == sizeof(txbuf)); + + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + + /* issue RST (This is a HACK, don't try this in your own app!) */ + sact_sock = lwip_socket_dbg_get_socket(sact); + fail_unless(sact_sock != NULL); + if (sact_sock != NULL) { + struct netconn *sact_conn = sact_sock->conn; + fail_unless(sact_conn != NULL); + if (sact_conn != NULL) { + struct tcp_pcb *pcb = sact_conn->pcb.tcp; + fail_unless(pcb != NULL); + if (pcb != NULL) { + tcp_rst(pcb, pcb->snd_nxt, pcb->rcv_nxt, &pcb->local_ip, &pcb->remote_ip, + pcb->local_port, pcb->remote_port); + } + } + } + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + + /* expect to receive data first */ + ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0); + fail_unless(ret > 0); + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + + /* expect to receive RST indication */ + ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0); + fail_unless(ret == -1); + err = errno; + fail_unless(err == ECONNRESET); + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + + /* expect to receive ENOTCONN indication */ + ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0); + fail_unless(ret == -1); + err = errno; + fail_unless(err == ENOTCONN); + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + + /* expect to receive ENOTCONN indication */ + ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0); + fail_unless(ret == -1); + err = errno; + fail_unless(err == ENOTCONN); + tcpip_thread_poll_one(); + tcpip_thread_poll_one(); + +cleanup: + ret = lwip_close(sl); + fail_unless(ret == 0); + ret = lwip_close(sact); + fail_unless(ret == 0); + if (spass >= 0) { + ret = lwip_close(spass); + fail_unless(ret == 0); + } +} +END_TEST + /** Create the suite including all tests for this module */ Suite * sockets_suite(void) @@ -650,6 +794,7 @@ sockets_suite(void) TESTFUNC(test_sockets_allfunctions_basic), TESTFUNC(test_sockets_msgapis), TESTFUNC(test_sockets_select), + TESTFUNC(test_sockets_recv_after_rst), }; return create_suite("SOCKETS", tests, sizeof(tests)/sizeof(testfunc), sockets_setup, sockets_teardown); }