* [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx
@ 2025-02-05 9:10 Juergen Gross
2025-02-05 9:16 ` Andrew Cooper
2025-02-05 9:54 ` Jan Beulich
0 siblings, 2 replies; 8+ messages in thread
From: Juergen Gross @ 2025-02-05 9:10 UTC (permalink / raw)
To: linux-kernel, x86
Cc: Juergen Gross, Boris Ostrovsky, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel
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@suse.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)
pop %ebx
pop %eax
#else
- lea xen_hypercall_amd(%rip), %rbx
- cmp %rax, %rbx
+ cmp xen_hypercall_amd(%rip), %rax
#ifdef CONFIG_FRAME_POINTER
pop %rax /* Dummy pop. */
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 9:10 [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx Juergen Gross @ 2025-02-05 9:16 ` Andrew Cooper 2025-02-05 9:17 ` Jürgen Groß 2025-02-05 9:54 ` Jan Beulich 1 sibling, 1 reply; 8+ messages in thread From: Andrew Cooper @ 2025-02-05 9:16 UTC (permalink / raw) To: Juergen Gross, linux-kernel, x86 Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel 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@suse.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 > pop %eax > #else > - lea xen_hypercall_amd(%rip), %rbx > - cmp %rax, %rbx > + cmp xen_hypercall_amd(%rip), %rax > #ifdef CONFIG_FRAME_POINTER > pop %rax /* Dummy pop. */ > #endif ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 9:16 ` Andrew Cooper @ 2025-02-05 9:17 ` Jürgen Groß 2025-02-05 9:38 ` Andrew Cooper 0 siblings, 1 reply; 8+ messages in thread From: Jürgen Groß @ 2025-02-05 9:17 UTC (permalink / raw) To: Andrew Cooper, linux-kernel, x86 Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel [-- Attachment #1.1.1: Type: text/plain, Size: 1086 bytes --] 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@suse.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. Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3743 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 9:17 ` Jürgen Groß @ 2025-02-05 9:38 ` Andrew Cooper 2025-02-05 10:04 ` Jürgen Groß 0 siblings, 1 reply; 8+ messages in thread From: Andrew Cooper @ 2025-02-05 9:38 UTC (permalink / raw) To: Jürgen Groß, linux-kernel, x86 Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel 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@suse.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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 9:38 ` Andrew Cooper @ 2025-02-05 10:04 ` Jürgen Groß 2025-02-05 10:30 ` Jan Beulich 0 siblings, 1 reply; 8+ messages in thread From: Jürgen Groß @ 2025-02-05 10:04 UTC (permalink / raw) To: Andrew Cooper, linux-kernel, x86 Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel [-- Attachment #1.1.1: Type: text/plain, Size: 1850 bytes --] On 05.02.25 10:38, Andrew Cooper wrote: > 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@suse.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. I've added this due to a review comment by Jan. As he is more into ABI matters, I believed him. Google is telling me you are right, so I'll remove those extra hunks in V2 of the patch adding FRAME_END. Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3743 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 10:04 ` Jürgen Groß @ 2025-02-05 10:30 ` Jan Beulich 0 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2025-02-05 10:30 UTC (permalink / raw) To: Jürgen Groß Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel, Andrew Cooper, linux-kernel, x86 On 05.02.2025 11:04, Jürgen Groß wrote: > On 05.02.25 10:38, Andrew Cooper wrote: >> 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@suse.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. > > I've added this due to a review comment by Jan. As he is more into ABI > matters, I believed him. > > Google is telling me you are right, so I'll remove those extra hunks in > V2 of the patch adding FRAME_END. Oh, I'm sorry for misleading you. Clearly I must have been mis-remembering (or things may have changed, and I should have re-checked). Jan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 9:10 [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx Juergen Gross 2025-02-05 9:16 ` Andrew Cooper @ 2025-02-05 9:54 ` Jan Beulich 2025-02-05 10:07 ` Jürgen Groß 1 sibling, 1 reply; 8+ messages in thread From: Jan Beulich @ 2025-02-05 9:54 UTC (permalink / raw) To: Juergen Gross Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel, linux-kernel, x86 On 05.02.2025 10:10, 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@suse.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) > pop %ebx > pop %eax > #else > - lea xen_hypercall_amd(%rip), %rbx > - cmp %rax, %rbx There's no memory access here, but ... > + cmp xen_hypercall_amd(%rip), %rax ... you now read from memory here. That can't be right. Afaict the original use of LEA needs to stay, just with a different scratch register. Jan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx 2025-02-05 9:54 ` Jan Beulich @ 2025-02-05 10:07 ` Jürgen Groß 0 siblings, 0 replies; 8+ messages in thread From: Jürgen Groß @ 2025-02-05 10:07 UTC (permalink / raw) To: Jan Beulich Cc: Boris Ostrovsky, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel, linux-kernel, x86 [-- Attachment #1.1.1: Type: text/plain, Size: 1306 bytes --] On 05.02.25 10:54, Jan Beulich wrote: > On 05.02.2025 10:10, 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@suse.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) >> pop %ebx >> pop %eax >> #else >> - lea xen_hypercall_amd(%rip), %rbx >> - cmp %rax, %rbx > > There's no memory access here, but ... > >> + cmp xen_hypercall_amd(%rip), %rax > > ... you now read from memory here. That can't be right. Afaict the original > use of LEA needs to stay, just with a different scratch register. Oh, right. Thanks for noticing. Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3743 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-02-05 10:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-05 9:10 [PATCH] x86/xen: fix xen_hypercall_hvm() to not clobber %rbx Juergen Gross 2025-02-05 9:16 ` Andrew Cooper 2025-02-05 9:17 ` Jürgen Groß 2025-02-05 9:38 ` Andrew Cooper 2025-02-05 10:04 ` Jürgen Groß 2025-02-05 10:30 ` Jan Beulich 2025-02-05 9:54 ` Jan Beulich 2025-02-05 10:07 ` Jürgen Groß
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.