DF-4596 — set_drive_parms() overwrites drive->freelist without freeing the previous allocation (re-init leak)

• File: sys/dev/raid/vinum/vinumio.c:143-148
• Severity: Low · Confidence: certain (code) / impact small (256 B per cycle)

What

drive->freelist_size = INITIAL_DRIVE_FREELIST;
drive->freelist = (struct drive_freelist *)
    Malloc(INITIAL_DRIVE_FREELIST * sizeof(struct drive_freelist));  /* old pointer clobbered */

set_drive_parms() is reached from init_drive() ← read_drive_label() ← check_drive()/config_drive(). Nothing here checks whether drive->freelist is already allocated. A drive that was previously initialized and later closed keeps its freelist: close_drive() / close_locked_drive() (vinumio.c:202-230) only drop the vnode; only free_drive() (vinumconfig.c:572-588) frees the freelist, and it is not called on the state transitions drive_up → drive_down (e.g. vinum setstate d1 down, I/O failure, daemon_save_config downing a drive).

Re-initializing such a drive — vinum read /dev/… on the same device, or the boot scan — runs set_drive_parms() again, and the old INITIAL_DRIVE_FREELIST(16) * sizeof(struct drive_freelist) allocation leaks. Loopable by an administrator (or the boot scan against flapping drives); 256 bytes per cycle. Kernel memory exhaustion over time; no use-after-free (the old block is simply orphaned).

Fix

     drive->freelist_size = INITIAL_DRIVE_FREELIST;
+    if (drive->freelist)
+   Free(drive->freelist);
     drive->freelist = (struct drive_freelist *)
    Malloc(INITIAL_DRIVE_FREELIST * sizeof(struct drive_freelist));

Not run on the guest

The leak is 256 B/cycle into M_DEVBUF; the guest has no per-allocation-site accounting to observe it cheaply, and the code path (down → re-read) is plain from source. Recorded as untested.

DF-4596 — VERDICT

Status: untested (code-certain; no guest run — the leak is 256 B per re-init cycle into M_DEVBUF and not observably attributable on the guest without allocation-site accounting).

Analysis

• sys/dev/raid/vinum/vinumio.c:144-145 — unconditional drive->freelist = Malloc(16 * sizeof(struct drive_freelist)).
• Lifetime facts verified in source:
• close_locked_drive() (vinumio.c:218-230) drops only the vnode;
• free_drive() (vinumconfig.c:572-588) is the only freer of freelist and it also bzeros the drive — it runs on removal, not on state-down transitions;
• therefore a drive that goes up → down while staying allocated (set_drive_state(…, drive_down, …) from e.g. daemon_save_config() write errors or vinum setstate) keeps freelist != NULL, and the next read_drive_label() → init_drive() → set_drive_parms() for the same device name (re-scan, vinum read, boot scan) overwrites the pointer.

Classification: slow kernel heap leak, administrator/boot-triggered — Low.

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.

(No fix.diff for this item: documented recommendation only - see Fix above.)