From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: xen-4.3.1:hvm.c: 2 * possible bad if tests ? Date: Thu, 21 Nov 2013 11:54:06 +0000 Message-ID: <528DF45E.7050905@citrix.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VjSpl-0003ln-PU for xen-devel@lists.xenproject.org; Thu, 21 Nov 2013 11:54:09 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Binderman Cc: "xen-devel@lists.xenproject.org" List-Id: xen-devel@lists.xenproject.org On 21/11/13 11:45, David Binderman wrote: > Hello there, > > I just ran the source code of xen-4.3.1 through the static analyser "cppcheck". > > It said > > 1. > > [hvm.c:2190]: (style) Expression '(X & 0xc00) != 0x6' is always true. > > Source code is > > if ( ((desc.b & (6u<<9)) != 6) && (dpl != rpl) ) > goto unmap_and_fail; > > You might be better off with > > if ( ((desc.b & (6u<<9))) && (dpl != rpl) ) > goto unmap_and_fail; > > 2. > > [hvm.c:2210]: (style) Expression '(X & 0xc00) != 0x6' is always true. > > Source code is > > if ( ((desc.b & (6u<<9)) != 6) && ((dpl < cpl) || (dpl < rpl)) ) > goto unmap_and_fail; These have both been flagged up by our Coverity scanning, but I haven't had enough time to pour over the manuals workout out the correct expression should be. The prevailing style for all other checks in this area is "(X & (6u<<9)) != (6u<<9)" , which is rather different to the result you came up with. As this is the security checks for segment selectors in the emulation code, leaving it in its current "too many operations are failed" is safer than being uncertain with the fix and introducing a vulnerability. ~Andrew