From 380f6048e0bbc762f12fa50b57b73cf29049f967 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Tue, 27 Oct 2020 16:36:00 +0200 Subject: [PATCH] Fix cluster access to unaligned memory (SIGBUS on old ARM) (#7958) Turns out this was broken since version 4.0 when we added sds size classes. The cluster code uses sds for the receive buffer, and then casts it to a struct and accesses a 64 bit variable. This commit replaces the use of sds with a simple reallocated buffer. --- src/cluster.c | 29 ++++++++++++++++++++++------- src/cluster.h | 4 +++- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 6da71c5da..6116614ea 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -77,6 +77,9 @@ uint64_t clusterGetMaxEpoch(void); int clusterBumpConfigEpochWithoutConsensus(void); void moduleCallClusterReceivers(const char *sender_id, uint64_t module_id, uint8_t type, const unsigned char *payload, uint32_t len); +#define RCVBUF_INIT_LEN 1024 +#define RCVBUF_MAX_PREALLOC (1<<20) /* 1MB */ + /* ----------------------------------------------------------------------------- * Initialization * -------------------------------------------------------------------------- */ @@ -613,7 +616,8 @@ clusterLink *createClusterLink(clusterNode *node) { clusterLink *link = zmalloc(sizeof(*link)); link->ctime = mstime(); link->sndbuf = sdsempty(); - link->rcvbuf = sdsempty(); + link->rcvbuf = zmalloc(link->rcvbuf_alloc = RCVBUF_INIT_LEN); + link->rcvbuf_len = 0; link->node = node; link->conn = NULL; return link; @@ -628,7 +632,7 @@ void freeClusterLink(clusterLink *link) { link->conn = NULL; } sdsfree(link->sndbuf); - sdsfree(link->rcvbuf); + zfree(link->rcvbuf); if (link->node) link->node->link = NULL; zfree(link); @@ -1728,7 +1732,7 @@ int clusterProcessPacket(clusterLink *link) { /* Perform sanity checks */ if (totlen < 16) return 1; /* At least signature, version, totlen, count. */ - if (totlen > sdslen(link->rcvbuf)) return 1; + if (totlen > link->rcvbuf_len) return 1; if (ntohs(hdr->ver) != CLUSTER_PROTO_VER) { /* Can't handle messages of different versions. */ @@ -2289,7 +2293,7 @@ void clusterReadHandler(connection *conn) { unsigned int readlen, rcvbuflen; while(1) { /* Read as long as there is data to read. */ - rcvbuflen = sdslen(link->rcvbuf); + rcvbuflen = link->rcvbuf_len; if (rcvbuflen < 8) { /* First, obtain the first 8 bytes to get the full message * length. */ @@ -2325,7 +2329,15 @@ void clusterReadHandler(connection *conn) { return; } else { /* Read data and recast the pointer to the new buffer. */ - link->rcvbuf = sdscatlen(link->rcvbuf,buf,nread); + size_t unused = link->rcvbuf_alloc - link->rcvbuf_len; + if ((size_t)nread > unused) { + size_t required = link->rcvbuf_len + nread; + /* If less than 1mb, grow to twice the needed size, if larger grow by 1mb. */ + link->rcvbuf_alloc = required < RCVBUF_MAX_PREALLOC ? required * 2: required + RCVBUF_MAX_PREALLOC; + link->rcvbuf = zrealloc(link->rcvbuf, link->rcvbuf_alloc); + } + memcpy(link->rcvbuf + link->rcvbuf_len, buf, nread); + link->rcvbuf_len += nread; hdr = (clusterMsg*) link->rcvbuf; rcvbuflen += nread; } @@ -2333,8 +2345,11 @@ void clusterReadHandler(connection *conn) { /* Total length obtained? Process this packet. */ if (rcvbuflen >= 8 && rcvbuflen == ntohl(hdr->totlen)) { if (clusterProcessPacket(link)) { - sdsfree(link->rcvbuf); - link->rcvbuf = sdsempty(); + if (link->rcvbuf_alloc > RCVBUF_INIT_LEN) { + zfree(link->rcvbuf); + link->rcvbuf = zmalloc(link->rcvbuf_alloc = RCVBUF_INIT_LEN); + } + link->rcvbuf_len = 0; } else { return; /* Link no longer valid. */ } diff --git a/src/cluster.h b/src/cluster.h index 95b710383..7e5f79c87 100644 --- a/src/cluster.h +++ b/src/cluster.h @@ -38,7 +38,9 @@ typedef struct clusterLink { mstime_t ctime; /* Link creation time */ connection *conn; /* Connection to remote node */ sds sndbuf; /* Packet send buffer */ - sds rcvbuf; /* Packet reception buffer */ + char *rcvbuf; /* Packet reception buffer */ + size_t rcvbuf_len; /* Used size of rcvbuf */ + size_t rcvbuf_alloc; /* Used size of rcvbuf */ struct clusterNode *node; /* Node related to this link if any, or NULL */ } clusterLink;