| 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
| ||
|
Message-ID: <69389d62-a520-fb1a-2c73-fecc9bc8a71f@huawei.com>
Date: Thu, 31 Oct 2024 11:35:28 +0800
From: Jinjie Ruan <ruanjinjie@...wei.com>
To: Mark Rutland <mark.rutland@....com>
CC: <oleg@...hat.com>, <linux@...linux.org.uk>, <will@...nel.org>,
<catalin.marinas@....com>, <sstabellini@...nel.org>, <maz@...nel.org>,
<tglx@...utronix.de>, <peterz@...radead.org>, <luto@...nel.org>,
<kees@...nel.org>, <wad@...omium.org>, <akpm@...ux-foundation.org>,
<samitolvanen@...gle.com>, <arnd@...db.de>, <ojeda@...nel.org>,
<rppt@...nel.org>, <hca@...ux.ibm.com>, <aliceryhl@...gle.com>,
<samuel.holland@...ive.com>, <paulmck@...nel.org>, <aquini@...hat.com>,
<petr.pavlu@...e.com>, <viro@...iv.linux.org.uk>,
<rmk+kernel@...linux.org.uk>, <ardb@...nel.org>,
<wangkefeng.wang@...wei.com>, <surenb@...gle.com>,
<linus.walleij@...aro.org>, <yangyj.ee@...il.com>, <broonie@...nel.org>,
<mbenes@...e.cz>, <puranjay@...nel.org>, <pcc@...gle.com>,
<guohanjun@...wei.com>, <sudeep.holla@....com>,
<Jonathan.Cameron@...wei.com>, <prarit@...hat.com>, <liuwei09@...tc.cn>,
<dwmw@...zon.co.uk>, <oliver.upton@...ux.dev>, <kristina.martsenko@....com>,
<ptosi@...gle.com>, <frederic@...nel.org>, <vschneid@...hat.com>,
<thiago.bauermann@...aro.org>, <joey.gouly@....com>,
<liuyuntao12@...wei.com>, <leobras@...hat.com>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH -next v4 02/19] arm64: entry: Refactor the entry and exit
for exceptions from EL1
On 2024/10/29 22:33, Mark Rutland wrote:
> On Fri, Oct 25, 2024 at 06:06:43PM +0800, Jinjie Ruan wrote:
>> These changes refactor the entry and exit routines for the exceptions
>> from EL1. They store the RCU and lockdep state in a struct
>> irqentry_state variable on the stack, rather than recording them
>> in the fields of pt_regs, since it is safe enough for these context.
>
> In general, please descirbe *why* we want to make the change first, e.g.
>
> | The generic entry code uses irqentry_state_t to track lockdep and RCU
> | state across exception entry and return. For historical reasons, arm64
> | embeds similar fields within its pt_regs structure.
> |
> | In preparation for moving arm64 over to the generic entry code, pull
> | these fields out of arm64's pt_regs, and use a seperate structure,
> | matching the style of the generic entry code.
>
>> Before:
>> struct pt_regs {
>> ...
>> u64 lockdep_hardirqs;
>> u64 exit_rcu;
>> }
>>
>> enter_from_kernel_mode(regs);
>> ...
>> exit_to_kernel_mode(regs);
>>
>> After:
>> typedef struct irqentry_state {
>> union {
>> bool exit_rcu;
>> bool lockdep;
>> };
>> } irqentry_state_t;
>>
>> irqentry_state_t state = enter_from_kernel_mode(regs);
>> ...
>> exit_to_kernel_mode(regs, state);
>
> I don't think this part is necessary.
Thank you, will remove it and explain why.
>
>>
>> No functional changes.
>>
>> Suggested-by: Mark Rutland <mark.rutland@....com>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>
>> ---
>> arch/arm64/include/asm/ptrace.h | 11 ++-
>> arch/arm64/kernel/entry-common.c | 129 +++++++++++++++++++------------
>> 2 files changed, 85 insertions(+), 55 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
>> index 3e5372a98da4..5156c0d5fa20 100644
>> --- a/arch/arm64/include/asm/ptrace.h
>> +++ b/arch/arm64/include/asm/ptrace.h
>> @@ -149,6 +149,13 @@ static inline unsigned long pstate_to_compat_psr(const unsigned long pstate)
>> return psr;
>> }
>>
>> +typedef struct irqentry_state {
>> + union {
>> + bool exit_rcu;
>> + bool lockdep;
>> + };
>> +} irqentry_state_t;
>
> AFAICT this can be moved directly into arch/arm64/kernel/entry-common.c.
>
>> +
>> /*
>> * This struct defines the way the registers are stored on the stack during an
>> * exception. struct user_pt_regs must form a prefix of struct pt_regs.
>> @@ -169,10 +176,6 @@ struct pt_regs {
>>
>> u64 sdei_ttbr1;
>> struct frame_record_meta stackframe;
>> -
>> - /* Only valid for some EL1 exceptions. */
>> - u64 lockdep_hardirqs;
>> - u64 exit_rcu;
>> };
>>
>> /* For correct stack alignment, pt_regs has to be a multiple of 16 bytes. */
>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>> index c547e70428d3..68a9aecacdb9 100644
>> --- a/arch/arm64/kernel/entry-common.c
>> +++ b/arch/arm64/kernel/entry-common.c
>> @@ -36,29 +36,36 @@
>> * This is intended to match the logic in irqentry_enter(), handling the kernel
>> * mode transitions only.
>> */
>> -static __always_inline void __enter_from_kernel_mode(struct pt_regs *regs)
>> +static __always_inline irqentry_state_t __enter_from_kernel_mode(struct pt_regs *regs)
>> {
>> - regs->exit_rcu = false;
>> + irqentry_state_t ret = {
>> + .exit_rcu = false,
>> + };
>
> I realise that the generic entry code calls this 'ret' in
> irqentry_enter() and similar, but could we please use 'state'
> consistently in the arm64 code?
>
> [...]
>
>> /*
>> @@ -190,9 +199,11 @@ asmlinkage void noinstr asm_exit_to_user_mode(struct pt_regs *regs)
>> * mode. Before this function is called it is not safe to call regular kernel
>> * code, instrumentable code, or any code which may trigger an exception.
>> */
>> -static void noinstr arm64_enter_nmi(struct pt_regs *regs)
>> +static noinstr irqentry_state_t arm64_enter_nmi(struct pt_regs *regs)
>> {
>> - regs->lockdep_hardirqs = lockdep_hardirqs_enabled();
>> + irqentry_state_t irq_state;
>
> Likewise, please use 'state' rather than 'irq_state'.
>
> In future we should probably have a separate structure for the NMI
> paths, and get rid of the union, which would avoid the possiblity of
> using mismatched helpers.
>
> Mark.
>
>
Powered by blists - more mailing lists