Re: [Xen-devel] [PATCH v2] x86/emul: Poision the stubs with debug traps

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/03/17 11:50, Jan Beulich wrote:
>>>> On 20.03.17 at 11:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>> ...rather than leaving fragments of old instructions in place.  This reduces
>> the chances of something going further-wrong (as the debug trap will be 
>> caught
>> and terminate the guest) in a cascade-failure where we end up executing the
>> instruction fragments.
>>
>> Before:
>>     (XEN) d2v0 exception 6 (ec=0000) in emulation stub (line 6239)
>>     (XEN) d2v0 stub: c4 e1 44 77 c3 80 d0 82 ff ff ff d1 90 ec 90
>>
>> After:
>>     (XEN) d3v0 exception 6 (ec=0000) in emulation stub (line 6239)
>>     (XEN) d3v0 stub: c4 e1 44 77 c3 cc cc cc cc cc cc cc cc cc cc
>>
>> To make this work, the int3 handler needs to be extended to attempt recovery
>> rather than simply returning back to Xen context.  This is a good general
>> precaution anyway, as nothing good will happen from letting Xen blindly
>> execute over 0xcc's.
> I partly disagree: It can be a very useful thing to not have to rebuild
> Xen with hard coded INT3 invocations removed/re-added between
> runs with or without a debugger attached.

What possible usecase is this for?  If you don't already modify
do_int3(), use of embedded int3's are of no use.

>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1206,9 +1206,21 @@ void do_int3(struct cpu_user_regs *regs)
>>  
>>      if ( !guest_mode(regs) )
>>      {
>> -        debugger_trap_fatal(TRAP_int3, regs);
>> -        return;
>> -    } 
>> +        unsigned long fixup;
>> +
>> +        if ( (fixup = search_exception_table(regs)) != 0 )
>> +        {
>> +            this_cpu(last_extable_addr) = regs->rip;
>> +            regs->rip = fixup;
> Should this be accompanied by a dprintk(), like most other such
> cases are?

Perhaps.  I'm not fussed, and it should be a rare occurrence.

>
>> +            return;
>> +        }
>> +
>> +        if ( debugger_trap_fatal(TRAP_int3, regs) )
>> +            return;
>> +
>> +        show_execution_state(regs);
>> +        panic("FATAL TRAP: vector = %d (int3)", TRAP_int3);
> Along the lines of what I've said above, I'd like to see this being made
> conditional upon NDEBUG.

Without some further explanation, I don't buy it as a plausible scenario.

> Plus I think you want to call fatal_trap() here.

fatal_trap() however would be better.

~Andrew

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