From 87e16a8f47e57065adc1bbbf196ff03be7f48da9 Mon Sep 17 00:00:00 2001 From: fbernon Date: Sun, 7 Oct 2007 17:26:54 +0000 Subject: [PATCH] sockets.c, api.h, api_lib.c: First step to fix "bug #20900 : Potential crash error problem with netconn_peer & netconn_addr". VERY IMPORTANT: this change cause an API breakage for netconn_peer, since a parameter type change. Any compiler should cause an error without any changes in yours netconn_peer calls (so, it can't be a "silent change"). It also reduce a little bit the footprint for socket layer (lwip_getpeername & lwip_getsockname use now a common lwip_getaddrname function since netconn_peer & netconn_addr have the same parameters). --- CHANGELOG | 10 +++++++ src/api/api_lib.c | 33 +++++++++++++-------- src/api/sockets.c | 67 +++++++++++++++++------------------------- src/include/lwip/api.h | 2 +- 4 files changed, 59 insertions(+), 53 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 63407654..4185e37c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -424,6 +424,16 @@ HISTORY ++ Bug fixes: + 2007-10-07 Frédéric Bernon + * sockets.c, api.h, api_lib.c: First step to fix "bug #20900 : Potential + crash error problem with netconn_peer & netconn_addr". VERY IMPORTANT: + this change cause an API breakage for netconn_peer, since a parameter + type change. Any compiler should cause an error without any changes in + yours netconn_peer calls (so, it can't be a "silent change"). It also + reduce a little bit the footprint for socket layer (lwip_getpeername & + lwip_getsockname use now a common lwip_getaddrname function since + netconn_peer & netconn_addr have the same parameters). + 2007-09-20 Simon Goldschmidt * tcp.c: Fixed bug #21080 (tcp_bind without check pcbs in TIME_WAIT state) by checking tcp_tw_pcbs also diff --git a/src/api/api_lib.c b/src/api/api_lib.c index ec1bd746..c0a93b66 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -186,6 +186,15 @@ err_t netconn_peer(struct netconn *conn, struct ip_addr *addr, u16_t *port) { LWIP_ERROR("netconn_peer: invalid conn", (conn != NULL), return ERR_ARG;); + LWIP_ERROR("netconn_peer: invalid addr", (addr != NULL), return ERR_ARG;); + LWIP_ERROR("netconn_peer: invalid port", (port != NULL), return ERR_ARG;); + + /* Not a good safe-thread protection, will be improved */ + if (conn->pcb.ip == NULL) + return ERR_CONN; + + *addr = conn->pcb.ip->remote_ip; + switch (NETCONNTYPE_GROUP(conn->type)) { #if LWIP_RAW case NETCONN_RAW: @@ -194,18 +203,13 @@ netconn_peer(struct netconn *conn, struct ip_addr *addr, u16_t *port) #endif /* LWIP_RAW */ #if LWIP_UDP case NETCONN_UDP: - if (conn->pcb.udp == NULL || - ((conn->pcb.udp->flags & UDP_FLAGS_CONNECTED) == 0)) - return ERR_CONN; - *addr = (conn->pcb.udp->remote_ip); + if ((conn->pcb.udp->flags & UDP_FLAGS_CONNECTED) == 0) + return ERR_CONN; *port = conn->pcb.udp->remote_port; break; #endif /* LWIP_UDP */ #if LWIP_TCP case NETCONN_TCP: - if (conn->pcb.tcp == NULL) - return ERR_CONN; - *addr = (conn->pcb.tcp->remote_ip); *port = conn->pcb.tcp->remote_port; break; #endif /* LWIP_TCP */ @@ -220,30 +224,35 @@ netconn_peer(struct netconn *conn, struct ip_addr *addr, u16_t *port) * @param conn the netconn to query * @param addr a pointer to which to save the local IP address * @param port a pointer to which to save the local port (or protocol for RAW) - * @return ERR_OK if the information was retrieved + * @return ERR_CONN for invalid connections + * ERR_OK if the information was retrieved */ err_t -netconn_addr(struct netconn *conn, struct ip_addr **addr, u16_t *port) +netconn_addr(struct netconn *conn, struct ip_addr *addr, u16_t *port) { LWIP_ERROR("netconn_addr: invalid conn", (conn != NULL), return ERR_ARG;); LWIP_ERROR("netconn_addr: invalid addr", (addr != NULL), return ERR_ARG;); LWIP_ERROR("netconn_addr: invalid port", (port != NULL), return ERR_ARG;); + + /* Not a good safe-thread protection, will be improved */ + if (conn->pcb.ip == NULL) + return ERR_CONN; + + *addr = conn->pcb.ip->local_ip; + switch (NETCONNTYPE_GROUP(conn->type)) { #if LWIP_RAW case NETCONN_RAW: - *addr = &(conn->pcb.raw->local_ip); *port = conn->pcb.raw->protocol; break; #endif /* LWIP_RAW */ #if LWIP_UDP case NETCONN_UDP: - *addr = &(conn->pcb.udp->local_ip); *port = conn->pcb.udp->local_port; break; #endif /* LWIP_UDP */ #if LWIP_TCP case NETCONN_TCP: - *addr = &(conn->pcb.tcp->local_ip); *port = conn->pcb.tcp->local_port; break; #endif /* LWIP_TCP */ diff --git a/src/api/sockets.c b/src/api/sockets.c index f61cf5a8..cf6c21a6 100644 --- a/src/api/sockets.c +++ b/src/api/sockets.c @@ -957,14 +957,17 @@ event_callback(struct netconn *conn, enum netconn_evt evt, u16_t len) } } -int lwip_shutdown(int s, int how) +int +lwip_shutdown(int s, int how) { LWIP_UNUSED_ARG(how); LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_shutdown(%d, how=%d)\n", s, how)); return lwip_close(s); /* XXX temporary hack until proper implementation */ } -int lwip_getpeername(int s, struct sockaddr *name, socklen_t *namelen) +static int +lwip_getaddrname(int s, struct sockaddr *name, socklen_t *namelen, + err_t (* netconn_addrfunc)(struct netconn *conn, struct ip_addr *addr, u16_t *port)) { struct lwip_socket *sock; struct sockaddr_in sin; @@ -978,10 +981,10 @@ int lwip_getpeername(int s, struct sockaddr *name, socklen_t *namelen) sin.sin_len = sizeof(sin); sin.sin_family = AF_INET; - /* get the IP address and port of the remote host */ - netconn_peer(sock->conn, &naddr, &sin.sin_port); + /* get the IP address and port */ + netconn_addrfunc(sock->conn, &naddr, &sin.sin_port); - LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_getpeername(%d, addr=", s)); + LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_getaddrname(%d, addr=", s)); ip_addr_debug_print(SOCKETS_DEBUG, &naddr); LWIP_DEBUGF(SOCKETS_DEBUG, (" port=%d)\n", sin.sin_port)); @@ -996,39 +999,20 @@ int lwip_getpeername(int s, struct sockaddr *name, socklen_t *namelen) return 0; } -int lwip_getsockname(int s, struct sockaddr *name, socklen_t *namelen) +int +lwip_getpeername(int s, struct sockaddr *name, socklen_t *namelen) { - struct lwip_socket *sock; - struct sockaddr_in sin; - struct ip_addr *naddr; - - sock = get_socket(s); - if (!sock) - return -1; - - memset(&sin, 0, sizeof(sin)); - sin.sin_len = sizeof(sin); - sin.sin_family = AF_INET; - - /* get the IP address and port of the remote host */ - netconn_addr(sock->conn, &naddr, &sin.sin_port); - - LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_getsockname(%d, addr=", s)); - ip_addr_debug_print(SOCKETS_DEBUG, naddr); - LWIP_DEBUGF(SOCKETS_DEBUG, (" port=%d)\n", sin.sin_port)); - - sin.sin_port = htons(sin.sin_port); - sin.sin_addr.s_addr = naddr->addr; - - if (*namelen > sizeof(sin)) - *namelen = sizeof(sin); - - SMEMCPY(name, &sin, *namelen); - sock_set_errno(sock, 0); - return 0; + return lwip_getaddrname(s, name, namelen, netconn_peer); } -int lwip_getsockopt(int s, int level, int optname, void *optval, socklen_t *optlen) +int +lwip_getsockname(int s, struct sockaddr *name, socklen_t *namelen) +{ + return lwip_getaddrname(s, name, namelen, netconn_addr); +} + +int +lwip_getsockopt(int s, int level, int optname, void *optval, socklen_t *optlen) { err_t err = ERR_OK; struct lwip_socket *sock = get_socket(s); @@ -1210,8 +1194,8 @@ int lwip_getsockopt(int s, int level, int optname, void *optval, socklen_t *optl return err ? -1 : 0; } - -static void lwip_getsockopt_internal(void *arg) +static void +lwip_getsockopt_internal(void *arg) { struct lwip_socket *sock; #ifdef LWIP_DEBUG @@ -1384,7 +1368,8 @@ static void lwip_getsockopt_internal(void *arg) sys_mbox_post(sock->conn->mbox, NULL); } -int lwip_setsockopt(int s, int level, int optname, const void *optval, socklen_t optlen) +int +lwip_setsockopt(int s, int level, int optname, const void *optval, socklen_t optlen) { struct lwip_socket *sock = get_socket(s); int err = ERR_OK; @@ -1576,7 +1561,8 @@ int lwip_setsockopt(int s, int level, int optname, const void *optval, socklen_t return err ? -1 : 0; } -static void lwip_setsockopt_internal(void *arg) +static void +lwip_setsockopt_internal(void *arg) { struct lwip_socket *sock; #ifdef LWIP_DEBUG @@ -1752,7 +1738,8 @@ static void lwip_setsockopt_internal(void *arg) sys_mbox_post(sock->conn->mbox, NULL); } -int lwip_ioctl(int s, long cmd, void *argp) +int +lwip_ioctl(int s, long cmd, void *argp) { struct lwip_socket *sock = get_socket(s); u16_t buflen = 0; diff --git a/src/include/lwip/api.h b/src/include/lwip/api.h index 494ee128..bc3a26e3 100644 --- a/src/include/lwip/api.h +++ b/src/include/lwip/api.h @@ -141,7 +141,7 @@ err_t netconn_peer (struct netconn *conn, struct ip_addr *addr, u16_t *port); err_t netconn_addr (struct netconn *conn, - struct ip_addr **addr, + struct ip_addr *addr, u16_t *port); err_t netconn_bind (struct netconn *conn, struct ip_addr *addr,