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.

(cherry picked from commit b71d06c269878887eed85b63a731c3d4ad7a8b12)
This commit is contained in:
Oran Agra 2020-10-27 16:36:00 +02:00
parent 39ac9ba73f
commit df32c5a812
2 changed files with 25 additions and 8 deletions

View File

@ -77,6 +77,9 @@ uint64_t clusterGetMaxEpoch(void);
int clusterBumpConfigEpochWithoutConsensus(void); int clusterBumpConfigEpochWithoutConsensus(void);
void moduleCallClusterReceivers(const char *sender_id, uint64_t module_id, uint8_t type, const unsigned char *payload, uint32_t len); 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 * Initialization
* -------------------------------------------------------------------------- */ * -------------------------------------------------------------------------- */
@ -613,7 +616,8 @@ clusterLink *createClusterLink(clusterNode *node) {
clusterLink *link = zmalloc(sizeof(*link)); clusterLink *link = zmalloc(sizeof(*link));
link->ctime = mstime(); link->ctime = mstime();
link->sndbuf = sdsempty(); link->sndbuf = sdsempty();
link->rcvbuf = sdsempty(); link->rcvbuf = zmalloc(link->rcvbuf_alloc = RCVBUF_INIT_LEN);
link->rcvbuf_len = 0;
link->node = node; link->node = node;
link->conn = NULL; link->conn = NULL;
return link; return link;
@ -628,7 +632,7 @@ void freeClusterLink(clusterLink *link) {
link->conn = NULL; link->conn = NULL;
} }
sdsfree(link->sndbuf); sdsfree(link->sndbuf);
sdsfree(link->rcvbuf); zfree(link->rcvbuf);
if (link->node) if (link->node)
link->node->link = NULL; link->node->link = NULL;
zfree(link); zfree(link);
@ -1728,7 +1732,7 @@ int clusterProcessPacket(clusterLink *link) {
/* Perform sanity checks */ /* Perform sanity checks */
if (totlen < 16) return 1; /* At least signature, version, totlen, count. */ 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) { if (ntohs(hdr->ver) != CLUSTER_PROTO_VER) {
/* Can't handle messages of different versions. */ /* Can't handle messages of different versions. */
@ -2282,7 +2286,7 @@ void clusterReadHandler(connection *conn) {
unsigned int readlen, rcvbuflen; unsigned int readlen, rcvbuflen;
while(1) { /* Read as long as there is data to read. */ while(1) { /* Read as long as there is data to read. */
rcvbuflen = sdslen(link->rcvbuf); rcvbuflen = link->rcvbuf_len;
if (rcvbuflen < 8) { if (rcvbuflen < 8) {
/* First, obtain the first 8 bytes to get the full message /* First, obtain the first 8 bytes to get the full message
* length. */ * length. */
@ -2318,7 +2322,15 @@ void clusterReadHandler(connection *conn) {
return; return;
} else { } else {
/* Read data and recast the pointer to the new buffer. */ /* 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; hdr = (clusterMsg*) link->rcvbuf;
rcvbuflen += nread; rcvbuflen += nread;
} }
@ -2326,8 +2338,11 @@ void clusterReadHandler(connection *conn) {
/* Total length obtained? Process this packet. */ /* Total length obtained? Process this packet. */
if (rcvbuflen >= 8 && rcvbuflen == ntohl(hdr->totlen)) { if (rcvbuflen >= 8 && rcvbuflen == ntohl(hdr->totlen)) {
if (clusterProcessPacket(link)) { if (clusterProcessPacket(link)) {
sdsfree(link->rcvbuf); if (link->rcvbuf_alloc > RCVBUF_INIT_LEN) {
link->rcvbuf = sdsempty(); zfree(link->rcvbuf);
link->rcvbuf = zmalloc(link->rcvbuf_alloc = RCVBUF_INIT_LEN);
}
link->rcvbuf_len = 0;
} else { } else {
return; /* Link no longer valid. */ return; /* Link no longer valid. */
} }

View File

@ -38,7 +38,9 @@ typedef struct clusterLink {
mstime_t ctime; /* Link creation time */ mstime_t ctime; /* Link creation time */
connection *conn; /* Connection to remote node */ connection *conn; /* Connection to remote node */
sds sndbuf; /* Packet send buffer */ 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 */ struct clusterNode *node; /* Node related to this link if any, or NULL */
} clusterLink; } clusterLink;