From d0da910b19e05bfbfe87a29bc72b4c7f4505e212 Mon Sep 17 00:00:00 2001 From: Jeremy Lakeman Date: Mon, 25 Jan 2016 17:02:21 +1030 Subject: [PATCH] Avoid undefined behaviour as highlighted by clang --- conf.h | 6 +++--- keyring.c | 7 +++---- os.c | 2 +- overlay_buffer.c | 7 ++++--- overlay_interface.c | 1 + overlay_mdp_services.c | 2 +- overlay_queue.c | 2 +- strbuf_helpers.c | 9 +++++---- tests/meshms | 1 + 9 files changed, 20 insertions(+), 17 deletions(-) diff --git a/conf.h b/conf.h index 7f81ae10..e9454952 100644 --- a/conf.h +++ b/conf.h @@ -251,8 +251,8 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #define CFUNSUPPORTED (1<<7) #define CF__SUB_SHIFT 16 #define CFSUB(f) ((f) << CF__SUB_SHIFT) -#define CF__SUBFLAGS CFSUB(~0) -#define CF__FLAGS (~0 & ~CF__SUBFLAGS) +#define CF__FLAGS (0xFFFF) +#define CF__SUBFLAGS (~CF__FLAGS) strbuf strbuf_cf_flags(strbuf, int); strbuf strbuf_cf_flag_reason(strbuf sb, int flags); @@ -268,7 +268,7 @@ struct cf_om_node { const char *key; // points inside fullkey, do not free() const char *text; // malloc() size_t nodc; - struct cf_om_node *nodv[10]; // malloc() + struct cf_om_node *nodv[0]; // malloc() }; int is_configvarname(const char *); diff --git a/keyring.c b/keyring.c index d5b41804..75015bf4 100644 --- a/keyring.c +++ b/keyring.c @@ -1038,7 +1038,6 @@ static keyring_identity *keyring_unpack_identity(unsigned char *slot, const char unsigned char ktype = rotbuf_getc(&rbuf); if (rbuf.wrap || ktype == 0x00) break; // End of data, stop looking - const struct keytype *kt = &keytypes[ktype]; size_t keypair_len; // No length bytes after the original four key types, for backward compatibility. All other key // types are followed by a two-byte keypair length. @@ -1047,7 +1046,7 @@ static keyring_identity *keyring_unpack_identity(unsigned char *slot, const char case KEYTYPE_CRYPTOSIGN: case KEYTYPE_RHIZOME: case KEYTYPE_DID: - keypair_len = kt->packed_size; + keypair_len = keytypes[ktype].packed_size; break; default: keypair_len = rotbuf_getc(&rbuf) << 8; @@ -1067,9 +1066,9 @@ static keyring_identity *keyring_unpack_identity(unsigned char *slot, const char return NULL; } struct rotbuf rbstart = rbuf; - if (ktype < NELS(keytypes) && kt->unpacker) { + if (ktype < NELS(keytypes) && keytypes[ktype].unpacker) { DEBUGF(keyring, "unpack key type = 0x%02x(%s) at offset %u", ktype, keytype_str(ktype, "unknown"), (int)rotbuf_position(&rbo)); - if (kt->unpacker(kp, &rbuf, keypair_len) != 0) { + if (keytypes[ktype].unpacker(kp, &rbuf, keypair_len) != 0) { // If there is an error, it is probably an empty slot. DEBUGF(keyring, "key type 0x%02x does not unpack", ktype); keyring_free_keypair(kp); diff --git a/os.c b/os.c index 184352bb..ae05cce1 100644 --- a/os.c +++ b/os.c @@ -182,7 +182,7 @@ time_ms_t sleep_ms(time_ms_t milliseconds) delay.tv_nsec = (milliseconds % 1000) * 1000000; if (nanosleep(&delay, &remain) == -1 && errno != EINTR) FATALF_perror("nanosleep(tv_sec=%ld, tv_nsec=%ld)", delay.tv_sec, delay.tv_nsec); - return remain.tv_sec * 1000 + remain.tv_nsec / 1000000; + return remain.tv_sec * 1000ull + remain.tv_nsec / 1000000; } struct timeval time_ms_to_timeval(time_ms_t milliseconds) diff --git a/overlay_buffer.c b/overlay_buffer.c index 1dcc84b0..9717afd6 100644 --- a/overlay_buffer.c +++ b/overlay_buffer.c @@ -219,7 +219,8 @@ ssize_t _ob_makespace(struct __sourceloc __whence, struct overlay_buffer *b, siz #endif if (!new) return 0; - bcopy(b->bytes,new,b->position); + if (b->position) + bcopy(b->bytes,new,b->position); if (b->allocated) { assert(b->allocated == b->bytes); free(b->allocated); @@ -490,7 +491,7 @@ uint32_t ob_get_ui32(struct overlay_buffer *b) { if (test_offset(b, 4)) return 0xFFFFFFFF; // ... unsigned - uint32_t ret = b->bytes[b->position] << 24 + uint32_t ret = (unsigned)b->bytes[b->position] << 24 | b->bytes[b->position +1] << 16 | b->bytes[b->position +2] << 8 | b->bytes[b->position +3]; @@ -505,7 +506,7 @@ uint32_t ob_get_ui32_rv(struct overlay_buffer *b) uint32_t ret = b->bytes[b->position] | b->bytes[b->position +1] << 8 | b->bytes[b->position +2] << 16 - | b->bytes[b->position +3] << 24; + | (unsigned)b->bytes[b->position +3] << 24; b->position+=4; return ret; } diff --git a/overlay_interface.c b/overlay_interface.c index 154da9d0..a394d51f 100644 --- a/overlay_interface.c +++ b/overlay_interface.c @@ -1282,6 +1282,7 @@ static int netlink_send_get() struct { struct nlmsghdr n; struct ifaddrmsg r; + uint8_t alignment_padding[64]; } req; memset(&req, 0, sizeof(req)); diff --git a/overlay_mdp_services.c b/overlay_mdp_services.c index 5e7ba762..e463af2c 100644 --- a/overlay_mdp_services.c +++ b/overlay_mdp_services.c @@ -78,7 +78,7 @@ int rhizome_mdp_send_block(struct subscriber *dest, const rhizome_bid_t *bid, ui int i; for(i=0;i<32;i++){ - if (bitmap&(1<<(31-i))) + if (bitmap&(1u<<(31-i))) continue; if (overlay_queue_remaining(header.qos) < 10) diff --git a/overlay_queue.c b/overlay_queue.c index 58298b50..3fd0d8ba 100644 --- a/overlay_queue.c +++ b/overlay_queue.c @@ -610,7 +610,7 @@ int overlay_queue_ack(struct subscriber *neighbour, struct network_destination * int frame_seq = frame->destinations[j].sent_sequence; if (frame_seq >=0 && (frame->destinations[j].next_hop == neighbour || !frame->destination)){ int seq_delta = (ack_seq - frame_seq)&0xFF; - char acked = (seq_delta==0 || (seq_delta <= 32 && ack_mask&(1<<(seq_delta-1))))?1:0; + char acked = (seq_delta==0 || (seq_delta <= 32 && ack_mask&((uint32_t)1<<(seq_delta-1))))?1:0; if (acked){ int this_rtt = now - frame->destinations[j].transmit_time; diff --git a/strbuf_helpers.c b/strbuf_helpers.c index dcb9d26e..8d9e449a 100644 --- a/strbuf_helpers.c +++ b/strbuf_helpers.c @@ -474,17 +474,18 @@ strbuf strbuf_append_sockaddr(strbuf sb, const struct sockaddr *addr, socklen_t { switch (addr->sa_family) { case AF_UNIX: { + struct sockaddr_un *addr_un = (struct sockaddr_un *)addr; strbuf_puts(sb, "AF_UNIX:"); size_t len = addrlen > sizeof addr->sa_family ? addrlen - sizeof addr->sa_family : 0; - if (addr->sa_data[0]) { - strbuf_toprint_quoted_len(sb, "\"\"", addr->sa_data, len); + if (addr_un->sun_path[0]) { + strbuf_toprint_quoted_len(sb, "\"\"", addr_un->sun_path, len); if (len < 2) strbuf_sprintf(sb, " (addrlen=%d too short)", (int)addrlen); - if (len == 0 || addr->sa_data[len - 1] != '\0') + if (len == 0 || addr_un->sun_path[len - 1] != '\0') strbuf_sprintf(sb, " (addrlen=%d, no nul terminator)", (int)addrlen); } else { strbuf_puts(sb, "abstract "); - strbuf_toprint_quoted_len(sb, "\"\"", addr->sa_data, len); + strbuf_toprint_quoted_len(sb, "\"\"", addr_un->sun_path, len); if (len == 0) strbuf_sprintf(sb, " (addrlen=%d too short)", (int)addrlen); } diff --git a/tests/meshms b/tests/meshms index 3266d5f7..e1b7b38c 100755 --- a/tests/meshms +++ b/tests/meshms @@ -28,6 +28,7 @@ teardown() { stop_all_servald_servers kill_all_servald_processes assert_no_servald_processes + report_all_servald_servers } setup_logging() {