Re: [Xen-devel] [PATCH 1/4] x86/alternatives: correct near branch check

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.03.16 at 16:43, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/03/16 11:27, Jan Beulich wrote:
>> Make sure the near JMP/CALL check doesn't consume uninitialized
>> data, not even in a benign way. And relax the length check at once.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/x86/alternative.c
>> +++ b/xen/arch/x86/alternative.c
>> @@ -174,7 +174,7 @@ static void __init apply_alternatives(st
>>          memcpy(insnbuf, replacement, a->replacementlen);
>>  
>>          /* 0xe8/0xe9 are relative branches; fix the offset. */
>> -        if ( (*insnbuf & 0xfe) == 0xe8 && a->replacementlen == 5 )
>> +        if ( a->replacementlen >= 5 && (*insnbuf & 0xfe) == 0xe8 )
>>              *(s32 *)(insnbuf + 1) += replacement - instr;
>>  
>>          add_nops(insnbuf + a->replacementlen,
>>
>>
>>
> 
> Swapping the order is definitely a good thing.
> 
> However, relaxing the length check seems less so.  `E8 rel32` or `E9
> rel32` encodings are strictly 5 bytes long.
> 
> There are complications with the `67 E{8,9} rel16` encodings, but those
> are not catered for anyway, and the manual warns about undefined
> behaviour if used in long mode.
> 
> What is your usecase for relaxing the check?  IMO, if it isn't exactly 5
> bytes long, there is some corruption somewhere and the relocation
> should't happen.

The relaxation is solely because at least CALL could validly
be followed by further instructions.

Jan


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