linux-kernel - Re: syscall_get_error() && TS

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: <CA+55aFwEm0A7SkRbAKpgkD+eM3hG99e_zgxhkr5pm0kd2pJYmw@mail.gmail.com>
Date:   Thu, 30 Mar 2017 10:46:19 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Jan Kratochvil <jan.kratochvil@...hat.com>,
        Pedro Alves <palves@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: syscall_get_error() && TS_ checks

On Thu, Mar 30, 2017 at 8:49 AM, Oleg Nesterov <oleg@...hat.com> wrote:
>>
>>       case offsetof(struct user32, regs.orig_eax):
>                                      ^^^^^^^^^^^^^^
> damn, just in case, of course this is typo, should be
>
>         case offsetof(struct user32, regs.eax);

So honestly, the first version of your patch I went "Ok, that looks
perfectly reasonable", because orig_eax is special, and always
sign-extending it sounds fine.

The *fixed* version of your patch I go "no, that's obviously complete
garbage, that's completely bogus".

Because the regular eax register is *not* special, and sign-extending
it sounds very dangerous indeed. The user will actually *use* that
register as a register, unlike orig_eax. And eax is no different from
all the other random registers. Yes, it happens to be the return value
for system calls, but this codepath is not at all limited to system
calls, it's absolutely *any* context when you use gdb on a 32-bit
binary.

Now, you may well say that "the upper bits aren't well-defined in that
case anyway". And you'd be mostly right. Except the semantics of
x86-64 is that 32-bit operations zero-extend the upper bits, so
zero-extension really *is* the right thing to do.

For example, let's assume that %eax contains a 32-bit pointer with the
high bit set, and we're using a 32-bit debugger on a 32-bit program
(ie you're just running a 32-bit distro on a 64-bit kernel, which
people have definitely done).

We *really* shouldn't sign-extend that value if the debugger ends up
updating the pointer (or maybe the debugger just reloads previous
values, not really "updating" anything - I think that's what gdb does
when you do a call within the context of the debugged program from
within gdb, for example)

So I really *really* don't think you can just sign-extend %eax. Which
is exactly why we have that nasty odd sign-extension in the signal
path instead, but then have to make it conditional on running a 32-bit
program.

But maybe there is still something I'm not understanding in your
argument. This thread has been a series of mis-understandings.

               Linus