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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <A58C5D0E-A557-4406-9317-442D78E4700B@zytor.com>
Date:	Fri, 12 Jun 2015 01:15:58 -0700
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Ingo Molnar <mingo@...nel.org>,
	Andy Lutomirski <luto@...capital.net>
CC:	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>,
	Brian Gerst <brgerst@...il.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86: General protection fault after STR (32 bit systems only)

%es is used implicitly by string instructions.

On June 12, 2015 12:50:13 AM PDT, Ingo Molnar <mingo@...nel.org> wrote:
>
>* Andy Lutomirski <luto@...capital.net> wrote:
>
>> > --- a/arch/x86/kernel/acpi/wakeup_32.S
>> > +++ b/arch/x86/kernel/acpi/wakeup_32.S
>> > @@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel)
>> >         jmp     ret_point
>> >         .p2align 4,,7
>> >  ret_point:
>> > +       /* In case the BIOS corrupted DS, make the kernel context
>minimally functional: */
>> > +       movl    $__KERNEL_DS, %eax
>> > +       movl    %eax, %ds
>> > +
>> 
>> On further thought, I think you want movl $__USER_DS, %eax.  The
>> 32-bit kernel is a strange beast.  Also, you should probably fix up
>> %es as well.
>
>So restore_processor_state() already restores ES. The idea here was to
>reload DS 
>early on, because the kernel implicitly uses it for data access so we
>need it to 
>be good to be able to continue executing any generic kernel code.
>
>We don't use %es: prefixed assembly AFAICS, what are the implicit users
>of ES?
>
>Also, to further confuse things, we also have:
>
>ENTRY(wakeup_pmode_return)
>wakeup_pmode_return:
>        movw    $__KERNEL_DS, %ax
>        movw    %ax, %ss
>        movw    %ax, %ds
>        movw    %ax, %es
>        movw    %ax, %fs
>        movw    %ax, %gs
>
>        # reload the gdt, as we need the full 32 bit address
>        lidt    saved_idt
>        lldt    saved_ldt
>        ljmp    $(__KERNEL_CS), $1f
>1:
>        movl    %cr3, %eax
>        movl    %eax, %cr3
>        wbinvd
>
>which seems to be another layer of restoration - but it possibly does
>not trigger 
>in the S2RAM case here.
>
>Oh, funny the 'reload the gdt' comment: do you see an LGDT there? It
>reloads all 
>segment selectors, the IDT, LDT and CR3, but does not seem to reload
>the GDT - the 
>only thing the comment describes.
>
>Thanks,
>
>	Ingo

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
--
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/