Alignment on own struct types violated by ringbuffer #483
Description
Some libqb structs enforce specific alignment like 8 bytes for struct qb_ipc_request_header, but it seems that not all the code path handling such a struct enforces that, at least when using the shared memory variant, and the library can end up passing unaligned pointers around, e.g., to the msg_process callback, resulting in undefined behavior when accessing its struct members.
As example use the following truncated msg_process callback:
static int32_t s1_msg_process_fn(
qb_ipcs_connection_t *c,
void *data,
size_t size
) {
struct qb_ipc_request_header *req_pt = (struct qb_ipc_request_header *) data;
int32_t request_id = req_pt->id;
int32_t request_size = req_pt->size;
// ...
return 0;
}Which is exercised on the client side by doing something along the lines of
int iov_len = 2;
struct iovec iov[iov_len];
struct qb_ipc_request_header req_header = {
.id = msgid,
.size = sizeof(struct qb_ipc_request_header) + len,
};
iov[0].iov_base = (char *)&req_header;
iov[0].iov_len = sizeof(req_header);
iov[1].iov_base = dataptr;
iov[1].iov_len = len;
int32_t timeout = -1;
int res = qb_ipcc_sendv_recv(conn, iov, iov_len, reply_buffer, sizeof(reply_buffer), timeout);Just for the record: The cast struct enforces 8byte alignment on members and itself:
struct qb_ipc_request_header {
int32_t id __attribute__ ((aligned(8)));
int32_t size __attribute__ ((aligned(8)));
} __attribute__ ((aligned(8)));Compiling above with ASan & (for this issue the actual relevant) UBSan, e.g. for a common Makefile it'd look something like:
CFLAGS += -fsanitize=address,undefined
LDFLAGS += -fsanitize=address,undefined -static-libasan -static-libubsan
one then sometimes (not always!) gets an member access within misaligned address error reported:
server.c:184:10: runtime error: member access within misaligned address 0x7f4f9d0a6094 for type 'struct qb_ipc_request_header', which requires 8 byte alignment
0x7f4f9d0a6094: note: pointer points here
a1 a1 a1 a1 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 68 61 2f 72 65 73 6f 75 72 63 65 73
^
server.c:185:10: runtime error: member access within misaligned address 0x7f4f9d0a6094 for type 'struct qb_ipc_request_header', which requires 8 byte alignment
0x7f4f9d0a6094: note: pointer points here
a1 a1 a1 a1 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 68 61 2f 72 65 73 6f 75 72 63 65 73
^
After looking through the whole call chain and checking how our side and libqb sides handles pointers, it seems that misalignment comes from the libqb ring buffer implementation, namely qb_rb_chunk_step which steps the write pointer always by sizeof(uint32_t), i.e., four bytes, breaking the alignment rules of the qb_ipc_request_header struct.
If the base alignment of most (all?) public types really should be 8 bytes then we could simply increment the write_puffer always by 8 bytes.
But IMO, the 8 byte alignment is rather odd in the first place and might waste quite a bit of (CPU cache) space, albeit I did not really analyze that part closely. So, is there an actual reason for keeping those? Besides ABI breakage causing churn, naturally.