linux-kernel - Re: [PATCH] arch: x86: power: cpu: init %gs before __restore_processor

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] [day] [month] [year] [list]

Date:   Sun, 4 Oct 2020 11:59:43 +0200
From:   Pavel Machek <pavel@...x.de>
To:     Roman Kiryanov <rkir@...gle.com>
Cc:     Pavel Machek <pavel@...x.de>, Borislav Petkov <bp@...en8.de>,
        rjw@...ysocki.net, Thomas Gleixner <tglx@...utronix.de>,
        mingo@...hat.com, x86@...nel.org, linux-pm@...r.kernel.org,
        Greg KH <gregkh@...uxfoundation.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Alistair Delva <adelva@...gle.com>,
        Haitao Shan <hshan@...gle.com>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arch: x86: power: cpu: init %gs before
 __restore_processor_state (clang)

Hi!

> > > > > I believe the kernel makes a questionable assumption on how clang
> > > > > uses registers (gs will not be used if stack protection is disabled).
> > > > > Both kernel and clang behaves unfortunate here.
> > > >
> > > > If the kernel is at fault here and this same thing happens with GCC,
> > > > sure, but this is a clang-specific fix.
> > >
> > > This is fair. Unfortunately I am not an x86 asm expert. I expect the proper
> > > fix should land into arch/x86/kernel/acpi/wakeup_64.S to init %gs
> > > (maybe some more registers) before "jmp restore_processor_state".
> >
> > That would certainly be nicer / more acceptable solution than patch
> > being proposed here.
> >
> > Code was written with assumption compiler random C code would not use
> > %gs. If that's no longer true, fixing it in wakeup_64.S _with comments
> > explaining what goes on_ might be solution.
> 
> I looked and restore_processor_state is referenced in several places,
> so changing
> wakeup_64.S is not enough. Is moving the beginning of restore_processor_state
> to an .S file ok? I see restore_processor_state initializes CR registers first,
> do you know if there is a reason to do so (can I init segment
> registers before CR ones)?

Yes, moving to .S file should be okay.

CR first... makes sense to me, they really select how segment registers will be
interpretted, etc. If it will work the other way... not sure.

I'd keep existing code if I were you. This is tricky to debug.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html