Re: xen-4.3.1:hvm.c: 2 * possible bad if tests ?

[Andrew Cooper <andrew.cooper3@citrix.com>]

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