Re: [Xen-devel] [PATCH 06/10] tools/insn-fuzz: Correct hook prototypes, and assert() appropriate segments

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/17 12:48, Jan Beulich wrote:
>>>> On 27.03.17 at 11:56, <andrew.cooper3@xxxxxxxxxx> wrote:
>> The correct prototypes for the hooks are to use enum x86_segment rather than
>> unsigned int.  It is implementation specific as to whether this compiles.
> I'm actually surprised this has worked so far. We should fix the test
> harness in the same way.

Oh yes.  I will fix that as well.

>
>> @@ -235,27 +246,37 @@ static int fuzz_rep_stos(
>>      unsigned long *reps,
>>      struct x86_emulate_ctxt *ctxt)
>>  {
>> +    /*
>> +     * STOS itself may only have an %es segment, but the stos() hook is 
>> reused
>> +     * for CLZERO.
>> +     */
>> +    assert(is_x86_user_segment(seg));
> Perhaps worth looking at ctxt->opcode?

I considered that but chose not to.  I think starting to special case
like that might get unwieldy.

>
>>  static int fuzz_cmpxchg(
>> -    unsigned int seg,
>> +    enum x86_segment seg,
>>      unsigned long offset,
>>      void *old,
>>      void *new,
>>      unsigned int bytes,
>>      struct x86_emulate_ctxt *ctxt)
>>  {
>> +    assert((unsigned int)seg < x86_seg_none);
> I guess this could be slightly more strict, not allowing IDTR and TR.
> Perhaps then also for the write handler.

Hmm - good point.  We have no architectural reason to perform a
cmpxchg() targeting the IDT or TR.

OTOH, we have no architectural reason to ever write to any of the system
segments, so that side of things can be stricter.

~Andrew

>
> Other than the above (which are only suggestions)
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> Jan
>


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