linux-kernel - Re: [GIT PULL] Generic entry for ARM

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] [day] [month] [year] [list]

Message-ID: <CACRpkdaYQx8gBnkjW0zy=-FNS-P+TtjXoNBsBR2D4FTWo28R1Q@mail.gmail.com>
Date: Mon, 31 Mar 2025 23:48:40 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: paulmck@...nel.org
Cc: Russell King <rmk+kernel@...linux.org.uk>, 
	Linux ARM <linux-arm-kernel@...ts.infradead.org>, Arnd Bergmann <arnd@...db.de>, 
	Thomas Gleixner <tglx@...utronix.de>, linux-kernel <linux-kernel@...r.kernel.org>, frederic@...nel.org
Subject: Re: [GIT PULL] Generic entry for ARM

On Wed, Mar 12, 2025 at 7:00 PM Paul E. McKenney <paulmck@...nel.org> wrote:

> Once you are confident that you have all the needed "noinstr"
> and "__always_inline" instances in place, could you please add
> ARCH_WANTS_NO_INSTR to the list of "select" clauses for "config ARM"
> in arch/arm/Kconfig?

I would love to do that, I'm just not sure what this really entails.

Surely this patchset tags a noinstr on every entry point from
exceptions and syscall software interrupts.
Documentation/core-api/entry.rst is pretty good at explaining this.

But what makes me uncertain are things that are tagged
"notrace", such as void notrace cpu_init(void) - surely we
don't trace, but should that be "noinstr"? It's even marked
"notrace" but not "noinstr" in arm64.

cpu_init() is called from e.g.:
asmlinkage void secondary_start_kernel(struct task_struct *task)
OK should this also be noinstr? Or is that just implied because
of asmlinkage?

<linux/compiler_types.h> will resolve to:

#if defined(CC_USING_HOTPATCH)
#define notrace                 __attribute__((hotpatch(0, 0)))
#elif defined(CC_USING_PATCHABLE_FUNCTION_ENTRY)
#define notrace                 __attribute__((patchable_function_entry(0, 0)))
#else
#define notrace                 __attribute__((__no_instrument_function__))
#endif

which I read as three different ways of saying "don't patch here".

Which is confusingly similar or identical to what noinstr does, I do see that
noinstr pushes the code to separate section but that in turn might
be what __attribute__((__no_instrument_function__)) and
friends does?

Are they equivalent?

sched_clock_noinstr() is tagged noinstr and sched_clock() is
tagged notrace, so there must be a difference here.

secondary_start_kernel() is tagged "notrace" on arm64 but
not on arm, should it be tagged "noinstr" or "notrace"?

This kind of stuff makes me uncertain about how this is to be
done. A "noinstr vs notrace" section in Documentation/core-api/entry.rst
would help a lot I think!

Yours,
Linus Walleij