Re: [Xen-devel] [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early

[Andy Lutomirski <luto@xxxxxxxxxxxxxx>]

On Sat, Apr 2, 2016 at 11:52 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Sat, Apr 02, 2016 at 07:01:35AM -0700, Andy Lutomirski wrote:
>> Now that early_fixup_exception has pt_regs, we can just call
>> fixup_exception from it.  This will make fancy exception handlers
>> work early.
>>
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
>> ---
>>  arch/x86/mm/extable.c | 19 ++-----------------
>>  1 file changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
>> index 8997022abebc..50dfe438bd91 100644
>> --- a/arch/x86/mm/extable.c
>> +++ b/arch/x86/mm/extable.c
>> @@ -95,10 +95,6 @@ extern unsigned int early_recursion_flag;
>>  /* Restricted version used during very early boot */
>>  void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>>  {
>> -     const struct exception_table_entry *e;
>> -     unsigned long new_ip;
>> -     ex_handler_t handler;
>> -
>>       /* Ignore early NMIs. */
>>       if (trapnr == X86_TRAP_NMI)
>>               return;
>> @@ -109,19 +105,8 @@ void __init early_fixup_exception(struct pt_regs *regs, 
>> int trapnr)
>>       if (regs->cs != __KERNEL_CS)
>>               goto fail;
>>
>> -     e = search_exception_tables(regs->ip);
>> -     if (!e)
>> -             goto fail;
>> -
>> -     new_ip  = ex_fixup_addr(e);
>> -     handler = ex_fixup_handler(e);
>> -
>> -     /* special handling not supported during early boot */
>> -     if (handler != ex_handler_default)
>> -             goto fail;
>
> Hold on, what happened to the uaccess handling not being supported
> during early boot?
>
> So before Tony changed it, the original code had:
>
>
>  /* Restricted version used during very early boot */
>  int __init early_fixup_exception(unsigned long *ip)
>  {
>
> ...
> -               if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
> -                       /* uaccess handling not supported during early boot */
> -                       return 0;
> -               }
>
> I'm guessing that wasn't supported early, probably because some stuff
> wasn't initialized yet. Our normal, late fixup is by doing:
>
>         current_thread_info()->uaccess_err = 1;
>
> and I'm assuming we can't do that early. current_thread_info is probably
> not setup yet...
>
>> -
>> -     regs->ip = new_ip;
>> -     return;
>> +     if (fixup_exception(regs, trapnr))
>> +             return;
>
> So why can we do it now, all of a sudden?

I have no idea why it was explicitly unsupported, but I'm guessing it
was just to avoid duplicating the code.  Early "ext" uaccess failures
are certainly not going to work, but I don't think this is a problem
-- there's no userspace before trap_init runs, so how exactly is an
"ext" uaccess going to happen in the first place?

In any event, if it did happen in older kernels, it would have
immediately panicked due to that code.  At least with my code it just
might manage to EFAULT correctly.

--Andy

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