linux-kernel - Re: [PATCH] x86/xen: fix xen_hypercall

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: <b70e246a-5e5e-431c-9b85-dc4644df7bd9@citrix.com>
Date: Wed, 5 Feb 2025 09:38:19 +0000
From: Andrew Cooper <andrew.cooper3@...rix.com>
To: Jürgen Groß <jgross@...e.com>,
 linux-kernel@...r.kernel.org, x86@...nel.org
Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, xen-devel@...ts.xenproject.org
Subject: Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx

On 05/02/2025 9:17 am, Jürgen Groß wrote:
> On 05.02.25 10:16, Andrew Cooper wrote:
>> On 05/02/2025 9:10 am, Juergen Gross wrote:
>>> xen_hypercall_hvm(), which is used when running as a Xen PVH guest at
>>> most only once during early boot, is clobbering %rbx. Depending on
>>> whether the caller relies on %rbx to be preserved across the call or
>>> not, this clobbering might result in an early crash of the system.
>>>
>>> This can be avoided by not modifying %rbx in xen_hypercall_hvm().
>>>
>>> Fixes: b4845bb63838 ("x86/xen: add central hypercall functions")
>>> Signed-off-by: Juergen Gross <jgross@...e.com>
>>> ---
>>>   arch/x86/xen/xen-head.S | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
>>> index 9252652afe59..4378b817ed32 100644
>>> --- a/arch/x86/xen/xen-head.S
>>> +++ b/arch/x86/xen/xen-head.S
>>> @@ -117,8 +117,7 @@ SYM_FUNC_START(xen_hypercall_hvm)
>>
>> The 32bit case, out of context up here, also clobbers %ebx.
>>
>> ~Andrew
>>
>>>       pop %ebx
>
> It does not, as this part of the context is showing.

Hmm, so it is, and worse, it can't be changed to match the 64bit side. 
That's nasty.

But while I'm here looking at the code, what's up with

#ifdef CONFIG_FRAME_POINTER
        pushq $0        /* Dummy push for stack alignment. */
#endif

?

That's covered by FRAME_{START,END} normally, and Linux's preferred
stack alignment is 8 not 16.

~Andrew