From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86emul: special case far branch validation outside of long mode Date: Fri, 11 Mar 2016 09:01:21 -0700 Message-ID: <56E2F9E102000078000DBAD6@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__PartF6C14FC1.1__=" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1aePVF-0003hM-1d for xen-devel@lists.xenproject.org; Fri, 11 Mar 2016 16:01:25 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: xen-devel Cc: Andrew Cooper , Keir Fraser 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. --=__PartF6C14FC1.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline In that case (with the new value being held in, or now in one case cast to, a 32-bit variable) there's no need to go through the long mode part of the checks. Primarily this was meant to hopefully address Coverity ID 1355278, but since the change produces smaller code as well I think we should use it even if it doesn't help that (benign) warning. Also it's more in line with jmp_rel() for commit_far_branch() to do the _regs.eip update, so adjust that at once. Signed-off-by: Jan Beulich --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -652,13 +652,20 @@ do { _regs.eip =3D ip; = \ } while (0) =20 -#define validate_far_branch(cs, ip) \ - generate_exception_if(in_longmode(ctxt, ops) && (cs)->attr.fields.l \ - ? !is_canonical_address(ip) \ - : (ip) > (cs)->limit, EXC_GP, 0) +#define validate_far_branch(cs, ip) ({ \ + if ( sizeof(ip) <=3D 4 ) { = \ + ASSERT(in_longmode(ctxt, ops) <=3D 0); = \ + generate_exception_if((ip) > (cs)->limit, EXC_GP, 0); \ + } else \ + generate_exception_if(in_longmode(ctxt, ops) && \ + (cs)->attr.fields.l \ + ? !is_canonical_address(ip) \ + : (ip) > (cs)->limit, EXC_GP, 0); \ +}) =20 #define commit_far_branch(cs, ip) ({ \ validate_far_branch(cs, ip); \ + _regs.eip =3D (ip); = \ ops->write_segment(x86_seg_cs, cs, ctxt); \ }) =20 @@ -2787,7 +2794,6 @@ x86_emulate( (rc =3D load_seg(x86_seg_cs, src.val, 1, &cs, ctxt, ops)) || (rc =3D commit_far_branch(&cs, dst.val)) ) goto done; - _regs.eip =3D dst.val; break; } =20 @@ -2830,9 +2836,8 @@ x86_emulate( eflags &=3D 0x257fd5; _regs.eflags &=3D mask; _regs.eflags |=3D (eflags & ~mask) | 0x02; - _regs.eip =3D eip; if ( (rc =3D load_seg(x86_seg_cs, sel, 1, &cs, ctxt, ops)) || - (rc =3D commit_far_branch(&cs, eip)) ) + (rc =3D commit_far_branch(&cs, (uint32_t)eip)) ) goto done; break; } @@ -3465,7 +3470,6 @@ x86_emulate( if ( (rc =3D load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || (rc =3D commit_far_branch(&cs, eip)) ) goto done; - _regs.eip =3D eip; break; } =20 @@ -3767,11 +3771,11 @@ x86_emulate( &_regs.eip, op_bytes, ctxt)) || (rc =3D ops->write_segment(x86_seg_cs, &cs, ctxt)) ) goto done; + _regs.eip =3D src.val; } else if ( (rc =3D load_seg(x86_seg_cs, sel, 0, &cs, ctxt, = ops)) || (rc =3D commit_far_branch(&cs, src.val)) ) goto done; - _regs.eip =3D src.val; =20 dst.type =3D OP_NONE; break; --=__PartF6C14FC1.1__= Content-Type: text/plain; name="x86emul-coverity-warning.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86emul-coverity-warning.patch" x86emul: special case far branch validation outside of long mode=0A=0AIn = that case (with the new value being held in, or now in one case cast=0Ato, = a 32-bit variable) there's no need to go through the long mode part=0Aof = the checks.=0A=0APrimarily this was meant to hopefully address Coverity ID = 1355278, but=0Asince the change produces smaller code as well I think we = should use it=0Aeven if it doesn't help that (benign) warning.=0A=0AAlso = it's more in line with jmp_rel() for commit_far_branch() to do the=0A_regs.= eip update, so adjust that at once.=0A=0ASigned-off-by: Jan Beulich = =0A=0A--- a/xen/arch/x86/x86_emulate/x86_emulate.c=0A+++= b/xen/arch/x86/x86_emulate/x86_emulate.c=0A@@ -652,13 +652,20 @@ do {=0A = _regs.eip =3D ip; = \=0A } while (0)=0A =0A-#define validate_far_branch(cs, ip) = \=0A- generate_exception_if(in_longmode(ctxt, ops) = && (cs)->attr.fields.l \=0A- ? !is_canonical_addre= ss(ip) \=0A- : (ip) > (cs)->limi= t, EXC_GP, 0)=0A+#define validate_far_branch(cs, ip) ({ = \=0A+ if ( sizeof(ip) <=3D 4 ) { = \=0A+ ASSERT(in_longmode(ctxt, ops) <=3D 0); = \=0A+ generate_exception_if((ip) > (cs)->limit= , EXC_GP, 0); \=0A+ } else = \=0A+ generate_exception_if(in_longmode(ct= xt, ops) && \=0A+ (cs)->attr.f= ields.l \=0A+ ? = !is_canonical_address(ip) \=0A+ = : (ip) > (cs)->limit, EXC_GP, 0); \=0A+})=0A =0A #define commit_far= _branch(cs, ip) ({ \=0A validate_far= _branch(cs, ip); \=0A+ _regs.eip = =3D (ip); \=0A = ops->write_segment(x86_seg_cs, cs, ctxt); \=0A = })=0A =0A@@ -2787,7 +2794,6 @@ x86_emulate(=0A (rc =3D = load_seg(x86_seg_cs, src.val, 1, &cs, ctxt, ops)) ||=0A (rc = =3D commit_far_branch(&cs, dst.val)) )=0A goto done;=0A- = _regs.eip =3D dst.val;=0A break;=0A }=0A =0A@@ -2830,9 = +2836,8 @@ x86_emulate(=0A eflags &=3D 0x257fd5;=0A = _regs.eflags &=3D mask;=0A _regs.eflags |=3D (eflags & ~mask) | = 0x02;=0A- _regs.eip =3D eip;=0A if ( (rc =3D load_seg(x86_se= g_cs, sel, 1, &cs, ctxt, ops)) ||=0A- (rc =3D commit_far_branch= (&cs, eip)) )=0A+ (rc =3D commit_far_branch(&cs, (uint32_t)eip)= ) )=0A goto done;=0A break;=0A }=0A@@ -3465,7 = +3470,6 @@ x86_emulate(=0A if ( (rc =3D load_seg(x86_seg_cs, sel, = 0, &cs, ctxt, ops)) ||=0A (rc =3D commit_far_branch(&cs, = eip)) )=0A goto done;=0A- _regs.eip =3D eip;=0A = break;=0A }=0A =0A@@ -3767,11 +3771,11 @@ x86_emulate(=0A = &_regs.eip, op_bytes, ctxt)) ||=0A = (rc =3D ops->write_segment(x86_seg_cs, &cs, ctxt)) )=0A = goto done;=0A+ _regs.eip =3D src.val;=0A = }=0A else if ( (rc =3D load_seg(x86_seg_cs, sel, 0, &cs, ctxt, = ops)) ||=0A (rc =3D commit_far_branch(&cs, src.val)) = )=0A goto done;=0A- _regs.eip =3D src.val;=0A = =0A dst.type =3D OP_NONE;=0A break;=0A --=__PartF6C14FC1.1__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --=__PartF6C14FC1.1__=--