Re: [PATCH] drm/i915/gvt: Prevent use-after-free in ppgtt_free_all_spt()

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3368 bytes --]

On 2019.04.04 08:30:56 +0100, Chris Wilson wrote:
> ppgtt_free_all_spt() iterates the radixtree as it is deleting it,
> forgoing all protection against the leaves being freed in the process
> (leaving the iter pointing into the void).
> 
> A minimal fix seems to be to use the available post_shadow_list to
> decompose the tree into a list prior to destroying the radixtree.
> 
> Alerted by the sparse warnings:
> 
> drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignment (different address spaces)
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    expected void **slot
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    got void [noderef] <asn:4> **
> drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignment (different address spaces)
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    expected void **slot
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    got void [noderef] <asn:4> **
> drivers/gpu/drm/i915/gvt/gtt.c:758:45: warning: incorrect type in argument 1 (different address spaces)
> drivers/gpu/drm/i915/gvt/gtt.c:758:45:    expected void [noderef] <asn:4> **slot
> drivers/gpu/drm/i915/gvt/gtt.c:758:45:    got void **slot
> drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in argument 1 (different address spaces)
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    expected void [noderef] <asn:4> **slot
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    got void **slot
> drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignment (different address spaces)
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    expected void **slot
> drivers/gpu/drm/i915/gvt/gtt.c:757:9:    got void [noderef] <asn:4> **
> 
> This would also have been loudly warning if run through CI for the
> invalid RCU dereferences.
> 
> Fixes: b6c126a39345 ("drm/i915/gvt: Manage shadow pages with radix tree")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Changbin Du <changbin.du@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index cf133ef03873..9814773882ec 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -750,14 +750,20 @@ static void ppgtt_free_spt(struct intel_vgpu_ppgtt_spt *spt)
>  
>  static void ppgtt_free_all_spt(struct intel_vgpu *vgpu)
>  {
> -	struct intel_vgpu_ppgtt_spt *spt;
> +	struct intel_vgpu_ppgtt_spt *spt, *spn;
>  	struct radix_tree_iter iter;
> -	void **slot;
> +	LIST_HEAD(all_spt);
> +	void __rcu **slot;
>  
> +	rcu_read_lock();
>  	radix_tree_for_each_slot(slot, &vgpu->gtt.spt_tree, &iter, 0) {
>  		spt = radix_tree_deref_slot(slot);
> -		ppgtt_free_spt(spt);
> +		list_move(&spt->post_shadow_list, &all_spt);
>  	}
> +	rcu_read_unlock();
> +
> +	list_for_each_entry_safe(spt, spn, &all_spt, post_shadow_list)
> +		ppgtt_free_spt(spt);
>  }
>

As we ensure to flush post shadow list, so this is safe to reuse.

Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>

thanks!

>  static int ppgtt_handle_guest_write_page_table_bytes(
> -- 
> 2.20.1
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx