From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhenyu Wang Subject: Re: [PATCH] drm/i915/gvt: Prevent use-after-free in ppgtt_free_all_spt() Date: Thu, 4 Apr 2019 15:48:34 +0800 Message-ID: <20190404074834.GS2322@zhen-hp.sh.intel.com> References: <20190404073056.12407-1-chris@chris-wilson.co.uk> Reply-To: Zhenyu Wang Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1429670063==" Return-path: In-Reply-To: <20190404073056.12407-1-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1429670063== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="lt3WynA+XK9Fj6D4" Content-Disposition: inline --lt3WynA+XK9Fj6D4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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). >=20 > 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. >=20 > Alerted by the sparse warnings: >=20 > drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignme= nt (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] ** > drivers/gpu/drm/i915/gvt/gtt.c:757:9: warning: incorrect type in assignme= nt (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] ** > drivers/gpu/drm/i915/gvt/gtt.c:758:45: warning: incorrect type in argumen= t 1 (different address spaces) > drivers/gpu/drm/i915/gvt/gtt.c:758:45: expected void [noderef] = **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] = **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 assignme= nt (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] ** >=20 > This would also have been loudly warning if run through CI for the > invalid RCU dereferences. >=20 > Fixes: b6c126a39345 ("drm/i915/gvt: Manage shadow pages with radix tree") > Signed-off-by: Chris Wilson > Cc: Changbin Du > Cc: Zhenyu Wang > Cc: Zhi Wang > --- > drivers/gpu/drm/i915/gvt/gtt.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gt= t.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) > =20 > 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; > =20 > + rcu_read_lock(); > radix_tree_for_each_slot(slot, &vgpu->gtt.spt_tree, &iter, 0) { > spt =3D 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 thanks! > static int ppgtt_handle_guest_write_page_table_bytes( > --=20 > 2.20.1 >=20 --=20 Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 --lt3WynA+XK9Fj6D4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EARECAB0WIQTXuabgHDW6LPt9CICxBBozTXgYJwUCXKW20gAKCRCxBBozTXgY JxtOAJ9jq4C/lQ1+Bd3qT1Y19rmLAJyDowCfb/K/kqxdXKlcsvGkBohiITBAsUM= =SvXj -----END PGP SIGNATURE----- --lt3WynA+XK9Fj6D4-- --===============1429670063== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4 --===============1429670063==--