Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] drm/i915/gvt: UAF and Double Free in PPGTT mm object lifecycle --
commit 01f0e076eca16f11c93a812eb503401ad6e94137 Author: Wentao Liang <[email protected]> drm/i915/gvt: Fix mm reference leak in handle_g2v_notification() This commit changes the PPGTT page table create path to avoid taking an extra reference for an already registered mm. It uses intel_vgpu_find_ppgtt_mm() to check if the mm exists and skips creation if it is already tracked. > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > b/drivers/gpu/drm/i915/gvt/handlers.c > index 22dd0179863ee..6b967da3a8836 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -1505,7 +1505,17 @@ static int handle_g2v_notification(struct intel_vgpu > *vgpu, int notification) > root_entry_type = GTT_TYPE_PPGTT_ROOT_L3_ENTRY; > fallthrough; > case VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE: > - mm = intel_vgpu_get_ppgtt_mm(vgpu, root_entry_type, pdps); > + /* > + * A newly created mm keeps its initial reference as the > + * registration reference, dropped by the DESTROY > + * notification. A duplicate CREATE for an already tracked > + * mm must not take an extra reference that nothing drops. > + */ > + mm = intel_vgpu_find_ppgtt_mm(vgpu, pdps); > + if (mm) > + return 0; [Severity: Critical] Can this logic lead to a use-after-free and double free if a guest issues a duplicate CREATE after a DESTROY while a workload is still active? If a guest VM performs the following sequence: 1. Issues a CREATE notification (refcount=1). 2. Submits Workload A using the page table (refcount=2). 3. Issues a DESTROY notification (refcount drops to 1, object remains due to Workload A). 4. Issues another CREATE for the same page table. Here, intel_vgpu_find_ppgtt_mm() finds the object and returns 0, but no registration reference is taken, leaving the refcount at 1. If the guest then submits Workload B (refcount=2), and Workload A finishes (refcount=1), a subsequent DESTROY from the guest will drop the refcount to 0: drivers/gpu/drm/i915/gvt/gtt.c:intel_vgpu_put_ppgtt_mm() { ... intel_vgpu_mm_put(mm); ... } This frees the object while Workload B is still active. Once Workload B finishes, the cleanup path accesses the prematurely freed shadow_mm and attempts to drop its reference again: drivers/gpu/drm/i915/gvt/scheduler.c:intel_vgpu_destroy_workload() { ... if (workload->shadow_mm) intel_vgpu_mm_put(workload->shadow_mm); ... } Does this new condition need to take a registration reference when the mm is found, to prevent it from being freed prematurely? > + > + mm = intel_vgpu_create_ppgtt_mm(vgpu, root_entry_type, pdps); > return PTR_ERR_OR_ZERO(mm); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
