mirror of https://mirror.osredm.com/root/redis.git
Resolve bounds checks on cluster_legacy.c (#13970)
Based on https://github.com/valkey-io/valkey/pull/1463 and https://github.com/valkey-io/valkey/pull/1481 In the failure of fully CI(https://github.com/redis/redis/actions/runs/14595343452/job/40979173087?pr=13965) in version 7.0 we are getting a number of errors like: ``` array subscript ‘clusterMsg[0]’ is partly outside array bounds of ‘unsigned char[2272]’ ``` Which is basically GCC telling us that we have an object which is longer than the underlying storage of the allocation. We actually do this a lot, but GCC is generally not aware of how big the underlying allocation is, so it doesn't throw this error. We are specifically getting this error because the msgBlock can be of variable length depending on the type of message, but GCC assumes it's the longest one possible. The solution I went with here was make the message type optional, so that it wasn't included in the size. I think this also makes some sense, since it's really just a helper for us to easily cast the object around. This compilation warning only occurs in version 7.2, because in [this PR](https://github.com/redis/redis/pull/13073), we started passing `-flto` to `CFLAGS` by default. It seems that in this case, GCC is unable to detect such warnings. However, this change is not present in version 7.2. So, to reproduce this compilation warning in versions after 7.2, we can pass `OPTIMIZATION=-O2` manually. --------- Co-authored-by: madolson <34459052+madolson@users.noreply.github.com>
This commit is contained in:
parent
e7cd611be1
commit
35eff3d49a
|
@ -337,9 +337,14 @@ int auxTlsPortPresent(clusterNode *n) {
|
|||
typedef struct {
|
||||
size_t totlen; /* Total length of this block including the message */
|
||||
int refcount; /* Number of cluster link send msg queues containing the message */
|
||||
clusterMsg msg;
|
||||
clusterMsg msg[];
|
||||
} clusterMsgSendBlock;
|
||||
|
||||
/* Helper function to extract a normal message from a send block. */
|
||||
static clusterMsg *getMessageFromSendBlock(clusterMsgSendBlock *msgblock) {
|
||||
return &msgblock->msg[0];
|
||||
}
|
||||
|
||||
/* -----------------------------------------------------------------------------
|
||||
* Initialization
|
||||
* -------------------------------------------------------------------------- */
|
||||
|
@ -1186,12 +1191,12 @@ void clusterReset(int hard) {
|
|||
* CLUSTER communication link
|
||||
* -------------------------------------------------------------------------- */
|
||||
static clusterMsgSendBlock *createClusterMsgSendBlock(int type, uint32_t msglen) {
|
||||
uint32_t blocklen = msglen + sizeof(clusterMsgSendBlock) - sizeof(clusterMsg);
|
||||
uint32_t blocklen = msglen + sizeof(clusterMsgSendBlock);
|
||||
clusterMsgSendBlock *msgblock = zcalloc(blocklen);
|
||||
msgblock->refcount = 1;
|
||||
msgblock->totlen = blocklen;
|
||||
server.stat_cluster_links_memory += blocklen;
|
||||
clusterBuildMessageHdr(&msgblock->msg,type,msglen);
|
||||
clusterBuildMessageHdr(getMessageFromSendBlock(msgblock),type,msglen);
|
||||
return msgblock;
|
||||
}
|
||||
|
||||
|
@ -3345,7 +3350,7 @@ void clusterWriteHandler(connection *conn) {
|
|||
while (totwritten < NET_MAX_WRITES_PER_EVENT && listLength(link->send_msg_queue) > 0) {
|
||||
listNode *head = listFirst(link->send_msg_queue);
|
||||
clusterMsgSendBlock *msgblock = (clusterMsgSendBlock*)head->value;
|
||||
clusterMsg *msg = &msgblock->msg;
|
||||
clusterMsg *msg = getMessageFromSendBlock(msgblock);
|
||||
size_t msg_offset = link->head_msg_send_offset;
|
||||
size_t msg_len = ntohl(msg->totlen);
|
||||
|
||||
|
@ -3519,7 +3524,7 @@ void clusterSendMessage(clusterLink *link, clusterMsgSendBlock *msgblock) {
|
|||
if (!link) {
|
||||
return;
|
||||
}
|
||||
if (listLength(link->send_msg_queue) == 0 && msgblock->msg.totlen != 0)
|
||||
if (listLength(link->send_msg_queue) == 0 && getMessageFromSendBlock(msgblock)->totlen != 0)
|
||||
connSetWriteHandlerWithBarrier(link->conn, clusterWriteHandler, 1);
|
||||
|
||||
listAddNodeTail(link->send_msg_queue, msgblock);
|
||||
|
@ -3530,7 +3535,7 @@ void clusterSendMessage(clusterLink *link, clusterMsgSendBlock *msgblock) {
|
|||
server.stat_cluster_links_memory += sizeof(listNode);
|
||||
|
||||
/* Populate sent messages stats. */
|
||||
uint16_t type = ntohs(msgblock->msg.type);
|
||||
uint16_t type = ntohs(getMessageFromSendBlock(msgblock)->type);
|
||||
if (type < CLUSTERMSG_TYPE_COUNT)
|
||||
server.cluster->stats_bus_messages_sent[type]++;
|
||||
}
|
||||
|
@ -3704,7 +3709,7 @@ void clusterSendPing(clusterLink *link, int type) {
|
|||
* sizeof(clusterMsg) or more. */
|
||||
if (estlen < (int)sizeof(clusterMsg)) estlen = sizeof(clusterMsg);
|
||||
clusterMsgSendBlock *msgblock = createClusterMsgSendBlock(type, estlen);
|
||||
clusterMsg *hdr = &msgblock->msg;
|
||||
clusterMsg *hdr = getMessageFromSendBlock(msgblock);
|
||||
|
||||
if (!link->inbound && type == CLUSTERMSG_TYPE_PING)
|
||||
link->node->ping_sent = mstime();
|
||||
|
@ -3837,7 +3842,7 @@ clusterMsgSendBlock *clusterCreatePublishMsgBlock(robj *channel, robj *message,
|
|||
msglen += sizeof(clusterMsgDataPublish) - 8 + channel_len + message_len;
|
||||
clusterMsgSendBlock *msgblock = createClusterMsgSendBlock(type, msglen);
|
||||
|
||||
clusterMsg *hdr = &msgblock->msg;
|
||||
clusterMsg *hdr = getMessageFromSendBlock(msgblock);
|
||||
hdr->data.publish.msg.channel_len = htonl(channel_len);
|
||||
hdr->data.publish.msg.message_len = htonl(message_len);
|
||||
memcpy(hdr->data.publish.msg.bulk_data,channel->ptr,sdslen(channel->ptr));
|
||||
|
@ -3860,7 +3865,7 @@ void clusterSendFail(char *nodename) {
|
|||
+ sizeof(clusterMsgDataFail);
|
||||
clusterMsgSendBlock *msgblock = createClusterMsgSendBlock(CLUSTERMSG_TYPE_FAIL, msglen);
|
||||
|
||||
clusterMsg *hdr = &msgblock->msg;
|
||||
clusterMsg *hdr = getMessageFromSendBlock(msgblock);
|
||||
memcpy(hdr->data.fail.about.nodename,nodename,CLUSTER_NAMELEN);
|
||||
|
||||
clusterBroadcastMessage(msgblock);
|
||||
|
@ -3877,7 +3882,7 @@ void clusterSendUpdate(clusterLink *link, clusterNode *node) {
|
|||
+ sizeof(clusterMsgDataUpdate);
|
||||
clusterMsgSendBlock *msgblock = createClusterMsgSendBlock(CLUSTERMSG_TYPE_UPDATE, msglen);
|
||||
|
||||
clusterMsg *hdr = &msgblock->msg;
|
||||
clusterMsg *hdr = getMessageFromSendBlock(msgblock);
|
||||
memcpy(hdr->data.update.nodecfg.nodename,node->name,CLUSTER_NAMELEN);
|
||||
hdr->data.update.nodecfg.configEpoch = htonu64(node->configEpoch);
|
||||
memcpy(hdr->data.update.nodecfg.slots,node->slots,sizeof(node->slots));
|
||||
|
@ -3899,7 +3904,7 @@ void clusterSendModule(clusterLink *link, uint64_t module_id, uint8_t type,
|
|||
msglen += sizeof(clusterMsgModule) - 3 + len;
|
||||
clusterMsgSendBlock *msgblock = createClusterMsgSendBlock(CLUSTERMSG_TYPE_MODULE, msglen);
|
||||
|
||||
clusterMsg *hdr = &msgblock->msg;
|
||||
clusterMsg *hdr = getMessageFromSendBlock(msgblock);
|
||||
hdr->data.module.msg.module_id = module_id; /* Already endian adjusted. */
|
||||
hdr->data.module.msg.type = type;
|
||||
hdr->data.module.msg.len = htonl(len);
|
||||
|
@ -3981,11 +3986,10 @@ void clusterRequestFailoverAuth(void) {
|
|||
uint32_t msglen = sizeof(clusterMsg)-sizeof(union clusterMsgData);
|
||||
clusterMsgSendBlock *msgblock = createClusterMsgSendBlock(CLUSTERMSG_TYPE_FAILOVER_AUTH_REQUEST, msglen);
|
||||
|
||||
clusterMsg *hdr = &msgblock->msg;
|
||||
/* If this is a manual failover, set the CLUSTERMSG_FLAG0_FORCEACK bit
|
||||
* in the header to communicate the nodes receiving the message that
|
||||
* they should authorized the failover even if the master is working. */
|
||||
if (server.cluster->mf_end) hdr->mflags[0] |= CLUSTERMSG_FLAG0_FORCEACK;
|
||||
if (server.cluster->mf_end) msgblock->msg[0].mflags[0] |= CLUSTERMSG_FLAG0_FORCEACK;
|
||||
clusterBroadcastMessage(msgblock);
|
||||
clusterMsgSendBlockDecrRefCount(msgblock);
|
||||
}
|
||||
|
|
|
@ -151,7 +151,7 @@ void evictionPoolPopulate(int dbid, dict *sampledict, dict *keydict, struct evic
|
|||
for (j = 0; j < count; j++) {
|
||||
unsigned long long idle;
|
||||
sds key;
|
||||
robj *o;
|
||||
robj *o = NULL;
|
||||
dictEntry *de;
|
||||
|
||||
de = samples[j];
|
||||
|
|
Loading…
Reference in New Issue