linux-kernel - Re: [PATCH] x86/entry/64/compat: Preserve r8-r11 in int $0x80

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: <CALCETrU3KLXwToBTOTnHpKrUhENFUVFJbWxfiPMbs2iQHoDDZA@mail.gmail.com>
Date:   Wed, 18 Apr 2018 09:53:16 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Denys Vlasenko <dvlasenk@...hat.com>
Cc:     Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>,
        Dominik Brodowski <linux@...inikbrodowski.net>
Subject: Re: [PATCH] x86/entry/64/compat: Preserve r8-r11 in int $0x80

On Tue, Apr 17, 2018 at 8:00 AM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
> This means that the new behavior is there for some 8 years already.
> Whoever was impacted by it, probably already switched to the new ABI.
>
> Current ABI is "weaker", it allows kernel to save fewer registers.
>
> Which is generally a good thing, since saving/restoring things cost
> cycles, and sometimes painful on entry paths where you may desperately
> need a scratch register or two. (Recall this one? -
> ...
>         movq    %rsp, PER_CPU_VAR(rsp_scratch)
>         movq    PER_CPU_VAR(cpu_current_top_of_stack), %rsp
>         /* Construct struct pt_regs on stack */
>         pushq   $__USER_DS                      /* pt_regs->ss */
>         pushq   PER_CPU_VAR(rsp_scratch)        /* pt_regs->sp */
> ...
> wouldn't it be _great_ if one of GPRs would be available here
> to hold userspace %rsp?
> )

But this is the int $0x80 entry, which uses the stack sanely and
doesn't have this problem at all.

>
> If userspace needs some registers saved, it's trivial for it to have:
>
>         push reg1
>         push reg2
>         int  0x80
>         pop  reg2
>         pop  reg1
>
> OTOH if userspace _does not_ need some registers saved,
> but they are defined as saved by the entrypoint ABI, then save/restore work
> is done every time, even when not needed.
>
> Thus, I propose to retain the current behavior.

The problems are:

1. If you look up how to do int $0x80, every answer you get doesn't
mention any clobbers.  The code works on x86_32 and seems to work on
x86_64.  I think we should make it actually work.

2. It's very easy to make this mistake and get away with it for a long
time, and the failure modes are hard to debug.  gcc doesn't allocate
r8..r11 that often, and there are plenty of contexts (near end of a
leaf function) where r8..r11 are dead even if they were allocated.  So
there is probably a decent body of code out there that makes this
mistake and is okay for now.  But if anyone ever compiles it with LTO,
it's reasonably likely to go boom.

So I think we should fix it in the interest of avoiding weird bugs.