linux-kernel - Re: [PATCH] x86: General protection fault after STR (32 bit systems only)

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: <CALCETrVqMRRXjx9wu-k+TejseeWG0aTXBdJbCK3B-RrUZN-T5g@mail.gmail.com>
Date:	Fri, 12 Jun 2015 11:11:34 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Brian Gerst <brgerst@...il.com>
Cc:	Ingo Molnar <mingo@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
	Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Pavel Machek <pavel@....cz>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>, X86 ML <x86@...nel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86: General protection fault after STR (32 bit systems only)

On Fri, Jun 12, 2015 at 8:48 AM, Brian Gerst <brgerst@...il.com> wrote:
> On Fri, Jun 12, 2015 at 4:36 AM, Ingo Molnar <mingo@...nel.org> wrote:
>>
>> * H. Peter Anvin <hpa@...or.com> wrote:
>>
>>> %es is used implicitly by string instructions.
>>
>> Ok, so we are probably better off reloading ES as well early, right
>> when we return from the firmware, just in case something does
>> a copy before we hit the ES restore in restore_processor_state(),
>> which is a generic C function?
>>
>> Something like the patch below?
>>
>> I also added FS/GS/SS reloading to make it complete. If this (or a variant
>> thereof, it's still totally untested) works then we can remove the segment
>> save/restore layer in __save/restore_processor_state().
>>
>> Thanks,
>>
>>         Ingo
>>
>> ===========>
>>  arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S
>> index 665c6b7d2ea9..1376a7fc21b7 100644
>> --- a/arch/x86/kernel/acpi/wakeup_32.S
>> +++ b/arch/x86/kernel/acpi/wakeup_32.S
>> @@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return)
>>
>>
>>  restore_registers:
>> +       /*
>> +        * In case the BIOS corrupted our segment descriptors,
>> +        * reload them to clear out any shadow descriptor
>> +        * state:
>> +        */
>> +       movl    $__USER_DS, %eax
>> +       movl    %eax, %ds
>> +       movl    %eax, %es
>> +       movl    %eax, %fs
>> +       movl    %eax, %gs
>> +       movl    $__KERNEL_DS, %eax
>> +       movl    %eax, %ss
>> +
>>         movl    saved_context_ebp, %ebp
>>         movl    saved_context_ebx, %ebx
>>         movl    saved_context_esi, %esi
>
> If you follow the convoluted flow of the calls in this file,
> wakeup_pmode_return is the first thing called from the trampoline on
> resume, and that loads the data segments with __KERNEL_DS.  The better
> fix would be to set ds/es to __USER_DS there.  Also since we are in
> the kernel, fs is fixed at __KERNEL_PERCPU, and gs is either
> __KERNEL_STACK_CANARY or user's gs.

So why is this issue rare?  Is it just that the first entry to
userspace after resume is only very rarely via SYSEXIT?  Or is there
some other code path that usually, but not quite always, fixes up the
segment registers?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/