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

Message-ID: <xgj3in4cuy3k2phafttxb4cwz6rn6vuj5tvwjioqbrl4hxivkt@wkr5pi6zfrv7>
Date: Mon, 7 Apr 2025 16:47:05 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: Linus Walleij <linus.walleij@...aro.org>, 
	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, peterz@...radead.org, 
	Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [GIT PULL] Generic entry for ARM

I think Thomas, Peter and Steven are the experts here, but I'll give my
limited understanding.

On Mon, Apr 07, 2025 at 10:00:54AM -0700, Paul E. McKenney wrote:
> > 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?

noinstr is much broader than notrace.  No instrumentation of any kind
(ftrace, kprobes, sanitizers, profilers, etc), in the prologue or body
of the function or its callees, except for regions marked by
instrumentation_{begin,end}().

noinstr is needed in early/late entry code before/after RCU transitions,
as instrumentation code might depend on RCU.  Also, entry code tends to
be sensitive and unable to handle random instrumentation code.

notrace is more narrow, it just means "no function tracing".  It's used
by tracing code and the functions it calls.  It's also used for early
boot code, as ftrace can be enabled on the kernel cmdline.

I believe noinstr is not typically needed for boot code, at least for
the primary CPU.  RCU isn't present yet, and many instrumentations might
not yet be available.  Or their handlers can detect whether they've been
initialized yet.  Or they're disabled in .init sections.

Whether noinstr is needed for *secondary* boot code, I don't know.  It
may be dependent on the instrumentation implementations, e.g., whether
they're enabled globally or per-CPU.  At least on x86, start_secondary()
is notrace.  I don't know enough to say whether that's sufficient.

> > 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!

Sounds like a good idea.  We just need a volunteer :-)

-- 
Josh