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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALCETrV7PxgmhU_yXjJSugY88T8Do=vBDuG54fOFSyLp7nGcJA@mail.gmail.com>
Date:	Sat, 2 Apr 2016 13:16:07 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	KVM list <kvm@...r.kernel.org>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	xen-devel <Xen-devel@...ts.xen.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early

On Sat, Apr 2, 2016 at 11:52 AM, Borislav Petkov <bp@...en8.de> 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@...nel.org>
>> ---
>>  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