linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Matz <matz@suse.de>
To: Willy Tarreau <w@1wt.eu>
Cc: Borislav Petkov <bp@alien8.de>,
	Ammar Faizi <ammar.faizi@students.amikom.ac.id>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list
Date: Wed, 13 Oct 2021 16:24:28 +0000 (UTC)	[thread overview]
Message-ID: <alpine.LSU.2.20.2110131601000.26294@wotan.suse.de> (raw)
In-Reply-To: <20211013142433.GB8557@1wt.eu>

Hello,

On Wed, 13 Oct 2021, Willy Tarreau wrote:

> On Wed, Oct 13, 2021 at 04:20:55PM +0200, Borislav Petkov wrote:
> > On Wed, Oct 13, 2021 at 04:07:23PM +0200, Willy Tarreau wrote:
> > > Yes I agree with the "potentially" here. If it can potentially be (i.e.
> > > the kernel is allowed by contract to later change the way it's currently
> > > done) then we have to save them even if it means lower code efficiency.
> > >
> > > If, however, the kernel performs such savings on purpose because it is
> > > willing to observe a stricter saving than the AMD64 ABI, we can follow
> > > it but only once it's written down somewhere that it is by contract and
> > > will not change.
> > 
> > Right, and Micha noted that such a change to the document can be done.
> 
> great.
> 
> > And we're basically doing that registers restoring anyway, in POP_REGS.
> 
> That's what I based my analysis on when I wanted to verify Ammar's
> finding. I would tend to think that if we're burning cycles popping
> plenty of registers it's probably for a reason, maybe at least a good
> one, which is that it's the only way to make sure we're not leaking
> internal kernel data! This is not a concern for kernel->kernel nor
> user->user calls but for user->kernel calls it definitely is one, and
> I don't think we could relax that series of pop without causing leaks
> anyway.

It might also be interesting to know that while the wording of the psABI 
was indeed intended to imply that all argument registers are potentially 
clobbered (like with normal calls) glibc's inline assembler to call 
syscalls relies on most registers to actually be preserved:

# define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
...
#define internal_syscall6(number, arg1, arg2, arg3, arg4, arg5, arg6) \
({                                                                      \
    unsigned long int resultvar;                                        \
    TYPEFY (arg6, __arg6) = ARGIFY (arg6);                              \
    TYPEFY (arg5, __arg5) = ARGIFY (arg5);                              \
    TYPEFY (arg4, __arg4) = ARGIFY (arg4);                              \
    TYPEFY (arg3, __arg3) = ARGIFY (arg3);                              \
    TYPEFY (arg2, __arg2) = ARGIFY (arg2);                              \
    TYPEFY (arg1, __arg1) = ARGIFY (arg1);                              \
    register TYPEFY (arg6, _a6) asm ("r9") = __arg6;                    \
    register TYPEFY (arg5, _a5) asm ("r8") = __arg5;                    \
    register TYPEFY (arg4, _a4) asm ("r10") = __arg4;                   \
    register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                   \
    register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                   \
    register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                   \
    asm volatile (                                                      \
    "syscall\n\t"                                                       \
    : "=a" (resultvar)                                                  \
    : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4),         \
      "r" (_a5), "r" (_a6)                                              \
    : "memory", REGISTERS_CLOBBERED_BY_SYSCALL);                        \
    (long int) resultvar;                                               \
})


Note in particular the missing clobbers or outputs of any of the argument 
regs.

So, even though the psABI (might have) meant something else, as glibc is 
doing the above we in fact have a de-facto standard that the kernel can't 
clobber any of the argument regs.  The wording and the linux x86-64 
syscall implementation (and use in glibc) all come from the same time in 
2001, so there never was a time when the kernel was not saving/restoring 
the arg registers, so it can't stop now.

In effect this means the psABI should be clarified to explicitely say the 
the arg registers aren't clobbered, i.e. that the mentioned list of 
clobbered regs isn't inclusive but exclusive.  I will do that.

When I was discussing this with Boris earlier I hadn't yet looked at glibc 
use but only gave my interpretation from memory and reading.  Obviously 
reality trumps anything like that :-)

In short:  Ammars initial claim:

> Linux x86-64 syscall only clobbers rax, rcx and r11 (and "memory").
> 
>   - rax for the return value.
>   - rcx to save the return address.
>   - r11 to save the rflags.
> 
> Other registers are preserved.

is accurate and I will clarify the psABI to make that explicit.


Ciao,
Michael.

  reply	other threads:[~2021-10-13 16:24 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-11  4:03 [PATCH] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list Ammar Faizi
2021-10-12  5:28 ` Willy Tarreau
2021-10-12  8:36   ` Ammar Faizi
2021-10-12  9:06     ` Willy Tarreau
2021-10-12 20:29       ` Borislav Petkov
2021-10-12 21:51         ` Borislav Petkov
2021-10-12 22:23         ` Ammar Faizi
2021-10-13  3:01           ` Willy Tarreau
2021-10-13  3:32             ` Ammar Faizi
2021-10-13  3:34               ` Ammar Faizi
2021-10-13  3:37                 ` Ammar Faizi
2021-10-13 12:43           ` Borislav Petkov
2021-10-13 12:51             ` Willy Tarreau
2021-10-13 13:06               ` Borislav Petkov
2021-10-13 14:07                 ` Willy Tarreau
2021-10-13 14:20                   ` Borislav Petkov
2021-10-13 14:24                     ` Willy Tarreau
2021-10-13 16:24                       ` Michael Matz [this message]
2021-10-13 16:30                         ` Willy Tarreau
2021-10-13 16:51                           ` Andy Lutomirski
2021-10-13 16:52                           ` Borislav Petkov
2021-10-14  8:44                             ` Ammar Faizi
2021-10-14 12:44                             ` Michael Matz
2021-10-14 14:31                               ` Borislav Petkov
2021-10-19  9:06                         ` David Laight
2021-10-23 20:40             ` H. Peter Anvin
2021-10-12 21:21       ` David Laight
2021-10-12 23:02         ` Subject: " Ammar Faizi
2021-10-13  9:03 ` [PATCH v2] " Ammar Faizi
2021-10-15  8:25   ` [PATCH 0/2] Fix clobber list and startup code bug Ammar Faizi
2021-10-15  8:25     ` [PATCH 1/2] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list Ammar Faizi
2021-10-18  5:52       ` Willy Tarreau
2021-10-15  8:25     ` [PATCH 2/2] tools/nolibc: x86-64: Fix startup code bug Ammar Faizi
2021-10-15  8:57       ` Ammar Faizi
2021-10-15  9:26         ` Bedirhan KURT
2021-10-15  9:58           ` Ammar Faizi
2021-10-15  9:41         ` Louvian Lyndal
2021-10-18  4:58       ` Willy Tarreau
2021-10-18  6:53         ` Ammar Faizi
2021-10-23 13:27           ` Ammar Faizi
2021-10-23 13:43             ` Willy Tarreau
2021-10-24  2:11               ` [PATCHSET v2 0/2] tools/nolibc: Fix startup code bug and small improvement Ammar Faizi
2021-10-24  2:11                 ` [PATCH 1/2] tools/nolibc: x86-64: Fix startup code bug Ammar Faizi
2021-10-24  2:11                 ` [PATCH 2/2] tools/nolibc: x86-64: Use `mov $60,%eax` instead of `mov $60,%rax` Ammar Faizi
2021-10-24 11:41                 ` [PATCHSET v2 0/2] tools/nolibc: Fix startup code bug and small improvement Willy Tarreau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.20.2110131601000.26294@wotan.suse.de \
    --to=matz@suse.de \
    --cc=ammar.faizi@students.amikom.ac.id \
    --cc=aou@eecs.berkeley.edu \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=tglx@linutronix.de \
    --cc=w@1wt.eu \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).