From: Sergiu Moga <sergiu@unikraft.io>
Date: Sat, 4 Nov 2023 08:54:06 +0000 (+0200)
Subject: lib/syscall_shim: Fix `arch_prctl`'s `ARCH_SET_FS` handling
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=0d36c37adebab961c2adb18452c283fc0ceb0b03;p=unikraft%2Funikraft.git

lib/syscall_shim: Fix `arch_prctl`'s `ARCH_SET_FS` handling

NOTE: This is interdependent on the `app-elfloader` commit that
changes `arch_prctl`'s `ARCH_SET_FS` behavior, entitled:
```
Do not change real `fs_base` register in `arch_prctl`
```

The code may store and restore userland TLS pointer properly in the
system call handler and it does check if it was changes by syscalls
such as `arch_prctl`'s `ARCH_SET_FS` command. However, one aspect
was missed: `__UK_SYSCALL_RETADDR_CLEAR`. The macro
`__UK_SYSCALL_RETADDR_CLEAR` accesses what is meant to be Unikraft's
TLS and is invoked in the time window between TLS store and restore.
In the `ARCH_SET_FS` case, `arch_prctl` sets the the `fs` register
regardless and thus we end up having the userland desired TLS pointer
during a time window where keeping our own TLS pointer is mandatory.

Thus, to avoid this, do not set any `fs` register value during
`arch_prctl` (see top NOTE) and, instead, simply modify the
`_uk_syscall_ultlsp` value which will be used instead to restore
the userland TLS as the last operation.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
---

diff --git a/lib/syscall_shim/uk_syscall_binary.c b/lib/syscall_shim/uk_syscall_binary.c
index f257e8890..26b4147a8 100644
--- a/lib/syscall_shim/uk_syscall_binary.c
+++ b/lib/syscall_shim/uk_syscall_binary.c
@@ -51,7 +51,7 @@ void ukplat_syscall_handler(struct __regs *r)
 {
 #if CONFIG_LIBSYSCALL_SHIM_HANDLER_ULTLS
 	struct uk_thread *self;
-	__uptr orig_tlsp;
+	__uptr ultlsp;
 #endif /* CONFIG_LIBSYSCALL_SHIM_HANDLER_ULTLS */
 	/* Place backup of extended register state on stack */
 	__sz ectx_align = ukarch_ectx_align();
@@ -75,11 +75,11 @@ void ukplat_syscall_handler(struct __regs *r)
 
 #if CONFIG_LIBSYSCALL_SHIM_HANDLER_ULTLS
 	/* Activate Unikraft TLS */
-	orig_tlsp = ukplat_tlsp_get();
+	ultlsp = ukplat_tlsp_get();
 	self = uk_thread_current();
 	UK_ASSERT(self);
 	ukplat_tlsp_set(self->uktlsp);
-	_uk_syscall_ultlsp = orig_tlsp;
+	_uk_syscall_ultlsp = ultlsp;
 #endif /* CONFIG_LIBSYSCALL_SHIM_HANDLER_ULTLS */
 
 	/* uk_syscall6_r() will clear _uk_syscall_return_addr on return */
@@ -112,17 +112,20 @@ void ukplat_syscall_handler(struct __regs *r)
 #endif /* CONFIG_LIBSYSCALL_SHIM_STRACE */
 
 #if CONFIG_LIBSYSCALL_SHIM_HANDLER_ULTLS
-	uk_thread_uktls_var(self, _uk_syscall_ultlsp) = 0x0;
-
 	/* Restore original TLS only if it was _NOT_
 	 * changed by the system call handler
 	 */
-	if (likely(ukplat_tlsp_get() == self->uktlsp)) {
-		ukplat_tlsp_set(orig_tlsp);
-	} else {
+#if CONFIG_LIBUKDEBUG_PRINTD
+	if (_uk_syscall_ultlsp != ultlsp) {
 		uk_pr_debug("System call updated userland TLS pointer register to %p (before: %p)\n",
-			    (void *) orig_tlsp, (void *) ukplat_tlsp_get());
+			    (void *)ultlsp, (void *)ukplat_tlsp_get());
 	}
+#endif
+
+	ultlsp = _uk_syscall_ultlsp;
+	uk_thread_uktls_var(self, _uk_syscall_ultlsp) = 0x0;
+
+	ukplat_tlsp_set(ultlsp);
 #endif /* CONFIG_LIBSYSCALL_SHIM_HANDLER_ULTLS */
 
 	/* Restore extended register state */