linux-kernel - Re: [RFC PATCH v1 13/15] x86/msr: Use the alternatives mechanism to read MSR

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: <0f4f2ed70829fffb2eb816e34e26be22681705a5.camel@gmail.com>
Date: Mon, 14 Apr 2025 19:13:09 +0200
From: Francesco Lavra <francescolavra.fl@...il.com>
To: xin@...or.com
Cc: acme@...nel.org, adrian.hunter@...el.com, ajay.kaher@...adcom.com, 
 alexander.shishkin@...ux.intel.com, alexey.amakhalov@...adcom.com, 
 andrew.cooper3@...rix.com, bcm-kernel-feedback-list@...adcom.com, 
 boris.ostrovsky@...cle.com, bp@...en8.de, bpf@...r.kernel.org, 
 dave.hansen@...ux.intel.com, decui@...rosoft.com, haiyangz@...rosoft.com, 
 hpa@...or.com, irogers@...gle.com, jgross@...e.com, jolsa@...nel.org, 
 kan.liang@...ux.intel.com, kvm@...r.kernel.org, kys@...rosoft.com, 
 linux-edac@...r.kernel.org, linux-hyperv@...r.kernel.org, 
 linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-perf-users@...r.kernel.org, linux-pm@...r.kernel.org,
 llvm@...ts.linux.dev,  luto@...nel.org, mark.rutland@....com,
 mingo@...hat.com, namhyung@...nel.org,  pbonzini@...hat.com,
 peterz@...radead.org, seanjc@...gle.com, tglx@...utronix.de, 
 tony.luck@...el.com, virtualization@...ts.linux.dev, vkuznets@...hat.com, 
 wei.liu@...nel.org, x86@...nel.org, xen-devel@...ts.xenproject.org
Subject: Re: [RFC PATCH v1 13/15] x86/msr: Use the alternatives mechanism to
 read MSR

On 2025-03-31 at 8:22, Xin Li (Intel) wrote:
> diff --git a/arch/x86/xen/xen-asm.S b/arch/x86/xen/xen-asm.S
> index e672632b1cc0..6e7a9daa03d4 100644
> --- a/arch/x86/xen/xen-asm.S
> +++ b/arch/x86/xen/xen-asm.S
> @@ -399,3 +399,37 @@ SYM_CODE_END(xen_entry_SYSCALL_compat)
>  	RET
>  SYM_FUNC_END(asm_xen_write_msr)
>  EXPORT_SYMBOL_GPL(asm_xen_write_msr)
> +
> +/*
> + * The prototype of the Xen C code:
> + * 	struct { u64 val, bool done } xen_do_read_msr(u32 msr)
> + */
> +SYM_FUNC_START(asm_xen_read_msr)
> +	ENDBR
> +	FRAME_BEGIN
> +	XEN_SAVE_CALLEE_REGS_FOR_MSR
> +	mov %ecx, %edi		/* MSR number */
> +	call xen_do_read_msr
> +	test %dl, %dl		/* %dl=1, i.e., ZF=0, meaning
> successfully done */
> +	XEN_RESTORE_CALLEE_REGS_FOR_MSR
> +	jnz 2f
> +
> +1:	rdmsr
> +	_ASM_EXTABLE_FUNC_REWIND(1b, -5, FRAME_OFFSET /
> (BITS_PER_LONG / 8))
> +	shl $0x20, %rdx
> +	or %rdx, %rax
> +	/*
> +	 * The top of the stack points directly at the return
> address;
> +	 * back up by 5 bytes from the return address.
> +	 */

This works only if this function has been called directly (e.g. via
`call asm_xen_write_msr`), but doesn't work with alternative call types
(like indirect calls). Not sure why one might want to use an indirect
call to invoke asm_xen_write_msr, but this creates a hidden coupling
between caller and callee.
I don't have a suggestion on how to get rid of this coupling, other
than setting ipdelta in _ASM_EXTABLE_FUNC_REWIND() to 0 and adjusting
the _ASM_EXTABLE_TYPE entries at the call sites to consider the
instruction that follows the function call (instead of the call
instruction) as the faulting instruction (which seems pretty ugly, at
least because what follows the function call could be an instruction
that might itself fault). But you may want to make this caveat explicit
in the comment.