| Message ID | 36cf1c15-faa5-4e25-8fdd-9c52076f4ca2@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/entry: don't clear DF when raising #UD for lack of syscall handler | expand |
On 06/03/2024 1:44 pm, Jan Beulich wrote: > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -47,12 +55,13 @@ UNLIKELY_START(z, syscall_no_callback) / > testb $4, X86_EXC_UD * TRAPINFO_sizeof + TRAPINFO_flags(%rdi) > setnz %cl > lea TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx > + or $~0, %esi # don't clear DF Our predominant comment style is /* */ > UNLIKELY_END(syscall_no_callback) > > movq %rax,TRAPBOUNCE_eip(%rdx) > movb %cl,TRAPBOUNCE_flags(%rdx) > call create_bounce_frame > - andl $~X86_EFLAGS_DF,UREGS_eflags(%rsp) > + and %esi, UREGS_eflags(%rsp) Could we gain a /* Conditionally clear DF */ comment here? Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> CC Oleksii for 4.19. This fixes a subtle regression in the PV ABI caused by a post-XSA fix a few years ago. It's a low-risk fix to take; while I still don't have an XTF test covering this, the corner case it's changing used to be completely fatal to guests, so it's a corner unused in practice. ~Andrew
On Mon, 2024-07-01 at 16:35 +0100, Andrew Cooper wrote: > On 06/03/2024 1:44 pm, Jan Beulich wrote: > > --- a/xen/arch/x86/x86_64/entry.S > > +++ b/xen/arch/x86/x86_64/entry.S > > @@ -47,12 +55,13 @@ UNLIKELY_START(z, syscall_no_callback) / > > testb $4, X86_EXC_UD * TRAPINFO_sizeof + > > TRAPINFO_flags(%rdi) > > setnz %cl > > lea TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx > > + or $~0, %esi # don't clear DF > > Our predominant comment style is /* */ > > > UNLIKELY_END(syscall_no_callback) > > > > movq %rax,TRAPBOUNCE_eip(%rdx) > > movb %cl,TRAPBOUNCE_flags(%rdx) > > call create_bounce_frame > > - andl $~X86_EFLAGS_DF,UREGS_eflags(%rsp) > > + and %esi, UREGS_eflags(%rsp) > > Could we gain a /* Conditionally clear DF */ comment here? > > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > CC Oleksii for 4.19. This fixes a subtle regression in the PV ABI > caused by a post-XSA fix a few years ago. > > It's a low-risk fix to take; while I still don't have an XTF test > covering this, the corner case it's changing used to be completely > fatal > to guests, so it's a corner unused in practice. > Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com> ~ Oleksii
--- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -38,6 +38,14 @@ FUNC_LOCAL(switch_to_kernel) setc %cl leal (,%rcx,TBF_INTERRUPT),%ecx + /* + * The PV ABI hardcodes the (guest-inaccessible and virtual) + * SYSCALL_MASK MSR such that DF (and nothing else) would be cleared. + * Note that the equivalent of IF (VGCF_syscall_disables_events) is + * dealt with separately above. + */ + mov $~X86_EFLAGS_DF, %esi + test %rax, %rax UNLIKELY_START(z, syscall_no_callback) /* TB_eip == 0 => #UD */ mov VCPU_trap_ctxt(%rbx), %rdi @@ -47,12 +55,13 @@ UNLIKELY_START(z, syscall_no_callback) / testb $4, X86_EXC_UD * TRAPINFO_sizeof + TRAPINFO_flags(%rdi) setnz %cl lea TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx + or $~0, %esi # don't clear DF UNLIKELY_END(syscall_no_callback) movq %rax,TRAPBOUNCE_eip(%rdx) movb %cl,TRAPBOUNCE_flags(%rdx) call create_bounce_frame - andl $~X86_EFLAGS_DF,UREGS_eflags(%rsp) + and %esi, UREGS_eflags(%rsp) /* %rbx: struct vcpu */ test_all_events: ASSERT_NOT_IN_ATOMIC
While doing so is intentional when invoking the actual callback, to mimic a hard-coded SYCALL_MASK / FMASK MSR, the same should not be done when no handler is available and hence #UD is raised. Fixes: ca6fcf4321b3 ("x86/pv: Inject #UD for missing SYSCALL callbacks") Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com>