From 1d21bc41451a6b38dccfc91df27ff91c4425059a Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 25 Jul 2017 10:43:47 -0700 Subject: [PATCH 1/4] zts_get_pico_socket needs to pass indirect pointer --- include/libzt.h | 2 +- src/libzt.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/libzt.h b/include/libzt.h index 595eaa2..ad14400 100644 --- a/include/libzt.h +++ b/include/libzt.h @@ -413,7 +413,7 @@ namespace ZeroTier /* * Gets a pointer to a pico_socket given a file descriptor */ -int zts_get_pico_socket(int fd, struct pico_socket *s); +int zts_get_pico_socket(int fd, struct pico_socket **s); /** * Returns the number of sockets either already provisioned or waiting to be diff --git a/src/libzt.cpp b/src/libzt.cpp index c19f1e2..ad3497d 100644 --- a/src/libzt.cpp +++ b/src/libzt.cpp @@ -868,8 +868,8 @@ int zts_setsockopt(ZT_SETSOCKOPT_SIG) } // Disable Nagle's algorithm - struct pico_socket *p; - err = zts_get_pico_socket(fd, p); + struct pico_socket *p = NULL; + err = zts_get_pico_socket(fd, &p); if(p) { int value = 1; if((err = pico_socket_setoption(p, PICO_TCP_NODELAY, &value)) < 0) { @@ -1501,7 +1501,7 @@ namespace ZeroTier { /* SDK Socket API Helper functions --- DON'T CALL THESE DIRECTLY */ /****************************************************************************/ -int zts_get_pico_socket(int fd, struct pico_socket *s) +int zts_get_pico_socket(int fd, struct pico_socket **s) { int err = 0; if(!zt1Service) { @@ -1518,7 +1518,7 @@ int zts_get_pico_socket(int fd, struct pico_socket *s) // during closure - it isn't yet stitched into the clockwork if(conn) { - s = conn->picosock; + *s = conn->picosock; return 1; // unassigned } else // assigned @@ -1532,7 +1532,7 @@ int zts_get_pico_socket(int fd, struct pico_socket *s) } else // found everything, begin closure { - s = p->first->picosock; + *s = p->first->picosock; return 0; } } From 8a6d4820cc8f99d541a66430cf5df019aebc521b Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 25 Jul 2017 10:47:13 -0700 Subject: [PATCH 2/4] Debug buffer address returned from stack should be static. --- src/picoTCP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/picoTCP.cpp b/src/picoTCP.cpp index eeb3e99..957fc4b 100644 --- a/src/picoTCP.cpp +++ b/src/picoTCP.cpp @@ -859,7 +859,7 @@ namespace ZeroTier { */ char *picoTCP::beautify_pico_state(int state) { - char state_str[512]; + static char state_str[512]; char *str_ptr = state_str; if(state & PICO_SOCKET_STATE_UNDEFINED) { From 24fa0c9a6cf1e277232be85192b29fa4fb13031d Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 25 Jul 2017 11:03:02 -0700 Subject: [PATCH 3/4] In write, buf_w was unchecked. We already checked for room, so the buffer should have room. We make this a kind of assertion; this silences compiler warnings. Later, if this becomes a thread-level race condition, come back and actually use buf_w more meaningfully to handle partial writes. --- src/picoTCP.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/picoTCP.cpp b/src/picoTCP.cpp index 957fc4b..94e98bd 100644 --- a/src/picoTCP.cpp +++ b/src/picoTCP.cpp @@ -746,6 +746,11 @@ namespace ZeroTier { } int buf_w = conn->TXbuf->write((const unsigned char*)data, len); + if (buf_w != len) { + // because we checked ZT_TCP_TX_BUF_SZ above, this should not happen + DEBUG_ERROR("TX wrote only %d but expected to write %d", buf_w, len); + exit(0); + } //DEBUG_INFO("TXbuf->count() = %d", conn->TXbuf->count()); int txsz = conn->TXbuf->count(); From a31f81a34d954740067f8c9034d6b18c70408241 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 25 Jul 2017 11:14:29 -0700 Subject: [PATCH 4/4] Silence an aligned access warning. clang is a bit too strict about validating address alignments when using packed structs. The only member is a 32-bit value, so the alignment was correct, but this approach is "correct" and guaranteed to work even if the structure was not aligned, at the cost of an extra temporary variable and 32-bit copy. --- src/picoTCP.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/picoTCP.cpp b/src/picoTCP.cpp index 94e98bd..c1424a1 100644 --- a/src/picoTCP.cpp +++ b/src/picoTCP.cpp @@ -552,11 +552,13 @@ namespace ZeroTier { int err = 0; if(conn->socket_family == AF_INET) { struct pico_ip4 zaddr; + uint32_t tempaddr; memset(&zaddr, 0, sizeof (struct pico_ip4)); struct sockaddr_in *in4 = (struct sockaddr_in*)addr; char ipv4_str[INET_ADDRSTRLEN]; inet_ntop(AF_INET, (const void *)&in4->sin_addr.s_addr, ipv4_str, INET_ADDRSTRLEN); - pico_string_to_ipv4(ipv4_str, &(zaddr.addr)); + pico_string_to_ipv4(ipv4_str, &tempaddr); + zaddr.addr = tempaddr; //DEBUG_EXTRA("addr=%s:%d", ipv4_str, Utils::ntoh(in4->sin_port)); err = pico_socket_bind(conn->picosock, &zaddr, (uint16_t *)&(in4->sin_port)); }