linux-kernel - Re: [PATCH v3 0/3] arm64: entry: Convert to generic entry

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: <ZxYiyiKJm-6A6poG@J2N7QTR9R3>
Date: Mon, 21 Oct 2024 10:45:56 +0100
From: Mark Rutland <mark.rutland@....com>
To: Jinjie Ruan <ruanjinjie@...wei.com>
Cc: catalin.marinas@....com, will@...nel.org, oleg@...hat.com,
	tglx@...utronix.de, peterz@...radead.org, luto@...nel.org,
	kees@...nel.org, wad@...omium.org, rostedt@...dmis.org,
	arnd@...db.de, ardb@...nel.org, broonie@...nel.org,
	rick.p.edgecombe@...el.com, leobras@...hat.com,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 0/3] arm64: entry: Convert to generic entry

On Mon, Oct 21, 2024 at 04:30:51PM +0800, Jinjie Ruan wrote:
> On 2024/10/17 23:25, Mark Rutland wrote:
> > On Sat, Jun 29, 2024 at 04:55:58PM +0800, Jinjie Ruan wrote:

> > Patch 3 in particular is very hard to follow because several unrelated
> > complex systems are updated simultaneously. It would be really nice if
> > we could move to the generic sycall code separately from moving the rest
> > of the entry code, as the sycall handling code is a particularly
> > important ABI concern, and it's difficult to see whether we're making
> > ABI changes (accidentaly or knowingly).
> > 
> > Can we split that up (e.g. splitting the generic code first into
> > separate entry and syscall files), or are those too tightly coupled for
> > that to be possible?
> 
> It will be hard, but I will try to split it, they are surely tightly
> coupled which make the 3th patch too big when I try to switch to generic
> entry.

I'm confused. The point I'm making is don't try to switch to *all* the
generic entry code in one go: split the 3rd patch into smaller,
logically-distinct separate steps. The 3rd patch shouldn't get larger as
you should be changing fewer lines in any individual patch.

The regular entry state management (e.g. enter_from_user_mode() and
exit_to_user_mode()) is largely separate from the syscall state
management, which is pretty clear given
syscall_enter_from_user_mode_prepare() and syscall_exit_to_user_mode()
wrap the regular entry logic:

| noinstr void syscall_enter_from_user_mode_prepare(struct pt_regs *regs)
| {
|         enter_from_user_mode(regs);
|         instrumentation_begin();
|         local_irq_enable();
|         instrumentation_end();
| }

| __visible noinstr void syscall_exit_to_user_mode(struct pt_regs *regs)
| {
|         instrumentation_begin();
|         __syscall_exit_to_user_mode_work(regs);
|         instrumentation_end();
|         exit_to_user_mode();
| }

... and while exit_to_user_mode_prepare() is called by
irqentry_exit_to_user_mode(), that's also just a wrapper around
exit_to_user_mode():

| noinstr void irqentry_exit_to_user_mode(struct pt_regs *regs)
| {
|         instrumentation_begin();
|         exit_to_user_mode_prepare(regs);
|         instrumentation_end();
|         exit_to_user_mode();
| }

... so AFAICT we could move arm64 over to enter_from_user_mode() and
exit_to_user_mode() without needing to use any of the generic syscall
logic.

Doing that first, *then* moving over to the generic syscall handling
would be much easier to review/test/bisect, and if there are any ABI
issues with the syscall handling in particular (which I think is
likely), it will be easier to handle those in isolation.

Thanks,
Mark.