Re: [Xen-devel] [PATCH 1/3] x86/HVM: restrict emulation in hvm_descriptor_access_intercept()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.04.17 at 16:59, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 13/04/17 15:51, Jan Beulich wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3598,6 +3598,28 @@ gp_fault:
>>      return X86EMUL_EXCEPTION;
>>  }
>>  
>> +static bool is_sysdesc_access(const struct x86_emulate_state *state,
>> +                              const struct x86_emulate_ctxt *ctxt)
>> +{
>> +    unsigned int ext;
>> +    int mode = x86_insn_modrm(state, NULL, &ext);
> 
> Unfortunately, this is another example which Coverity will pick up on,
> along with the use of x86_insn_modrm() in is_invlpg().
> 
> In the case that we return -EINVAL, ext is uninitialised when it gets
> used below.

Sigh. Yes, we can work around this tool limitation, but honestly I
don't really agree doing so in cases like this (where the code is
clearly correct, as -EINVAL can't come back). Plus we have
precedent to the above other than just in is_invlpg():
priv_op_validate() does the same, as does emulate_gate_op().
If there was a way to work around the warnings without growing
generated code, I'd be less opposed (but still not entirely in
agreement), but all I can see is either making the return value
check more involved or adding initializers. In neither case the
compiler will be able to eliminate the resulting insns.

Jan


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