From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH 08/11] gnttab: remove host map in the event of a grant_map failure Date: Wed, 21 Jun 2017 03:36:53 -0600 Message-ID: <594A5A5502000078001650B6@prv-mh.provo.novell.com> References: <594A57B10200007800165012@prv-mh.provo.novell.com> <594A57B10200007800165012@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part60587E25.3__=" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dNc4H-0000Rh-DL for xen-devel@lists.xenproject.org; Wed, 21 Jun 2017 09:36:57 +0000 In-Reply-To: <594A57B10200007800165012@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: xen-devel Cc: Stefano Stabellini , Wei Liu , George Dunlap , Andrew Cooper , Ian Jackson , Tim Deegan List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part60587E25.3__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline From: George Dunlap The current code appropriately removes the reference and type counts on failure, but leaves the mapping set up. As the only path which can trigger this is failure from IOMMU manipulation, and as unprivileged domains are being crashed in that case, this is not by itself a security issue. Reported-by: Jan Beulich Signed-off-by: George Dunlap Reviewed-by: Jan Beulich --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -764,6 +764,7 @@ __gnttab_map_grant_ref( u32 old_pin; u32 act_pin; unsigned int cache_flags, refcnt =3D 0, typecnt =3D 0; + bool host_map_created =3D false; struct active_grant_entry *act =3D NULL; struct grant_mapping *mt; grant_entry_header_t *shah; @@ -923,6 +924,8 @@ __gnttab_map_grant_ref( cache_flags); if ( rc !=3D GNTST_okay ) goto undo_out; + + host_map_created =3D true; } } else if ( owner =3D=3D rd || owner =3D=3D dom_cow ) @@ -960,6 +963,8 @@ __gnttab_map_grant_ref( rc =3D create_grant_host_mapping(op->host_addr, frame, = op->flags, 0); if ( rc !=3D GNTST_okay ) goto undo_out; + + host_map_created =3D true; } } else @@ -1030,6 +1035,12 @@ __gnttab_map_grant_ref( return; =20 undo_out: + if ( host_map_created ) + { + replace_grant_host_mapping(op->host_addr, frame, 0, op->flags); + gnttab_flush_tlb(ld); + } + while ( typecnt-- ) put_page_type(pg); =20 --=__Part60587E25.3__= Content-Type: text/plain; name="gnttab-remove-host-map.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="gnttab-remove-host-map.patch" gnttab: remove host map in the event of a grant_map failure=0A=0AFrom: = George Dunlap =0A=0AThe current code appropriatel= y removes the reference and type counts=0Aon failure, but leaves the = mapping set up. As the only path which can=0Atrigger this is failure from = IOMMU manipulation, and as unprivileged=0Adomains are being crashed in = that case, this is not by itself a=0Asecurity issue.=0A=0AReported-by: Jan = Beulich =0ASigned-off-by: George Dunlap =0AReviewed-by: Jan Beulich =0A=0A--- = a/xen/common/grant_table.c=0A+++ b/xen/common/grant_table.c=0A@@ -764,6 = +764,7 @@ __gnttab_map_grant_ref(=0A u32 old_pin;=0A = u32 act_pin;=0A unsigned int cache_flags, refcnt =3D 0, = typecnt =3D 0;=0A+ bool host_map_created =3D false;=0A = struct active_grant_entry *act =3D NULL;=0A struct grant_mapping = *mt;=0A grant_entry_header_t *shah;=0A@@ -923,6 +924,8 @@ __gnttab_map_= grant_ref(=0A cache_flags);=0A = if ( rc !=3D GNTST_okay )=0A goto undo_out;=0A+= =0A+ host_map_created =3D true;=0A }=0A }=0A = else if ( owner =3D=3D rd || owner =3D=3D dom_cow )=0A@@ -960,6 +963,8 @@ = __gnttab_map_grant_ref(=0A rc =3D create_grant_host_mapping(op-= >host_addr, frame, op->flags, 0);=0A if ( rc !=3D GNTST_okay = )=0A goto undo_out;=0A+=0A+ host_map_created = =3D true;=0A }=0A }=0A else=0A@@ -1030,6 +1035,12 @@ = __gnttab_map_grant_ref(=0A return;=0A =0A undo_out:=0A+ if ( = host_map_created )=0A+ {=0A+ replace_grant_host_mapping(op->host_= addr, frame, 0, op->flags);=0A+ gnttab_flush_tlb(ld);=0A+ = }=0A+=0A while ( typecnt-- )=0A put_page_type(pg);=0A =0A --=__Part60587E25.3__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --=__Part60587E25.3__=--