linux-kernel - Re: [PATCH v2 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

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: <CALCETrXwVjHoOiH_LBpnZfbAGbBUMLM0V1-HtcwQbYNiGZ+-9g@mail.gmail.com>
Date:	Wed, 11 Mar 2015 05:22:48 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Andi Kleen <andi@...stfloor.org>, "H. Peter Anvin" <hpa@...or.com>,
	X86 ML <x86@...nel.org>, Al Viro <viro@...iv.linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH v2 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

On Mar 11, 2015 5:29 AM, "Borislav Petkov" <bp@...en8.de> wrote:
>
> On Tue, Mar 10, 2015 at 07:03:25AM -0700, Andy Lutomirski wrote:
> > As far as I can tell, these fields have been set to zero on save and
> > ignored on restore since Linux was imported into git.  Rename them
>
> Actually from the very beginning:
>
> commit 47f16da277d10ef9494f3e9da2a9113bb22bcd75
> Author: Andi Kleen <ak@....de>
> Date:   Tue Feb 12 20:17:35 2002 -0800
>
>     [PATCH] x86_64 merge: arch + asm
>
> ...
>
> +static int
> +setup_sigcontext(struct sigcontext *sc, struct _fpstate *fpstate,
> +                struct pt_regs *regs, unsigned long mask)
> +{
> +       int tmp, err = 0;
> +
> +       tmp = 0;
> +       __asm__("movl %%gs,%0" : "=r"(tmp): "0"(tmp));
> +       err |= __put_user(tmp, (unsigned int *)&sc->gs);
> +       __asm__("movl %%fs,%0" : "=r"(tmp): "0"(tmp));
> +       err |= __put_user(tmp, (unsigned int *)&sc->fs);
>

Wait... It looks like it really was saving them.  Of course, this is
mostly useless unless the code also does something very clever with
fsbase and gsbase.

After wrestling with the historic git a bit:

commit a104ba57b3b3697fd77778918b6a77ed16bb2ff8
Author: Andi Kleen <ak@....de>
Date:   Mon Mar 10 01:41:56 2003 -0800

    [PATCH] x86-64 updates for 2.5.64-bk3

...

 static inline int
 setup_sigcontext(struct sigcontext *sc, struct pt_regs *regs,
unsigned long mask, struct task_struct *me)
 {
-       int tmp, err = 0;
+       int err = 0;

-       tmp = 0;
-       __asm__("movl %%gs,%0" : "=r"(tmp): "0"(tmp));
-       err |= __put_user(tmp, (unsigned int *)&sc->gs);
-       __asm__("movl %%fs,%0" : "=r"(tmp): "0"(tmp));
-       err |= __put_user(tmp, (unsigned int *)&sc->fs);
+       err |= __put_user(0, &sc->gs);
+       err |= __put_user(0, &sc->fs);

Prior to that, we actually saved and restored the thing.  In restore_context:

        {
                unsigned int seg;
                err |= __get_user(seg, &sc->gs);
                load_gs_index(seg);
                err |= __get_user(seg, &sc->fs);
                loadsegment(fs,seg);
        }

We did not, however, save and restore fsbase and gsbase.

Interestingly, in the same BK change, we also remove set_thread_area
as a 64-bit syscall entirely (take that, ABI compatibility!), but
arch_prctl existed before that change.

IOW, in 2.5.63 and earlier, we tried to save and restore TLS state
across signals, but we did it wrong and would corrupt it for any
program that used arch_prctl.  At that time, programs could switch
userspace threads using signals and their TLS pointers would switch.
In 2.5.64 and later, we broke set_thread_area users, fixed arch_prctl
users, and made signals effectively *not* switch TLS pointers.

It seems unlikely to me that many pre-2.5.64 programs still run due to
the aforementioned removal of a syscall, but I think that we should at
least document this in the header so that anyone who tries to recycle
those padding bits is appropriately careful.

--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/