linux-kernel - Re: [PATCH v2 7/8] x86/segments/32: Introduce CPU

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]

Date:   Wed, 6 Jun 2018 10:16:14 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     "Bae, Chang Seok" <chang.seok.bae@...el.com>
Cc:     Andrew Lutomirski <luto@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Metzger, Markus T" <markus.t.metzger@...el.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 7/8] x86/segments/32: Introduce CPU_NUMBER segment

On Wed, Jun 6, 2018 at 9:23 AM Chang S. Bae <chang.seok.bae@...el.com> wrote:
>
> The new entry will be equivalent to that of x86-64 which
> stores CPU number. The entry is placed in segment 23 in GDT
> by bumping down 23-28 by one, which are all kernel-internal
> segments and so have no impact on user space.
>
> CPU_NUMBER segment will always be at '%ss (USER_DS) + 80'
> for the default (flat, initial) user space %ss.

No, it won't :(  This is because, on Xen PV, user code very frequently
sees a different, Xen-supplied "flat" SS value.  This is definitely
true right now on 64-bit, and I'm reasonably confident it's also the
case on 32-bit.

As it stands, as far as I can tell, we don't have a "cpu number"
segment on 32-bit kernels.  I see no compelling reason to add one, and
we should definitely not add one as part of the FSGSBASE series.  I
think the right solution is to rename the 64-bit segment to
"CPU_NUMBER" and then have the rearrangement of the initialization
code as a followup patch.  The goal is to make the patches
individually reviewable.  As it stands, this patch adds some #defines
without making them work, which is extra confusing.

Given how many times we screwed it up, I really want the patch that
moves the initialization of the 64-bit CPU number to be obviously
correct and to avoid changing the sematics of anything except the
actual CPU number fields during boot.

So NAK to this patch, at least as part of the FSGSBASE series.

(My apologies -- a bunch of this is because I along with everyone else
misunderstood the existing code.)