Re: [Xen-devel] [PATCH] x86/svm: Adjust ModRM Mode check in is_invlpg()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/01/17 11:46, Jan Beulich wrote:
>>>> On 12.01.17 at 12:31, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 12/01/17 10:09, Jan Beulich wrote:
>>>>>> On 11.01.17 at 18:33, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> Coverity points out that x86_insn_modrm() returns -EINVAL for instructions 
>>>> not
>>>> encoded with a ModRM byte.  A consequence is that checking != 3 is
>>>> insufficient to confirm that &ext was actually written to.
>>>>
>>>> In practice, this check is only used after decode has been successful, and
>>>> 0f01 will have a ModRM byte.
>>> I think there may be one or two more such instances elsewhere.
>> I did audit the other callsites, and thought I found them to be safe.
>>
>> However, thinking again, I think the gate_op case isn't
>>
>>     case 0xff:
>>         if ( x86_insn_modrm(state, NULL, &modrm_345) >= 3 )
>>             break;
>>         switch ( modrm_345 & 7 )
>>
>> This also needs to be an unsigned comparison to catch the -EINVAL case.
> Right. However, how about using 3U instead of a cast to achieve this?

That is very subtle, and will go wrong silently if x86_insn_modrm()
changes return type, e.g. to long.

In this case, the explicit cast is much clearer, as it shows an obvious
intent.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel