DF-3635 — i915_gem_object_get_pages_phys() error path leaks the GEM shmem vm_object hold+token

What

i915_gem_object_get_pages_phys() takes the GEM backing-store vm_object lock for the whole page-in copy loop:

305 VM_OBJECT_LOCK(vm_obj);
306 for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
...
313     page = shmem_read_mapping_page(vm_obj, i);
315     if (IS_ERR(page)) {
316         err = PTR_ERR(page);
317         goto err_phys;          /* <-- lock still held */
318     }
...
328 VM_OBJECT_UNLOCK(vm_obj);          /* success path only */
...
357 err_phys:
358 drm_pci_free(obj->base.dev, phys);
359
360 return err;                          /* <-- returns with hold+token leaked */

(The #if 0 Linux original has no VM object locking at all; the lock/unlock pair is DragonFly-port glue, and the error jump bypasses the unlock. The sibling function i915_gem_object_get_pages_gtt() does unlock on its error paths — VM_OBJECT_UNLOCK(mapping) at i915_gem.c:3032 — proving the omission here is accidental.)

On this tree VM_OBJECT_LOCK is vm_object_hold() (sys/vm/vm_object.h:267) = refcount_acquire(&obj->hold_count) + lwkt_gettoken(&obj->token) (sys/vm/vm_object.c:283-294). Returning without the matching VM_OBJECT_UNLOCK/vm_object_drop therefore leaks both a hold reference and a token reference on the object that backs the GEM bo: the object can never be reclaimed (hold_count pinned), and the owning thread carries an unbalanced token count (serialization accounting corrupted for every later VM operation on that object by that thread).

Reachability (why Low)

1. Only i915_gem_object_attach_phys() (i915_gem.c:6670) installs the phys ops whose ->get_pages is this function, and on this tree attach_phys is called only from the display modesetting path (intel_display.c:13183) — DRM master territory, not direct unprivileged ioctl.
2. The error return requires shmem_read_mapping_page() to fail on the OBJT_DEFAULT backing object: fresh pages are zero-filled unconditionally (default pager haspage = FALSE, sys/vm/default_pager.c:138-141), so failure needs the page to have real swap backing (vm_pager_get_page → swap_pager I/O error) — i.e. swap-device failure while the bo is paged out.

Once triggered: permanent vm_object leak + token imbalance; worst case a wedge of subsequent VM operations on that GEM object (local DoS, no memory-safety impact).

Fix

One-line unlock before the jump — see fix.diff.

Verification

Not attempted at runtime (Low severity, hardware path): the guest has no i915 device. Status: untested / source-certain. The identical error-path discipline in i915_gem_object_get_pages_gtt (:3026-3033) confirms the intended pattern.

DF-3635 VERDICT

Status: untested (runtime) / certain (source). Severity Low.

Root cause (source-certain)

i915_gem_object_get_pages_phys(): - sys/dev/drm/i915/i915_gem.c:305 — VM_OBJECT_LOCK(vm_obj); (= vm_object_hold: refcount_acquire(hold_count) + lwkt_gettoken, sys/vm/vm_object.c:283-294, sys/vm/vm_object.h:267). - sys/dev/drm/i915/i915_gem.c:313-318 — on shmem_read_mapping_page() failure: goto err_phys without unlock. - sys/dev/drm/i915/i915_gem.c:357-360 — err_phys: frees the dma block and returns; the only VM_OBJECT_UNLOCK(vm_obj) is on the success path at :328. - Control-flow proof: the three other exits (drm_pci_alloc failure :301, kmalloc :332, sg_alloc :338) are taken before or after the locked region; only the in-loop page failure leaks.

Effect: leaked hold_count pins the GEM backing vm_object forever (never reclaimed; vm_object_terminate requires no holders) and leaves an unbalanced LWKT token count on the faulting thread.

Why not reproduced

• Runtime path needs (a) i915 hardware (absent: guest is virtio-only, no /dev/dri — see DF-3634/run.log), (b) i915_gem_object_attach_phys via display modeset (intel_display.c:13183, DRM-master), (c) a swap-pager I/O failure during page-in of a paged-out GEM page (OBJT_DEFAULT fresh pages never fail: default_pager_haspage == FALSE, sys/vm/default_pager.c:138-141). The combination is not constructible on this audit infrastructure.

Impact ceiling

Object-lifetime/serialization corruption confined to one GEM object's backing store: permanent memory leak + possible wedge (local DoS), no memory-safety primitive. Low.

Fix

fix.diff: add VM_OBJECT_UNLOCK(vm_obj); before goto err_phys;, mirroring the pattern already used by the sibling shmem path i915_gem_object_get_pages_gtt() at i915_gem.c:3031-3033 (err_pages: label unlocks). Not kernel-built (no i915 target); one-line symmetric correction, low risk.

Kernel references

• sys/dev/drm/i915/i915_gem.c:305 (lock taken)
• sys/dev/drm/i915/i915_gem.c:315-317 (leaking jump)
• sys/dev/drm/i915/i915_gem.c:328 (success-only unlock)
• sys/dev/drm/i915/i915_gem.c:357-360 (err_phys without unlock)
• sys/dev/drm/i915/i915_gem.c:3031-3033 (correct pattern, sibling fn)
• sys/vm/vm_object.c:283-294 (VM_OBJECT_LOCK semantics)

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.