Re: [Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.02.16 at 13:12, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 11/02/16 11:16, Jan Beulich wrote:
>>>>> On 11.02.16 at 11:52, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -114,6 +114,56 @@ boolean_param("ler", opt_ler);
>>>  #define stack_words_per_line 4
>>>  #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
>>>  
>>> +static void show_code(const struct cpu_user_regs *regs)
>>> +{
>>> +    unsigned char insns_before[8], insns_after[16];
>>> +    unsigned int i, missing_before, missing_after;
>>> +
>>> +    if ( guest_mode(regs) )
>>> +        return;
>>> +
>>> +    /*
>>> +     * This dance with {insns,missing}_{before,after} is to ensure that, if
>>> +     * %rip -8/+16 wraps around a bounday, we read a non-wrapped regs->rip
>>> +     * pointer, and calculate which bytes were not read so they may be
>>> +     * replaced with dashes in the printed output.
>>> +     */
>>> +    missing_before = __copy_from_user(
>>> +        insns_before, (void __user *)regs->rip - 8, 
>>> ARRAY_SIZE(insns_before));
>>> +    missing_after = __copy_from_user(
>>> +        insns_after, (void __user *)regs->rip, ARRAY_SIZE(insns_after));
>> ... iirc __copy_from_user() doesn't range check the addresses.
>> Also reading the leading bytes is done in kind of a strange way: It'll
>> read initial bytes (farther away from RIP) and perhaps not read
>> later ones (closer to RIP), albeit clearly the ones closer are of more
>> interest. In the extreme case, where RIP is only a few bytes into a
>> page following an unmapped one, no leading bytes would be printed
>> at all despite some actually being readable.
> 
> I think in this specific case, it might be best to hand roll some asm
> using rep movs and the direction flag, with manual fault handling.

That's certainly an option.

>> Avoiding actual wrapping could be easily done by extending the
>> guest_mode() check above to also range check RIP against the
>> hypervisor image boundaries (post-boot this could even be limited
>> further, but perhaps using the full XEN_VIRT_{START,END} range is
>> the better route with xSplice in mind.
> 
> I would like, where possible, to avoid omitting the instruction stream
> if Xen is outside of its expected boundaries.

Which is because of ...? What useful information do you think can
be gained from the actual instruction when the mere fact of being
outside the boundaries is a bug?

Jan


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