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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

As for the other uses,  __get_instruction_length_from_list() checks
against an explicit field while searching the array, priv_op_validate()
breaks on != 3, while x86_insn_is_cr_access() checks >= 0.  All of these
are safe.

~Andrew

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