From 113a52d091c5cb4798924a4944c43cd7328a71f5 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Sun, 2 Dec 2007 14:53:50 +0000 Subject: [PATCH] fix bug #21656 (recvmbox problem in netconn API): always allocate a recvmbox in netconn_new_with_proto_and_callback. For a tcp-listen netconn, this recvmbox is later freed and a new mbox is allocated for acceptmbox. This is a fix for thread-safety and allocates all items needed for a netconn when the netconn is created. --- CHANGELOG | 7 +++++++ src/api/api_lib.c | 28 +++++++++------------------- src/api/api_msg.c | 6 ++++++ 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4235ce91..df9d1906 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -508,6 +508,13 @@ HISTORY ++ Bug fixes: + 2007-12-02 Simon Goldschmidt + * api_msg.c, api_lib.c: fix bug #21656 (recvmbox problem in netconn API): always + allocate a recvmbox in netconn_new_with_proto_and_callback. For a tcp-listen + netconn, this recvmbox is later freed and a new mbox is allocated for acceptmbox. + This is a fix for thread-safety and allocates all items needed for a netconn + when the netconn is created. + 2007-11-30 Simon Goldschmidt * udp.c: first attempt to fix bug #21655 (DHCP doesn't work reliably with multiple netifs): if LWIP_DHCP is enabled, UDP packets to DHCP_CLIENT_PORT are passed diff --git a/src/api/api_lib.c b/src/api/api_lib.c index 9bedc1a9..3bd6be24 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -80,6 +80,9 @@ netconn_new_with_proto_and_callback(enum netconn_type t, u8_t proto, if (conn->err != ERR_OK) { LWIP_ASSERT("freeing conn without freeing pcb", conn->pcb.tcp == NULL); + LWIP_ASSERT("conn has no mbox", conn->mbox != SYS_MBOX_NULL); + LWIP_ASSERT("conn has no recvmbox", conn->recvmbox != SYS_MBOX_NULL); + LWIP_ASSERT("conn->acceptmbox shouldn't exist", conn->acceptmbox != SYS_MBOX_NULL); sys_mbox_free(conn->mbox); sys_mbox_free(conn->recvmbox); memp_free(MEMP_NETCONN, conn); @@ -116,8 +119,9 @@ netconn_delete(struct netconn *conn) if (conn->recvmbox != SYS_MBOX_NULL) { while (sys_mbox_tryfetch(conn->recvmbox, &mem) != SYS_MBOX_EMPTY) { if (conn->type == NETCONN_TCP) { - if(mem != NULL) + if(mem != NULL) { pbuf_free((struct pbuf *)mem); + } } else { netbuf_delete((struct netbuf *)mem); } @@ -202,12 +206,6 @@ netconn_bind(struct netconn *conn, struct ip_addr *addr, u16_t port) LWIP_ERROR("netconn_bind: invalid conn", (conn != NULL), return ERR_ARG;); - if (conn->type != NETCONN_TCP && conn->recvmbox == SYS_MBOX_NULL) { - if ((conn->recvmbox = sys_mbox_new()) == SYS_MBOX_NULL) { - return ERR_MEM; - } - } - msg.function = do_bind; msg.msg.conn = conn; msg.msg.msg.bc.ipaddr = addr; @@ -231,12 +229,6 @@ netconn_connect(struct netconn *conn, struct ip_addr *addr, u16_t port) LWIP_ERROR("netconn_connect: invalid conn", (conn != NULL), return ERR_ARG;); - if (conn->recvmbox == SYS_MBOX_NULL) { - if ((conn->recvmbox = sys_mbox_new()) == SYS_MBOX_NULL) { - return ERR_MEM; - } - } - msg.function = do_connect; msg.msg.conn = conn; msg.msg.msg.bc.ipaddr = addr; @@ -330,12 +322,10 @@ netconn_recv(struct netconn *conn) LWIP_ERROR("netconn_recv: invalid conn", (conn != NULL), return NULL;); if (conn->recvmbox == SYS_MBOX_NULL) { - if ((conn->recvmbox = sys_mbox_new()) == SYS_MBOX_NULL) { - /* @todo: should calling netconn_recv on a TCP listen conn be fatal?? */ - /* TCP listen conn don't have a recvmbox! */ - conn->err = ERR_CONN; - return NULL; - } + /* @todo: should calling netconn_recv on a TCP listen conn be fatal (ERR_CONN)?? */ + /* TCP listen conns don't have a recvmbox! */ + conn->err = ERR_CONN; + return NULL; } if (ERR_IS_FATAL(conn->err)) { diff --git a/src/api/api_msg.c b/src/api/api_msg.c index d9f9802c..9c81819b 100644 --- a/src/api/api_msg.c +++ b/src/api/api_msg.c @@ -713,6 +713,12 @@ do_listen(struct api_msg_msg *msg) if (lpcb == NULL) { msg->conn->err = ERR_MEM; } else { + /* delete the recvmbox and allocate the acceptmbox */ + if (msg->conn->recvmbox != SYS_MBOX_NULL) { + /* @todo: should we drain the recvmbox here? */ + sys_mbox_free(msg->conn->recvmbox); + msg->conn->recvmbox = NULL; + } if (msg->conn->acceptmbox == SYS_MBOX_NULL) { if ((msg->conn->acceptmbox = sys_mbox_new()) == SYS_MBOX_NULL) { msg->conn->err = ERR_MEM;