DF-4800 — concurrent revive_block() calls race the revive cursor: both advance it, blocks between the advances are silently skipped, and the subdisk comes up with never-revived (stale) data

• File: sys/dev/raid/vinum/vinumrevive.c:56-223 (revive_block)
• Severity: Medium (silent data corruption on a "healthy" mirror; root starts the revive, every reader of the volume then sees stale data)
• Bucket: base:raid / data-corruption
• Status: reproduced 3/3 (152, 152, 153 corrupted 4k blocks of 1536), fix validated on the guest

Root cause

revive_block() has no serialization whatsoever:

• the whole VINUM_SETSTATE ioctl path (vinumioctl → setstate → start_object vinumstate.c:848 → revive_block) runs without lock_config() (the DF-4478 family finding: only VINUM_CREATE takes it, vinumioctl.c:103), and revive_block itself takes no per-sd lock;
• sd->revived is read at entry (vinumrevive.c:89, size/address computation) and advanced only after the data write completes (:187) — the two biowaits in between are the interleaving window;
• two callers that both read cursor R copy the SAME block, then both execute sd->revived += size >> DEV_BSHIFT — the cursor jumps to R+2N and the block [R+N, R+2N) is never copied;
• set_sd_state(sdno, sd_up, setstate_force) at :190 then declares the subdisk up with the holes in place (save_config() persists the state), and both concurrent finishers also race save_config() / set_sd_state on the shared config.

On the default md (malloc) backend the non-MPSAFE vinum ioctl holds the MP lock for the whole call and md completes inline (no sleep → no passive mplock release), which incidentally serializes the two callers. On any backing that completes asynchronously (real disks — here a virtio-blk disk), biowait sleeps, the scheduler passively releases the MP lock, and the interleave becomes the norm: every retry of vinum start on an already-reviving subdisk (two scripts, a retry loop, a stuck first starter plus an impatient admin) corrupts the mirror.

Reproduction (guest, root)

setup_async.sh builds a 3 MB mirrored concat volume on two GPT partitions of a second virtio disk (/dev/vbd1s0, /dev/vbd1s1), writes a random pattern through the volume, marks vol.p1.s0 stale and zeroes its whole data window (drive sectors [265, 6409)). racer.c + run.sh then start two processes that tight-loop the VINUM_SETSTATE(object_up, blocksize=4096) ioctl 150 times each, then a single vinum start finishes the revive, and the two plex data windows are compared.

Stock kernel (3 runs):

MID:      620 4k blocks already differ (skipped during the race)
RESULT: DATA WINDOWS DIFFER -- skipped-block corruption persisted:
total corrupted 4k blocks:      152 of 1536     (runs: 152, 152, 153)

152+ blocks (~620 KB) of the "successfully revived" plex still contain the pre-revive zeros; volume reads round-robin between the plexes, so readers silently receive that stale data. run.log, run.2.log, run.3.log.

Fix (validated)

fix.diff adds a per-subdisk one-bit interlock:

+   int revive_busy;        /* DF-4800 fix: one revival at a time */

     if (sd->plexno < 0)                        /* no plex? */
    return EINVAL;
+    if (atomic_cmpset_int(&sd->revive_busy, 0, 1) == 0)
+   return EBUSY;

(cleared on both exits; the ENOMEM parity path and the final return). Second and later starters get EBUSY on their first ioctl and userland stops; the survivor copies every block.

Patched module (same guest, fix_run.log):

child 907: 1 ioctls, final error=16 msg=          <- EBUSY interlock
child 908: stopped after 150 ioctls (bounded)
RESULT: DATA WINDOWS IDENTICAL (no skipped blocks)

Files

• setup_async.sh — async (vbd1) mirrored-volume setup + clobber
• setup_concurrent.sh — md variant of the same setup
• racer.c — N tight-loop concurrent VINUM_SETSTATE revive drivers
• run.sh — phase 1 (bounded 2-racer race + mid compare), phase 2 (single clean finish + final compare)
• run.log, run.2.log, run.3.log — stock reproductions
• fix.diff — the interlock (validated)
• fix_run.log — patched-module validation
• env.txt — guest/kernel info

DF-4800 verdict — REPRODUCED (silent stale data on an "up" mirror, 3/3), fix validated

• finding: concurrent revive_block() calls race sd->revived (read at sys/dev/raid/vinum/vinumrevive.c:89, advanced at :187 after two biowaits): both callers copy the same block and both advance the cursor, so the block between the two advances is silently skipped. set_sd_state(sd_up) at :190 then brings the subdisk up with never-revived content; volume reads round-robin across plexes and serve the stale data. No lock anywhere on the path (vinumioctl.c:230 VINUM_SETSTATE → setstate → start_object vinumstate.c:880 → revive_block).
• guest: DragonFly dfbsd 6.5-DEVELOPMENT #0 (stock kernel, stock module), 3 MB mirrored concat volume on two GPT partitions of a second virtio-blk disk (async completions open the mplock window that the inline md backend coincidentally closes).

Runs (stock)

Mid-race snapshot (run 1): 620 differing blocks = 468 uncopied tail + 152 already-skipped. After the single-reviver finish, exactly the skipped blocks remain (~620 KB of zeros vs pattern). vinum list reports both plexes up; no error is logged anywhere — the corruption is completely silent.

Control: the same two-racer workload on md (malloc) backing does not skip (cursor advanced exactly once per ioctl): the non-MPSAFE vinum ioctl (vinum.c:58 { "vinum", 0, D_DISK }, no D_MPSAFE → dev_needmplock) holds the MP lock across the whole call, and inline md completions never sleep, so the kernel passively-never releases the mp lock. On real (async) disks the biowait sleeps release it (lwkt passive release) and the race is the default behavior. This also explains why the interlock-free code only ever "worked" by accident.

Fix validation

fix.diff (per-sd revive_busy cmpset interlock, EBUSY to the second caller, cleared on both exits), built as a module on the same guest (together with the DF-4482 v2 fix):

child 907: 1 ioctls, final error=16 msg=            (EBUSY, refused)
child 908: stopped after 150 ioctls (bounded)
RESULT: DATA WINDOWS IDENTICAL (no skipped blocks)   (fix_run.log)

fix_status: fixed (baseline_reproduced=1, patched_reproduced=0).

Exploitability

Demonstrated impact is silent data corruption (integrity), not memory corruption. Trigger needs root to start two revives of the same subdisk (double vinum start, retry scripts, or a stuck starter plus a manual restart); victims are any readers of the volume (e.g. an unprivileged user reading a filesystem mounted on it).

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.