# DF-1397 — vinum config_plex auto-naming strcpy/strcat overflow

## Verdict: REPRODUCED (live kernel + byte-exact harness). Fix VALIDATED (live before/after).

A 63-character vinum volume name combined with an unnamed plex causes
`config_plex` to overflow the 64-byte `plex->name` buffer into the adjacent
`organization` enum field. This was reproduced **live on the DragonFly guest**
(`kldload vinum` + `vinum create`) and deterministically with a byte-exact
userspace harness, and the authored fix was validated with a true before/after
kernel-module rebuild + re-test.

## Mechanism (trigger → primitive → effect)

`struct plex` (`sys/dev/raid/vinum/vinumvar.h:548-550`):
```c
struct plex {
    char name[MAXPLEXNAME];        /* MAXPLEXNAME = 64  (vinumvar.h:103) */
    enum plexorg organization;     /* IMMEDIATELY AFTER name -> overflow target */
    ...
};
```

Volume names are bounded to `MAXVOLNAME-1 = 63` chars + NUL by
`ksnprintf(vol->name, sizeof(vol->name), "%s", name)` at
`vinumconfig.c:925`. When an unnamed plex is auto-named, `config_plex`
runs (`vinumconfig.c:1459-1464`):

```c
strcpy(plex->name, VOL[plex->volno].name);   /* 63 chars + NUL fills name[0..63] exactly */
ksprintf(plexsuffix, ".p%d", pindex);          /* ".p0" */
strcat(plex->name, plexsuffix);                /* writes ".p0\0" at name[63..66] -> OOB */
```

`strcat` finds the NUL at `name[63]`, writes `.`,`p`,`0`,`\0` at indices
63,64,65,66 — indices 64/65/66 are past the 64-byte buffer, corrupting the
`organization` enum. (A symmetric site exists for subdisks at `:1274-1278`:
`strcpy(sd->name, PLEX[].name)` + `strcat(".sN")`.)

The same primitive appears for subdisks (`config_sd`, `:1269-1278`):
`strcpy(sd->name, PLEX[sd->plexno].name)` + `strcat(".s%d")` with a 63-char
plex name; the `sd->state` field is the overflow target.

## Primitive

- **Class:** heap (in-object) overwrite — 3-5 bytes past a 64-byte name into the
  adjacent `enum plexorg organization` (or `enum sdstate state`) field of the
  same kmalloc'd `struct plex`/`struct sd` array element.
- **Content:** low-attacker-controlled ASCII (`.pN\0` / `.sN\0`).
- **Effect:** corrupts plex organization → the immediately-following
  `if (isstriped(plex))` (`:1466`) reads the corrupted enum; misbehavior /
  potential panic. Because the overflow lands inside the SAME struct (not a
  slab boundary), INVARIANTS slab checks do NOT trip — corruption is silent.

## Reachability / threat model

- `vinum` is a loadable module (`vinum.ko`, prebuilt at `/boot/kernel/`), NOT in
  `X86_64_GENERIC`.
- The live trigger is `vinum create` (root) with a 63-char volume name + unnamed
  plex. The realistic untrusted-input vector is **on-disk vinum config**
  auto-parsed at boot/autoconfig: an attacker who can write a vinum config to a
  disk (e.g. via `vfs.usermount` + a user-owned image, or a USB drive) can
  place a 63-char volume name that the kernel parses on autoconfig.
- This is NOT a privilege boundary the bright-line rule forbids for
  demonstration (kldload is used only to load the vulnerable module; the bug
  path itself is `vinum create` / on-disk config). No `uid=0` chain is claimed
  — the realistic impact is **memory corruption (DoS / misbehavior)**, not
  priv-esc, because the controlled bytes are limited ASCII into a single enum.

## Live reproduction (proof)

On unpatched `6.5-DEVELOPMENT #0`:
```
# kldload vinum ; vinum create -f vc.conf   (vc.conf: 63-char "volume AAAA..." + "plex org concat")
P AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA.p0   ...
# plex name printed length = 66   (buffer is 64 -> 2 bytes overflowed into `organization`)
```
The plex name printing as **66 characters** is direct proof: the terminating
NUL now lives inside the `organization` field, so the char* walk runs past the
64-byte name. See `run.log` (harness) and the live output in this VERDICT.

## Fix validation (before/after, live)

Authored `fix.diff`: replace `strcpy`+`ksprintf`+`strcat` at both sites with a
single bounded `ksnprintf(.., sizeof(name), "%s.p%d", volname, pindex)` /
`"%s.s%d"`.

- **Baseline (unpatched `#0`):** rebuilt module is the stock `vinum.ko`; live
  repro → plex name = **66 chars** (overflow present).
- **Patched (rebuilt `vinum.ko` with fix, `cc ... -Werror`, RC=0):** kldload +
  re-run repro → plex name = **63 chars** (ksnprintf bounded; overflow GONE).

```
fixed module: /usr/obj/usr/src/sys/dev/raid/vinum/vinum.ko   (rebuilt, -Werror clean)
BEFORE (stock): plex name length: 66
AFTER  (fixed): plex name length: 63   -> overflow into organization eliminated
```
**fix_status = fixed.** Both the compile (`-Werror`) and the runtime
before/after confirm the fix closes the bug.

## PoC changes

- `vinum_overflow.c` — byte-exact userspace harness replicating the `struct plex`
  layout and the `strcpy`+`ksprintf`+`strcat` logic; shows the printed name
  walking to 66 chars and `organization` corrupted from `plex_concat(1)` to
  `12400`.
- `live_repro.sh` — the live kernel reproduction (`kldload vinum` + `vinum
  create` with a 63-char volume name).
- `fix.diff` — the verified fix.

## Kernel references

- `sys/dev/raid/vinum/vinumconfig.c:1459-1464` (plex auto-name sink)
- `sys/dev/raid/vinum/vinumconfig.c:1274-1278` (subdisk auto-name sink)
- `sys/dev/raid/vinum/vinumconfig.c:925` (volume-name 63-char bound)
- `sys/dev/raid/vinum/vinumvar.h:103` (MAXPLEXNAME=64), `:548-550` (struct plex layout)