Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180621211754.12757-1-h.peter.anvin@intel.com> <20180621211754.12757-2-h.peter.anvin@intel.com>
In-Reply-To: <20180621211754.12757-2-h.peter.anvin@intel.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Fri, 22 Jun 2018 07:24:56 -0700
Message-ID: <CALCETrWerhEUMw+JS4p5MVEopiMnUjbUtX58Ci7UYfYT=Y5Cvw@mail.gmail.com>
Subject: Re: [PATCH v3 1/7] x86/ldt: refresh %fs and %gs in refresh_ldt_segments()
To:     "H. Peter Anvin" <h.peter.anvin@intel.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        "H. Peter Anvin" <hpa@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Andrew Lutomirski <luto@kernel.org>,
        "Bae, Chang Seok" <chang.seok.bae@intel.com>,
        "Metzger, Markus T" <markus.t.metzger@intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jun 21, 2018 at 2:18 PM H. Peter Anvin, Intel
<h.peter.anvin@intel.com> wrote:
>
> From: "H. Peter Anvin" <hpa@linux.intel.com>
>
> It is not only %ds and %es which contain cached user descriptor
> information, %fs and %gs do as well.
>
> To make sure we don't do something stupid that will affect processes
> which wouldn't want this requalification, be more restrictive about
> which selector numbers will be requalified: they need to be LDT
> selectors (which by definition are never null), have an RPL of 3
> (always the case in user space unless null), and match the updated
> descriptor.

That RPL3 part is false.  The following program does:

#include <stdio.h>

int main()
{
    unsigned short sel;
    asm volatile ("mov %%ss, %0" : "=rm" (sel));
    sel &= ~3;
    printf("Will write 0x%hx to GS\n", sel);
    asm volatile ("mov %0, %%gs" :: "rm" (sel & ~3));
    asm volatile ("mov %%gs, %0" : "=rm" (sel));
    printf("GS = 0x%hx\n", sel);
    return 0;
}

prints:

Will write 0x28 to GS
GS = 0x28

The x86 architecture is *insane*.

Other than that, this patch seems generally sensible.  But my
objection that it's incorrect with FSGSBASE enabled for %fs and %gs
still applies.