Re: [PATCH v4 12/20] Revert "powerpc/syscall: Save r3 in regs->orig_r3"

From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Rohan McLure" <rmclure@linux.ibm.com>, <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v4 12/20] Revert "powerpc/syscall: Save r3 in regs->orig_r3"
Date: Mon, 12 Sep 2022 21:14:28 +1000	[thread overview]
Message-ID: <CMUDNS3IJ8L1.2PM50DMHILXT2@bobo> (raw)
In-Reply-To: <20220824020548.62625-13-rmclure@linux.ibm.com>

On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote:
> This reverts commit 8875f47b7681aa4e4484a9b612577b044725f839.

Can you use short hash and commit title format? Also it's no longer
just reverting that patch, so maybe just come up with a new title
for this patch and reference the two patches here?

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Oh, I meant to say for the last patch and this one. Can you move
them to after patch 13? That way all your build and wrapper mucking
are in the first patches, and then all the zeroizing comes next.

Thanks,
Nick

>
> Save caller's original r3 state to the kernel stackframe before entering
> system_call_exception. This allows for user registers to be cleared by
> the time system_call_exception is entered, reducing the influence of
> user registers on speculation within the kernel.
>
> Prior to this commit, orig_r3 was saved at the beginning of
> system_call_exception. Instead, save orig_r3 while the user value is
> still live in r3.
>
> Also replicate this early save in 32-bit. A similar save was removed in
> commit 6f76a01173cc ("powerpc/syscall: implement system call entry/exit logic in C for PPC32")
> when 32-bit adopted system_call_exception. Revert its removal of orig_r3
> saves.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> V2 -> V3: New commit.
> ---
>  arch/powerpc/kernel/entry_32.S     | 1 +
>  arch/powerpc/kernel/interrupt_64.S | 2 ++
>  arch/powerpc/kernel/syscall.c      | 1 -
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 1d599df6f169..44dfce9a60c5 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -101,6 +101,7 @@ __kuep_unlock:
>  
>  	.globl	transfer_to_syscall
>  transfer_to_syscall:
> +	stw	r3, ORIG_GPR3(r1)
>  	stw	r11, GPR1(r1)
>  	stw	r11, 0(r1)
>  	mflr	r12
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index ce25b28cf418..71d2d9497283 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -91,6 +91,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
>  	li	r11,\trapnr
>  	std	r11,_TRAP(r1)
>  	std	r12,_CCR(r1)
> +	std	r3,ORIG_GPR3(r1)
>  	addi	r10,r1,STACK_FRAME_OVERHEAD
>  	ld	r11,exception_marker@toc(r2)
>  	std	r11,-16(r10)		/* "regshere" marker */
> @@ -275,6 +276,7 @@ END_BTB_FLUSH_SECTION
>  	std	r10,_LINK(r1)
>  	std	r11,_TRAP(r1)
>  	std	r12,_CCR(r1)
> +	std	r3,ORIG_GPR3(r1)
>  	addi	r10,r1,STACK_FRAME_OVERHEAD
>  	ld	r11,exception_marker@toc(r2)
>  	std	r11,-16(r10)		/* "regshere" marker */
> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> index 81ace9e8b72b..64102a64fd84 100644
> --- a/arch/powerpc/kernel/syscall.c
> +++ b/arch/powerpc/kernel/syscall.c
> @@ -25,7 +25,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
>  	kuap_lock();
>  
>  	add_random_kstack_offset();
> -	regs->orig_gpr3 = r3;
>  
>  	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
>  		BUG_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED);
> -- 
> 2.34.1