linux-kernel - Re: [PATCH] x86/ftrace: Don't bother preserving/restoring R10/R11

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: <20241009120257.7b2de987@gandalf.local.home>
Date: Wed, 9 Oct 2024 12:02:57 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Ard Biesheuvel <ardb+git@...gle.com>
Cc: linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org, Ard
 Biesheuvel <ardb@...nel.org>, Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH] x86/ftrace: Don't bother preserving/restoring R10/R11

On Wed,  9 Oct 2024 17:39:01 +0200
Ard Biesheuvel <ardb+git@...gle.com> wrote:

> diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
> index 214f30e9f0c0..9e13f9b203d1 100644
> --- a/arch/x86/kernel/ftrace_64.S
> +++ b/arch/x86/kernel/ftrace_64.S
> @@ -216,8 +216,6 @@ SYM_INNER_LABEL(ftrace_regs_caller_op_ptr, SYM_L_GLOBAL)
>  	movq %r14, R14(%rsp)
>  	movq %r13, R13(%rsp)
>  	movq %r12, R12(%rsp)
> -	movq %r11, R11(%rsp)
> -	movq %r10, R10(%rsp)
>  	movq %rbx, RBX(%rsp)

This may need to still be saved. Even though they are pretty much useless,
it can be used by kprobes, and perhaps they want to see what's in R10 or
R11 from the caller. I'm not sure we want to remove this. Especially since
we are even saving "flags"!

That is, this part is visible by the callbacks, and the ftrace_get_regs()
is to return a fully populated pt_regs if FTRACE_OPS_FL_SAVE_REGS is set.

>  	/* Copy saved flags */
>  	movq MCOUNT_REG_SIZE(%rsp), %rcx
> @@ -256,7 +254,6 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL)
>  	movq R14(%rsp), %r14
>  	movq R13(%rsp), %r13
>  	movq R12(%rsp), %r12
> -	movq R10(%rsp), %r10

This part of the patch I think is perfectly fine. We haven't been restoring
R11 for 12 years I'm sure nobody will notice if we don't restore R10.

-- Steve

>  	movq RBX(%rsp), %rbx
>  
>  	movq ORIG_RAX(%rsp), %rax
> --