From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dan Carpenter <dan.carpenter@oracle.com>
Subject: re: KVM: x86: Handle errors when RIP is set during far jumps
Date: Mon, 27 Oct 2014 16:42:05 +0300
Message-ID: <20141027134204.GA21267@mwanda>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org
To: namit@cs.technion.ac.il
Return-path: <kvm-owner@vger.kernel.org>
Received: from aserp1040.oracle.com ([141.146.126.69]:18890 "EHLO
	aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751836AbaJ0Nmv (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 27 Oct 2014 09:42:51 -0400
Content-Disposition: inline
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hello Nadav Amit,

The patch d1442d85cc30: "KVM: x86: Handle errors when RIP is set
during far jumps" from Sep 18, 2014, leads to the following static
checker warning:

	arch/x86/kvm/emulate.c:2015 em_jmp_far()
	warn: add some parenthesis here?

arch/x86/kvm/emulate.c
  2013          rc = assign_eip_far(ctxt, ctxt->src.val, new_desc.l);
  2014          if (rc != X86EMUL_CONTINUE) {
  2015                  WARN_ON(!ctxt->mode != X86EMUL_MODE_PROT64);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
No idea what was intended here.  The negate has higher precedence than
the !=.

  2016                  /* assigning eip failed; restore the old cs */
  2017                  ops->set_segment(ctxt, old_sel, &old_desc, 0, VCPU_SREG_CS);
  2018                  return rc;
  2019          }

There are a couple other static checker warnings as well:

	arch/x86/kvm/emulate.c:579 assign_eip_far()
	warn: bitwise AND condition is false here

   567  static inline int assign_eip_far(struct x86_emulate_ctxt *ctxt, ulong dst,
   568                                 int cs_l)
   569  {
   570          switch (ctxt->op_bytes) {
   571          case 2:
   572                  ctxt->_eip = (u16)dst;
   573                  break;
   574          case 4:
   575                  ctxt->_eip = (u32)dst;
   576                  break;
   577          case 8:
   578                  if ((cs_l && is_noncanonical_address(dst)) ||
   579                      (!cs_l && (dst & ~(u32)-1)))
					     ^^^^^^^^
This is a very complicated way of saying zero.

   580                          return emulate_gp(ctxt, 0);
   581                  ctxt->_eip = dst;
   582                  break;
   583          default:
   584                  WARN(1, "unsupported eip assignment size\n");
   585          }
   586          return X86EMUL_CONTINUE;
   587  }

	arch/x86/kvm/emulate.c:2112 em_ret_far()
	warn: add some parenthesis here?

  2110          rc = assign_eip_far(ctxt, eip, new_desc.l);
  2111          if (rc != X86EMUL_CONTINUE) {
  2112                  WARN_ON(!ctxt->mode != X86EMUL_MODE_PROT64);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  2113                  ops->set_segment(ctxt, old_cs, &old_desc, 0, VCPU_SREG_CS);
  2114          }
  2115          return rc;
  2116  }

regards,
dan carpenter