linux-kernel - Re: [PATCH] x86: add 'runtime constant' infrastructure

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: <CAHk-=wh5DAR=a12cbKbxDy875hTOtPmNUDEn+dU2VS47h9MgcQ@mail.gmail.com>
Date: Mon, 10 Jun 2024 11:06:44 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Peter Anvin <hpa@...or.com>, Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Thomas Gleixner <tglx@...utronix.de>, Rasmus Villemoes <linux@...musvillemoes.dk>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, "the arch/x86 maintainers" <x86@...nel.org>, 
	linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [PATCH] x86: add 'runtime constant' infrastructure

On Mon, 10 Jun 2024 at 03:43, Peter Zijlstra <peterz@...radead.org> wrote:
>
> --- linux-2.6.orig/arch/Kconfig
> +++ linux-2.6/arch/Kconfig
> @@ -1492,6 +1492,9 @@ config HAVE_SPARSE_SYSCALL_NR
>  config ARCH_HAS_VDSO_DATA
>         bool
>
> +config HAVE_RUNTIME_CONST
> +       bool

No. We're not adding a stupid config variable, when nothing actually wants it.

> +#define __runtime_const(sym, op, type)                 \
> +({                                                     \
> +       typeof(sym) __ret;                              \
> +       asm(op " %1, %0\n1:\n"                          \
> +           ".pushsection __runtime_const, \"aw\"\n\t"  \
> +           ".long %c3 - .      # sym \n\t"             \
> +           ".long %c2          # size \n\t"            \
> +           ".long 1b - %c2 - . # addr \n\t"            \
> +           ".popsection\n\t"                           \
> +           : "=r" (__ret)                              \
> +           : "i" ((type)0x0123456789abcdefull),        \
> +             "i" (sizeof(type)),                       \
> +             "i" ((void *)&sym));                      \
> +       __ret;                                          \
> +})
> +
> +#define runtime_const(sym)                                             \
> +({                                                                     \
> +       typeof(sym) __ret;                                              \
> +       switch(sizeof(sym)) {                                           \
> +       case 1: __ret = __runtime_const(sym, "movb", u8); break;        \
> +       case 2: __ret = __runtime_const(sym, "movs", u16); break;       \
> +       case 4: __ret = __runtime_const(sym, "movl", u32); break;       \
> +       case 8: __ret = __runtime_const(sym, "movq", u64); break;       \
> +       default: BUG();                                                 \
> +       }                                                               \
> +       __ret;                                                          \
> +})

And no. We're not adding magic "generic" helpers that have zero use
and just make the code harder to read, and don't even work on 32-bit
x86 anyway.

Because I didn't test, but I am pretty sure that clang will not
compile the above on x86-32, because clang verifies the inline asm,
and "movq" isn't a valid instruction.

We had exactly that for the uaccess macros, and needed to special-case
the 64-bit case for that reason.

And we don't *need* to. All of the above is garbage waiting for a use
that shouldn't exist.

> +++ linux-2.6/kernel/runtime_const.c
> @@ -0,0 +1,119 @@

And here you basically tripled the size of the patch in order to have
just one section, when I had per-symbol sections.

So no.

I envision a *couple* of runtime constants. The absolutely only reason
to use a runtime constant is that it is *so* hot that the difference
between "load a variable" and "write code with a constant" is
noticeable.

I can point to exactly one such case in the kernel right now.

I'm sure there are others, but I'd expect that "others" to be mainly a handful.

This needs to be simple, obvious, and literally designed for very targeted use.

This is a *very* special case for a *very* special code. Not for generic use.

I do not ever expect to see this used by modules, for example. There
is no way in hell I will expose the instruction rewriting to a module.
The *use* of a constant is a _maybe_, but it's questionable too. By
definition, module code cannot be all that core.

             Linus