Re: [RFC PATCH v2 08/13] um: nommu: configure fs register on host syscall invocation

[Hajime Tazaki <thehajime@gmail.com>]

On Wed, 27 Nov 2024 19:00:11 +0900,
Benjamin Berg wrote:

> > +
> > +	os_info("Checking FSGSBASE instructions...");
> > +	if (sigsetjmp(jmpbuf, 0) == 0) {
> > +		asm volatile("rdfsbase %0" : "=r" (fsbase) :: "memory");
> > +		host_has_fsgsbase = 1;
> > +		os_info("OK\n");
> > +	} else {
> > +		host_has_fsgsbase = 0;
> > +		os_info("disabled\n");
> > +	}
> > +}
> 
> According to Documentation/arch/x86/x86_64/fsgs.rst it looks like this
> can also be checked using the HWCAP2_FSGSBASE bit in AT_HWCAP2.
> 
> Maybe that is a bit simpler?

Ah, thanks.  This should be much simpler:

+#include <sys/auxv.h>
+#include <asm/hwcap2.h>
+void __init check_fsgsbase(void)
+{
+       unsigned long auxv = getauxval(AT_HWCAP2);
+
+       os_info("Checking FSGSBASE instructions...");
+       if (auxv & HWCAP2_FSGSBASE) {
+               host_has_fsgsbase = 1;
+               os_info("OK\n");
+       } else {
+               host_has_fsgsbase = 0;
+               os_info("disabled\n");
+       }
+}


> 
> > [SNIP]
> > 
> >  __visible void do_syscall_64(struct pt_regs *regs)
> >  {
> >  	int syscall;
> > @@ -49,6 +76,9 @@ __visible void do_syscall_64(struct pt_regs *regs)
> >  	if (syscall == __NR_vfork)
> >  		stack_copy = vfork_save_stack();
> >  
> > +	/* set fs register to the original host one */
> > +	os_x86_arch_prctl(0, ARCH_SET_FS, (void *)host_fs);
> > +
> >  	if (likely(syscall < NR_syscalls)) {
> >  		PT_REGS_SET_SYSCALL_RETURN(regs,
> >  				EXECUTE_SYSCALL(syscall, regs));
> > @@ -63,6 +93,11 @@ __visible void do_syscall_64(struct pt_regs *regs)
> >  	set_thread_flag(TIF_SIGPENDING);
> >  	interrupt_end();
> >  
> > +	/* restore back fs register to userspace configured one */
> > +	os_x86_arch_prctl(0, ARCH_SET_FS,
> > +		      (void *)(current->thread.regs.regs.gp[FS_BASE
> > +						     / sizeof(unsigned long)]));
> > +
> >  	/* execve succeeded */
> >  	if (syscall == __NR_execve && regs->regs.gp[HOST_AX] == 0)
> >  		userspace(&current->thread.regs.regs);
> > diff --git a/arch/x86/um/syscalls_64.c b/arch/x86/um/syscalls_64.c
> > index edb17fc73e07..d56df936a2d7 100644
> > --- a/arch/x86/um/syscalls_64.c
> > +++ b/arch/x86/um/syscalls_64.c
> > @@ -12,11 +12,26 @@
> >  #include <asm/prctl.h> /* XXX This should get the constants from libc */
> >  #include <registers.h>
> >  #include <os.h>
> > +#include <asm/thread_info.h>
> > +#include <asm/mman.h>
> > +
> > +#ifndef CONFIG_MMU
> > +/*
> > + * The guest libc can change FS, which confuses the host libc.
> > + * In fact, changing FS directly is not supported (check
> > + * man arch_prctl). So, whenever we make a host syscall,
> > + * we should be changing FS to the original FS (not the
> > + * one set by the guest libc). This original FS is stored
> > + * in host_fs.
> > + */
> > +long long host_fs = -1;
> 
> Right, the libc already uses it for its own thread-local storage. That
> is a bit annoying, as UML doesn't need threading in that sense.
> 
> Note that similar handling needs to happen for every userspace to
> kernel switch. I guess the only other location is the signal handler.

Thanks too.
I guess the former (arch_switch_to) handles in my patch but the latter
(signal handler) doesn't.  Let me try to check.

-- Hajime