linux-kernel - Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

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: <Z6oo0MExaEy5J55_@shell.armlinux.org.uk>
Date: Mon, 10 Feb 2025 16:26:56 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Ben Segall <bsegall@...gle.com>,
	Catalin Marinas <catalin.marinas@....com>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>,
	Mel Gorman <mgorman@...e.de>, Peter Zijlstra <peterz@...radead.org>,
	Shrikanth Hegde <sshegde@...ux.ibm.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Will Deacon <will@...nel.org>, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

On Mon, Feb 10, 2025 at 04:39:02PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-02-10 15:16:11 [+0000], Russell King (Oracle) wrote:
> > On Mon, Feb 10, 2025 at 01:04:29PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2025-02-03 15:16:26 [+0100], To linux-kernel@...r.kernel.org wrote:
> > > > __die() invokes later __show_regs() -> show_regs_print_info() which
> > > > prints the current preemption model.
> > > > Remove it from the initial line.
> > > > 
> > > > Cc: Russell King <linux@...linux.org.uk>
> > > > Cc: linux-arm-kernel@...ts.infradead.org
> > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> > > 
> > > Is it okay, to route this via the sched tree?
> > 
> > Sorry, its not obvious where show_regs_print_info() prints this.
> > dump_stack_print_info() itself doesn't directly. print_worker_info()
> > doesn't. print_stop_info() doesn't. Not sure whether print_scx_info()
> > does.
> > 
> > Maybe showing example output would help?
> 
> Patch 2/9 adds this. [ https://lore.kernel.org/all/20250203141632.440554-3-bigeasy@linutronix.de/ ]

That explains it - patch 2 is only sent to a very restricted subset
and not even to mailing lists that would be relevant in the absence
of a direct Cc. Ditto patch 1. All I received were patches 3 and 4.

In patch 1:

+	static char buf[128];
...
+		seq_buf_init(&s, buf, 128);

Why not sizeof(buf) ?

For patch 2:

"Use
pr_warn() instead of printk() to pass a loglevel. This makes it part of
generic WARN/ BUG traces.
"

How about cases which use dump_stack_lvl() to dump the output at a more
severe level than warning? Should the message be printed at a less
severe level than the other messages?

> This this applied, die("test") on ARM ends as:
> 
> [    1.595106] Kernel panic - not syncing: test
> [    1.596044] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W 6.14.0-rc2-00009-gb80a798df08c-dirty #13 PREEMPT
> [    1.596768] Tainted: [W]=WARN
> [    1.596946] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022

Hmm. I've no idea what you're testing, what you've quoted makes zero
sense to me.

First...

void die(const char *str, struct pt_regs *regs, int err)

is the die function prototype, so it takes a bit more than what you've
indicated.

Second, "Kernel panic" suggests that panic() has been called. However,
this only happens when die() is called (or more specifically
oops_end()) from either interrupt context (in which case we get
"Kernel panic - Fatal exception in interrupt") or if panic_on_oops
is set ("Kernel panic - Fatal exception").

I don't see a path which would result in
"Kernel panic - not syncing: test" to be printed from this path.

Since __die() does not call dump_stack(), we're not going to call
dump_stack_print_info() from __die(), so I don't think it's appropriate
to remove this information.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!