[v92-audit] clusterbus-offload: misaligned clusterMsgHeader reads in offloaded packet framing (UBSan)

[madolson]

Cluster Bus IO offload (#3438) added a framing pass that walks the receive buffer packet by packet and casts rcvbuf + offset to clusterMsgHeader *, where offset is the running sum of the preceding packets' totlen. Cluster bus messages carry no length padding, so once two or more packets arrive in one read that offset is usually not a multiple of 4, and reading hdr->totlen and hdr->type through the struct pointer is a misaligned access. Before this change the header was only ever parsed at offset 0 of a zmalloced buffer, which is aligned. It does not fault on x86-64 or aarch64, but it is undefined behavior of exactly the kind the daily SANITIZER=undefined job exists to catch, and it appears only when cluster bus reads are genuinely offloaded, which needs io-threads > 1 with the pool active.

Details

Problem

Two new sites read the header at an arbitrary byte offset.

src/cluster_legacy.c:5037 in clusterFindCompletePackets(), which runs on an IO worker thread (only caller is clusterReadJob() at src/cluster_legacy.c:9290):

clusterMsgHeader *hdr = (clusterMsgHeader *)(rcvbuf + offset);
...
uint32_t totlen = ntohl(hdr->totlen);   /* 5046 */
uint16_t type = ntohs(hdr->type);       /* 5047 */
...
offset += totlen;

src/cluster_legacy.c:4845 in clusterDrainCompletePackets() on the main thread:

clusterMsgHeader *hdr = (clusterMsgHeader *)(link->rcvbuf + consumed);
uint32_t totlen = ntohl(hdr->totlen);   /* 4846 */

offset and consumed accumulate raw totlen values. A light PUBLISH message has totlen == CLUSTERMSG_LIGHT_MIN_LEN + channel_len + message_len, so any value is reachable and every header after the first in a batched read lands on an arbitrary alignment.

The pre-existing synchronous reader does not have this. It parses only (clusterMsgHeader *)link->rcvbuf, offset 0 of the allocation (src/cluster_legacy.c:5096).

Introduced by 2f3bcea ("Cluster Bus IO offload (#3438)").

Repro

Two nodes with io-threads 5 and io-threads-always-active yes, built SANITIZER=undefined, then PUBLISH at varying payload lengths so bus messages batch:

port 7392
cluster-enabled yes
io-threads 5
io-threads-always-active yes

valkey-cli -p 7391 cluster addslotsrange 0 8191
valkey-cli -p 7392 cluster addslotsrange 8192 16383
valkey-cli -p 7391 cluster meet 127.0.0.1 7392
valkey-cli -p 7391 eval "for i=1,30000 do redis.call('publish','t', string.rep('a', 1 + (i % 977))) end return 1" 0

Receiving node's stderr:

cluster_legacy.c:5046:27: runtime error: member access within misaligned address 0x000030cd9cb9 for type 'struct clusterMsgHeader', which requires 4 byte alignment
0x000030cd9cb9: note: pointer points here
 61 61 61  61 52 43 6d 62 00 00 00  1e 00 01 00 00 80 04 00  00 00 00 00 01 00 00 00  05 74 61 61 61
              ^
cluster_legacy.c:5047:25: runtime error: member access within misaligned address 0x000030cd9cb9 for type 'struct clusterMsgHeader', which requires 4 byte alignment
0x000030cd9cb9: note: pointer points here
 61 61 61  61 52 43 6d 62 00 00 00  1e 00 01 00 00 80 04 00  00 00 00 00 01 00 00 00  05 74 61 61 61
              ^
cluster_legacy.c:4846:27: runtime error: member access within misaligned address 0x000030cd9cb9 for type 'struct clusterMsgHeader', which requires 4 byte alignment
0x000030cd9cb9: note: pointer points here
 61 61 61  61 52 43 6d 62 00 00 00  1e 00 01 00 00 80 04 00  00 00 00 00 01 00 00 00  05 74 61 61 61
              ^

The dumped bytes are a real light PUBLISH: 52 43 6d 62 is RCmb, totlen 0x1e, ver 1, type 0x8004, channel_len 1, message_len 5. It sits at ...9cb9, one byte past a 4 byte boundary, because the packet ahead of it in the same read ended there. clusterFindCompletePackets() is reachable only from the offload job, so the worker path definitely ran rather than the synchronous fallback.

No delivery loss: all 30000 messages arrived, and the same run without offload delivered the same count. The only observable effect is the UB.

.github/workflows/daily.yml:1256 builds SANITIZER=undefined and runs ./runtest --accurate, but the suite defaults to io-threads 1, so this path is probably not engaged there today.

Fix

Do not read the fields through the struct pointer. memcpy the 4 byte totlen and 2 byte type out of a char * cursor at both sites.

Note on the sanitizer build

SANITIZER=undefined and SANITIZER=address binaries segfault at startup in the string2ll ifunc resolver on this host's GCC 7 (src/util.c:644); the resolver runs before the sanitizer runtime initializes and VALKEY_NO_SANITIZE does not suppress it on that compiler. Forcing HAVE_IFUNC 0 in src/config.h works around it, and is the only delta from the tested commit. It does not touch cluster code. The siphash.c:147 misalignment the same run reports is pre-existing and unrelated; it is useful only as proof the alignment checker was live.

This was generated by AI but verified, with love, by a human.

[madolson]

Independent audit reproduced this on a single node at 3f9062e, no second node and no PUBLISH traffic needed. Two crafted PING frames written back to back on the bus port, the first padded to an odd totlen, is enough to fire all three sites; --io-threads 1 on the same binary and the same input is clean, which pins it to the offload path.

Repro

valkey-server --port 7397 --cluster-enabled yes --io-threads 4 \
  --io-threads-always-active yes --save '' --appendonly no

Then, against the bus port 17397, where TMPL is any real 2568-byte cluster bus packet captured off the wire:

def ping(pad, sender):
    d = bytearray(TMPL) + bytearray(pad)
    d[40:80] = sender
    struct.pack_into('>H', d, 12, 0)                 # type = PING
    struct.pack_into('>I', d, 4, len(TMPL) + pad)    # totlen
    return bytes(d)

s = socket.create_connection(('127.0.0.1', 17397))
s.sendall(ping(1, b'a'*40) + ping(0, b'b'*40))       # odd first totlen

--io-threads 4:

cluster_io_threaded_reads_processed:2
cluster_legacy.c:4846:27: runtime error: member access within misaligned address 0x00001d11bbf9 for type 'struct clusterMsgHeader', which requires 4 byte alignment
cluster_legacy.c:5046:27: runtime error: member access within misaligned address 0x00001d11bbf9 for type 'struct clusterMsgHeader', which requires 4 byte alignment
cluster_legacy.c:5047:25: runtime error: member access within misaligned address 0x00001d11bbf9 for type 'struct clusterMsgHeader', which requires 4 byte alignment

--io-threads 1, same binary, same bytes:

cluster_io_threaded_reads_processed:0
--- UBSan reports for io-threads=1:
--- end

The framing pass alone is enough, so this does not depend on the packet being acceptable to clusterProcessPacket().

This was generated by AI but verified, with love, by a human.