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 <ch...@chris-wilson.co.uk>
> Cc: Changbin Du <changbin...@intel.com>
> Cc: Zhenyu Wang <zhen...@linux.intel.com>
> Cc: Zhi Wang <zhi.a.w...@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 <zhen...@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: signature.asc
Description: PGP signature

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

Reply via email to