DF-5192 — Latent negative bucket index in i915_gem_batch_pool_get (size < PAGE_SIZE)

File: sys/dev/drm/i915/i915_gem_batch_pool.c:95-98 Severity: Info (hardening / latent OOB — not reachable from any current caller) Confidence: likely (hazard certain from code; current unreachability certain from exhaustive call-site analysis; hence no runtime PoC exists)

What the hazard is

n = fls(size >> PAGE_SHIFT) - 1;              /* line 95 */
if (n >= ARRAY_SIZE(pool->cache_list))         /* line 96: upper clamp ONLY */
        n = ARRAY_SIZE(pool->cache_list) - 1;
list = &pool->cache_list[n];                   /* line 98: n may be -1 */

DragonFly's fls() returns 0 for 0 (sys/cpu/x86_64/include/cpufunc.h:283-288), so any size < PAGE_SIZE (including size == 0) yields n = -1, and pool->cache_list[-1] is a struct-relative out-of-bounds list head:

• struct i915_gem_batch_pool is { struct intel_engine_cs *engine; struct list_head cache_list[4]; } (i915_gem_batch_pool.h:13-16), embedded in struct intel_engine_cs at intel_ringbuffer.h:439, right after pmu.sample[] (intel_ringbuffer.h:431).
• cache_list[-1].prev aliases the engine pointer; cache_list[-1].next aliases the 8 bytes preceding the pool (tail of pmu.sample[]).
• The walk list_for_each_entry(obj, list, batch_pool_link) (line 100) then chases a wild next pointer, and list_move_tail(&obj->batch_pool_link, list) (line 142) writes: head->prev->next = obj-link treats the engine pointer as a list_head* and stores a kernel heap pointer into engine->pmu, then overwrites pool->engine with &obj->batch_pool_link.

That is a kernel memory-corruption primitive — if it were reachable.

Why it is NOT reachable today (the verification)

There are exactly two compiled call sites of i915_gem_batch_pool_get() in the entire tree (verified by grep):

1. sys/dev/drm/i915/i915_gem_execbuffer.c:1153 (__reloc_gpu_alloc): passes the literal PAGE_SIZE → fls(1)-1 = 0. Safe.

2. sys/dev/drm/i915/i915_gem_execbuffer.c:1939 (eb_parse): passes PAGE_ALIGN(eb->batch_len). This is gated by eb_use_cmdparser() (i915_gem_execbuffer.c:312-315):

c return intel_engine_needs_cmd_parser(eb->engine) && eb->batch_len;

The && eb->batch_len term forces batch_len >= 1 (u32), so PAGE_ALIGN(batch_len) >= 4096 → size >> PAGE_SHIFT >= 1 → fls(...) >= 1 → n >= 0. Note the ordering in i915_gem_do_execbuffer: the parser gate/call is at lines 2326-2333, before the batch_len == 0 → whole-object expansion at 2351-2352, so eb_parse always sees the raw user value — but the gate sees that same raw value, so size == 0 cannot pass. The legacy v1 ioctl funnels into the same i915_gem_do_execbuffer (v1→v2 conversion at :2503).

eb_parse() is called only from i915_gem_execbuffer.c:2329 under that gate; i915_gem_batch_pool_get is declared in i915_gem_batch_pool.h and referenced nowhere else. Therefore no userspace-reachable path passes size < PAGE_SIZE, and no runtime PoC can exist for this hazard without first adding a new (faulty) kernel caller. Verification was by exhaustive static call-site proof, not by guest execution — a userspace "trigger" is impossible by construction.

Why file it anyway

The invariant that protects the code lives ~1000 lines away in a different file (eb_use_cmdparser in i915_gem_execbuffer.c:314) and is enforced by a single && term with no local defense in the pool itself. Any future caller (e.g. a new parser user, a backport that changes the gate, or the v1/v2 conversion being relaxed) turns this into an immediate wild-pointer write/panic at best, struct corruption at worst. i915_gem_object_create_internal() already asserts GEM_BUG_ON(!size) and page alignment (i915_gem_internal.c:191-192) — the pool should defend its own index the same way. Upstream ultimately deleted the batch pool entirely (Linux 5.x), which corroborates the fragility of this API.

Fix

One-line lower clamp; see fix.diff (never applied to sys/ — audit is read-only).

Reproduce

Not applicable — no reachable code path. This pack is the analysis record: VERDICT.md documents the proof, verdict.json carries status: untested with impact: none.

DF-5192 — VERDICT

Status: untested (analysis-only, by construction). Reproduced: no. Impact: none.

Bottom line

i915_gem_batch_pool_get() computes its LRU bucket index as n = fls(size >> PAGE_SHIFT) - 1 (sys/dev/drm/i915/i915_gem_batch_pool.c:95) and clamps only the upper bound (lines 96-97). DragonFly's fls(0) == 0 (sys/cpu/x86_64/include/cpufunc.h:283-288), so any caller passing size < PAGE_SIZE — most degenerally size == 0 — produces n == -1 and list = &pool->cache_list[-1] (line 98). Given the struct layout ({ engine; cache_list[4]; }, i915_gem_batch_pool.h:13-16, embedded in struct intel_engine_cs at intel_ringbuffer.h:439 immediately after pmu.sample[] at :431), cache_list[-1].prev aliases the live engine pointer and .next aliases pmu tail memory. The subsequent list walk (line 100) chases a wild pointer, and list_move_tail() (line 142) performs head->prev->next = new — writing through the engine pointer as if it were a list_head* — plus clobbers pool->engine. That is a textbook struct-relative OOB write primitive.

It is not reachable from userspace on this tree. Exhaustive proof:

1. i915_gem_batch_pool_get has exactly two call sites in the whole tree (grep over sys/): i915_gem_execbuffer.c:1153 and :1939.
2. Call site 1 (__reloc_gpu_alloc) passes the compile-time constant PAGE_SIZE → n = 0. Safe unconditionally.
3. Call site 2 (eb_parse) passes PAGE_ALIGN(eb->batch_len) where eb->batch_len is the raw u32 user argument. eb_parse is invoked only from i915_gem_do_execbuffer (i915_gem_execbuffer.c:2326-2333) under eb_use_cmdparser(&eb), defined at :312-315 as intel_engine_needs_cmd_parser(eb->engine) && eb->batch_len. The && eb->batch_len conjunct requires batch_len ≥ 1 at the moment eb_parse runs (the batch_len == 0 → whole-object rewrite at :2351-2352 happens strictly after the parser block). PAGE_ALIGN(x ≥ 1) ≥ 4096 → size >> PAGE_SHIFT ≥ 1 → fls ≥ 1 → n ≥ 0.
4. The legacy v1 ioctl converts into the same execbuffer2 path (:2503), so it inherits the same gate.
5. No other module references the symbol (it is not EXPORT_SYMBOL'd; the i915 Makefile/sys/conf/files build set contains no other user).

Hence: hazard certain, reachability zero, nothing to run on the guest. A userspace trigger would require a new kernel caller that violates the size invariant — at which point the primitive is a wild-pointer write through the engine field, realistically a panic, conceivably struct corruption.

Why no guest run (and no false "skipped")

The finding is filed as Info/hardening precisely because the reachability analysis kills exploitation today. The evidence pack therefore contains no build.log/run.log by design; verdict.json records status: untested, reproduced: false, impact: none, attempts: 0. Had this been reachable, the appropriate classification would have been High (kernel OOB write from ioctl-reachable path) and Phase V would have required a guest PoC.

Classes hunted alongside (all killed, for the record)

• LRU move vs detach/get race: every mutator of pool->cache_list[] runs under drm.struct_mutex — i915_gem_batch_pool_get (assert at i915_gem_batch_pool.c:89; callers hold via i915_mutex_lock_interruptible at i915_gem_execbuffer.c:2297) and i915_gem_batch_pool_fini from both intel_engine_cleanup_common (intel_engine_cs.c:671) and the GT park path __i915_gem_park (intel_engine_cs.c:1097 ← i915_gem.c:175, whose caller asserts the same mutex at i915_gem.c:153). Serialized; no race.
• fini with still-active objects → leak/UAF: DF's __i915_gem_object_release_unless_active (i915_gem.c:5420-5429) converts the pool's ownership ref into an active reference for busy objects (dropped on retire); idle objects are put. Nothing in the retire/free path ever touches batch_pool_link again (tree-wide the field has exactly five references: the three in this file, the INIT at i915_gem.c:5099, and the declaration at i915_gem_object.h:129), and frees are RCU-deferred (i915_gem.c:5400-5418), so the detached-list INIT_LIST_HEAD at i915_gem_batch_pool.c:64 leaves no later walker and no stale write.
• list_move_tail on a freshly created object: batch_pool_link is initialized for every object by i915_gem_object_init (i915_gem.c:5092-5099), which i915_gem_object_create_internal calls (i915_gem_internal.c:202) → self-linked head → the move is a benign delete-self + add-tail. No uninitialized-list corruption.
• fence clearing on the idle transition: reservation_object_add_excl_fence(resv, NULL) (i915_gem_batch_pool.c:121) is NULL-safe in the vendored implementation (sys/dev/drm/linux_reservation.c:244-246 if (fence) dma_fence_get(fence);); the resv cannot be freed under the walker because the pool owns the object and struct_mutex is held.
• 4 GiB truncation into object creation: DF widened i915_gem_object_create_internal to phys_addr_t size with an overflows_type() guard (i915_gem_internal.c:186-195), and the bucket arithmetic itself saturates at n=3 long before any int truncation matters for the u32-bounded callers.
• GEM_BUG_ON panic reachability (i915_gem_batch_pool.c:109/126): the signaled-then-retired invariant holds because request submission and this walk are both under struct_mutex; no interleaving found. (GEM_BUG_ON is a real BUG() here since CONFIG_DRM_I915_DEBUG_GEM=1, sys/dev/drm/kconfig.h:19 — worth remembering, but no trigger exists.)
• DF-2192 (object leak when pin_pages fails on the freshly created object, i915_gem_batch_pool.c:133-140): known from pass 1; not re-reported and not double-counted here.

Recommended fix

Clamp the lower bound where the upper bound is already clamped — see fix.diff. One line, defense-in-depth, no behavior change for any current caller.

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.