Re: [PATCH] drm/i915/gvt: Fix mm reference leak in handle_g2v_notification()

[sashiko-bot@kernel.org]

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 <vulab@iscas.ac.cn>

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/20260916173720.2088455-1-vulab@iscas.ac.cn?part=1