* [PATCH v9 00/10] Increased address space for 64 bit
@ 2024-09-19 12:45 Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 01/10] um: Add generic stub_syscall1 function Benjamin Berg
` (9 more replies)
0 siblings, 10 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
The new version of the patchset uses execveat on a memfd instead of
cloning twice to disable rseq. This should be much more robust going
forward as it will also avoid issues with other new features like mseal.
This patchset fixes a few bugs, adds a new method of discovering the
host task size and switches to four-level page tables on 64 bit. All of
this means the userspace TASK_SIZE is much larger and in turn permits
userspace applications that need a lot of virtual addresses to work
fine.
One such application is ASAN which uses a fixed address in memory that
would otherwise not be addressable.
v9:
* Drop support for 3-level page tables (on top of Tiwei Bie's patch)
* Add patches to set PR_SET_PDEATHSIG for more robust cleanup
* Cleanup top address calculation a bit more
* Use code address for stub data location
v8:
* Make changes suggested by Johannes Berg
v7:
* Plenty of changes to fix 32 bit and improve the logic
v6:
* Apply fixes pointed out by Tiwei Bie
* Add temporary file fallback as memfd is not always supported
v5:
* Use execveat with memfd instead of double clone
v4:
* Do not use WNOHANG in wait for CLONE_VFORK
v3:
* Undo incorrect change in child wait loop
v2:
* Improved double clone logic using CLONE_VFORK
* Kconfig fixes pointed out by Tiwei Bie
Benjamin Berg (10):
um: Add generic stub_syscall1 function
um: use execveat to create userspace MMs
um: Set parent death signal for userspace process
um: Set parent death signal for winch thread/process
um: Add compile time assert that stub fits on a page
um: Calculate stub data address relative to stub code
um: Limit TASK_SIZE to the addressable range
um: Discover host_task_size from envp
um: clear all memory in new userspace processes
um: Switch to 4 level page tables on 64 bit
arch/um/Kconfig | 4 +-
arch/um/Makefile | 3 +-
arch/um/drivers/chan_user.c | 3 +
arch/um/include/asm/page.h | 14 +-
arch/um/include/asm/pgalloc.h | 11 +-
.../{pgtable-3level.h => pgtable-4level.h} | 40 +++-
arch/um/include/asm/pgtable.h | 8 +-
arch/um/include/shared/as-layout.h | 2 +-
arch/um/include/shared/os.h | 3 -
arch/um/include/shared/skas/stub-data.h | 11 ++
arch/um/kernel/dyn.lds.S | 3 +
arch/um/kernel/mem.c | 17 +-
arch/um/kernel/skas/.gitignore | 2 +
arch/um/kernel/skas/Makefile | 33 +++-
arch/um/kernel/skas/mmu.c | 25 +--
arch/um/kernel/skas/stub_exe.c | 91 +++++++++
arch/um/kernel/skas/stub_exe_embed.S | 11 ++
arch/um/kernel/um_arch.c | 34 +++-
arch/um/os-Linux/main.c | 9 +-
arch/um/os-Linux/mem.c | 2 +-
arch/um/os-Linux/skas/process.c | 181 ++++++++++++------
arch/x86/um/Kconfig | 3 -
arch/x86/um/os-Linux/Makefile | 2 +-
arch/x86/um/os-Linux/task_size.c | 151 ---------------
arch/x86/um/shared/sysdep/stub_32.h | 10 +-
arch/x86/um/shared/sysdep/stub_64.h | 19 +-
26 files changed, 420 insertions(+), 272 deletions(-)
rename arch/um/include/asm/{pgtable-3level.h => pgtable-4level.h} (66%)
create mode 100644 arch/um/kernel/skas/.gitignore
create mode 100644 arch/um/kernel/skas/stub_exe.c
create mode 100644 arch/um/kernel/skas/stub_exe_embed.S
delete mode 100644 arch/x86/um/os-Linux/task_size.c
--
2.46.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v9 01/10] um: Add generic stub_syscall1 function
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 02/10] um: use execveat to create userspace MMs Benjamin Berg
` (8 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
The 64bit version did not have a stub_syscall1 function yet. Add it as
it will be useful to implement a static binary for stub loading.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/x86/um/shared/sysdep/stub_64.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/x86/um/shared/sysdep/stub_64.h b/arch/x86/um/shared/sysdep/stub_64.h
index 67f44284f1aa..8e4ff39dcade 100644
--- a/arch/x86/um/shared/sysdep/stub_64.h
+++ b/arch/x86/um/shared/sysdep/stub_64.h
@@ -28,6 +28,17 @@ static __always_inline long stub_syscall0(long syscall)
return ret;
}
+static __always_inline long stub_syscall1(long syscall, long arg1)
+{
+ long ret;
+
+ __asm__ volatile (__syscall
+ : "=a" (ret)
+ : "0" (syscall), "D" (arg1) : __syscall_clobber );
+
+ return ret;
+}
+
static __always_inline long stub_syscall2(long syscall, long arg1, long arg2)
{
long ret;
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 01/10] um: Add generic stub_syscall1 function Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-10-16 2:34 ` Nathan Chancellor
2024-10-17 7:17 ` David Gow
2024-09-19 12:45 ` [PATCH v9 03/10] um: Set parent death signal for userspace process Benjamin Berg
` (7 subsequent siblings)
9 siblings, 2 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
Using clone will not undo features that have been enabled by libc. An
example of this already happening is rseq, which could cause the kernel
to read/write memory of the userspace process. In the future the
standard library might also use mseal by default to protect itself,
which would also thwart our attempts at unmapping everything.
Solve all this by taking a step back and doing an execve into a tiny
static binary that sets up the minimal environment required for the
stub without using any standard library. That way we have a clean
execution environment that is fully under the control of UML.
Note that this changes things a bit as the FDs are not anymore shared
with the kernel. Instead, we explicitly share the FDs for the physical
memory and all existing iomem regions. Doing this is fine, as iomem
regions cannot be added at runtime.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
v9:
- Avoid clash of libc __close_range with the kernel version,
thanks to Tiwei Bie for pointing this out
v8:
- Make changes suggested by Johannes Berg
v7:
- Rename stub_elf to stub_exe
- Move into architecture independent directory
- Fix 32 bit issues
- Improve tempfile logic
- Other cleanups
v6:
- Apply fixes pointed out by Tiwei Bie
- Add temporary file fallback as memfd is not always supported
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/Makefile | 3 +-
arch/um/include/shared/skas/stub-data.h | 11 ++
arch/um/kernel/skas/.gitignore | 2 +
arch/um/kernel/skas/Makefile | 33 ++++-
arch/um/kernel/skas/stub_exe.c | 88 ++++++++++++
arch/um/kernel/skas/stub_exe_embed.S | 11 ++
arch/um/os-Linux/mem.c | 2 +-
arch/um/os-Linux/skas/process.c | 181 ++++++++++++++++--------
8 files changed, 271 insertions(+), 60 deletions(-)
create mode 100644 arch/um/kernel/skas/.gitignore
create mode 100644 arch/um/kernel/skas/stub_exe.c
create mode 100644 arch/um/kernel/skas/stub_exe_embed.S
diff --git a/arch/um/Makefile b/arch/um/Makefile
index 00b63bac5eff..31e367e8ab4d 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -61,7 +61,8 @@ KBUILD_CFLAGS += $(CFLAGS) $(CFLAGS-y) -D__arch_um__ \
$(ARCH_INCLUDE) $(MODE_INCLUDE) -Dvmap=kernel_vmap \
-Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
-Din6addr_loopback=kernel_in6addr_loopback \
- -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr
+ -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr \
+ -D__close_range=kernel__close_range
KBUILD_RUSTFLAGS += -Crelocation-model=pie
diff --git a/arch/um/include/shared/skas/stub-data.h b/arch/um/include/shared/skas/stub-data.h
index 2b6b44759dfa..3fbdda727373 100644
--- a/arch/um/include/shared/skas/stub-data.h
+++ b/arch/um/include/shared/skas/stub-data.h
@@ -12,6 +12,17 @@
#include <as-layout.h>
#include <sysdep/tls.h>
+struct stub_init_data {
+ unsigned long stub_start;
+
+ int stub_code_fd;
+ unsigned long stub_code_offset;
+ int stub_data_fd;
+ unsigned long stub_data_offset;
+
+ unsigned long segv_handler;
+};
+
#define STUB_NEXT_SYSCALL(s) \
((struct stub_syscall *) (((unsigned long) s) + (s)->cmd_len))
diff --git a/arch/um/kernel/skas/.gitignore b/arch/um/kernel/skas/.gitignore
new file mode 100644
index 000000000000..c3409ced0f38
--- /dev/null
+++ b/arch/um/kernel/skas/.gitignore
@@ -0,0 +1,2 @@
+stub_exe
+stub_exe.dbg
diff --git a/arch/um/kernel/skas/Makefile b/arch/um/kernel/skas/Makefile
index 6f86d53e3d69..fbb61968055f 100644
--- a/arch/um/kernel/skas/Makefile
+++ b/arch/um/kernel/skas/Makefile
@@ -3,14 +3,43 @@
# Copyright (C) 2002 - 2007 Jeff Dike (jdike@{addtoit,linux.intel}.com)
#
-obj-y := stub.o mmu.o process.o syscall.o uaccess.o
+obj-y := stub.o mmu.o process.o syscall.o uaccess.o \
+ stub_exe_embed.o
+
+# Stub executable
+
+stub_exe_objs-y := stub_exe.o
+
+stub_exe_objs := $(foreach F,$(stub_exe_objs-y),$(obj)/$F)
+
+# Object file containing the ELF executable
+$(obj)/stub_exe_embed.o: $(src)/stub_exe_embed.S $(obj)/stub_exe
+
+$(obj)/stub_exe.dbg: $(stub_exe_objs) FORCE
+ $(call if_changed,stub_exe)
+
+$(obj)/stub_exe: OBJCOPYFLAGS := -S
+$(obj)/stub_exe: $(obj)/stub_exe.dbg FORCE
+ $(call if_changed,objcopy)
+
+quiet_cmd_stub_exe = STUB_EXE $@
+ cmd_stub_exe = $(CC) -nostdlib -o $@ \
+ $(KBUILD_CFLAGS) $(STUB_EXE_LDFLAGS) \
+ $(filter %.o,$^)
+
+STUB_EXE_LDFLAGS = -n -static
+
+targets += stub_exe.dbg stub_exe $(stub_exe_objs-y)
+
+# end
# stub.o is in the stub, so it can't be built with profiling
# GCC hardened also auto-enables -fpic, but we need %ebx so it can't work ->
# disable it
CFLAGS_stub.o := $(CFLAGS_NO_HARDENING)
-UNPROFILE_OBJS := stub.o
+CFLAGS_stub_exe.o := $(CFLAGS_NO_HARDENING)
+UNPROFILE_OBJS := stub.o stub_exe.o
KCOV_INSTRUMENT := n
include $(srctree)/arch/um/scripts/Makefile.rules
diff --git a/arch/um/kernel/skas/stub_exe.c b/arch/um/kernel/skas/stub_exe.c
new file mode 100644
index 000000000000..bc6ba2e4d805
--- /dev/null
+++ b/arch/um/kernel/skas/stub_exe.c
@@ -0,0 +1,88 @@
+#include <sys/ptrace.h>
+#include <sys/prctl.h>
+#include <asm/unistd.h>
+#include <sysdep/stub.h>
+#include <stub-data.h>
+
+void _start(void);
+
+noinline static void real_init(void)
+{
+ struct stub_init_data init_data;
+ unsigned long res;
+ struct {
+ void *ss_sp;
+ int ss_flags;
+ size_t ss_size;
+ } stack = {
+ .ss_size = STUB_DATA_PAGES * UM_KERN_PAGE_SIZE,
+ };
+ struct {
+ void *sa_handler_;
+ unsigned long sa_flags;
+ void *sa_restorer;
+ unsigned long long sa_mask;
+ } sa = {
+ /* Need to set SA_RESTORER (but the handler never returns) */
+ .sa_flags = SA_ONSTACK | SA_NODEFER | SA_SIGINFO | 0x04000000,
+ /* no need to mask any signals */
+ .sa_mask = 0,
+ };
+
+ /* set a nice name */
+ stub_syscall2(__NR_prctl, PR_SET_NAME, (unsigned long)"uml-userspace");
+
+ /* read information from STDIN and close it */
+ res = stub_syscall3(__NR_read, 0,
+ (unsigned long)&init_data, sizeof(init_data));
+ if (res != sizeof(init_data))
+ stub_syscall1(__NR_exit, 10);
+
+ stub_syscall1(__NR_close, 0);
+
+ /* map stub code + data */
+ res = stub_syscall6(STUB_MMAP_NR,
+ init_data.stub_start, UM_KERN_PAGE_SIZE,
+ PROT_READ | PROT_EXEC, MAP_FIXED | MAP_SHARED,
+ init_data.stub_code_fd, init_data.stub_code_offset);
+ if (res != init_data.stub_start)
+ stub_syscall1(__NR_exit, 11);
+
+ res = stub_syscall6(STUB_MMAP_NR,
+ init_data.stub_start + UM_KERN_PAGE_SIZE,
+ STUB_DATA_PAGES * UM_KERN_PAGE_SIZE,
+ PROT_READ | PROT_WRITE, MAP_FIXED | MAP_SHARED,
+ init_data.stub_data_fd, init_data.stub_data_offset);
+ if (res != init_data.stub_start + UM_KERN_PAGE_SIZE)
+ stub_syscall1(__NR_exit, 12);
+
+ /* setup signal stack inside stub data */
+ stack.ss_sp = (void *)init_data.stub_start + UM_KERN_PAGE_SIZE;
+ stub_syscall2(__NR_sigaltstack, (unsigned long)&stack, 0);
+
+ /* register SIGSEGV handler */
+ sa.sa_handler_ = (void *) init_data.segv_handler;
+ res = stub_syscall4(__NR_rt_sigaction, SIGSEGV, (unsigned long)&sa, 0,
+ sizeof(sa.sa_mask));
+ if (res != 0)
+ stub_syscall1(__NR_exit, 13);
+
+ stub_syscall4(__NR_ptrace, PTRACE_TRACEME, 0, 0, 0);
+
+ stub_syscall2(__NR_kill, stub_syscall0(__NR_getpid), SIGSTOP);
+
+ stub_syscall1(__NR_exit, 14);
+
+ __builtin_unreachable();
+}
+
+void _start(void)
+{
+ char *alloc;
+
+ /* Make enough space for the stub (including space for alignment) */
+ alloc = __builtin_alloca((1 + 2 * STUB_DATA_PAGES - 1) * UM_KERN_PAGE_SIZE);
+ asm volatile("" : "+r,m"(alloc) : : "memory");
+
+ real_init();
+}
diff --git a/arch/um/kernel/skas/stub_exe_embed.S b/arch/um/kernel/skas/stub_exe_embed.S
new file mode 100644
index 000000000000..6d8914fbe8f1
--- /dev/null
+++ b/arch/um/kernel/skas/stub_exe_embed.S
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/init.h>
+#include <linux/linkage.h>
+
+__INITDATA
+
+SYM_DATA_START(stub_exe_start)
+ .incbin "arch/um/kernel/skas/stub_exe"
+SYM_DATA_END_LABEL(stub_exe_start, SYM_L_GLOBAL, stub_exe_end)
+
+__FINIT
diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c
index cf44d386f23c..857e3deab293 100644
--- a/arch/um/os-Linux/mem.c
+++ b/arch/um/os-Linux/mem.c
@@ -42,7 +42,7 @@ void kasan_map_memory(void *start, size_t len)
}
/* Set by make_tempfile() during early boot. */
-static char *tempdir = NULL;
+char *tempdir = NULL;
/* Check if dir is on tmpfs. Return 0 if yes, -1 if no or error. */
static int __init check_tmpfs(const char *dir)
diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
index b6f656bcffb1..2caaa9e85752 100644
--- a/arch/um/os-Linux/skas/process.c
+++ b/arch/um/os-Linux/skas/process.c
@@ -10,8 +10,11 @@
#include <sched.h>
#include <errno.h>
#include <string.h>
+#include <fcntl.h>
+#include <mem_user.h>
#include <sys/mman.h>
#include <sys/wait.h>
+#include <sys/stat.h>
#include <asm/unistd.h>
#include <as-layout.h>
#include <init.h>
@@ -189,69 +192,135 @@ static void handle_trap(int pid, struct uml_pt_regs *regs)
extern char __syscall_stub_start[];
-/**
- * userspace_tramp() - userspace trampoline
- * @stack: pointer to the new userspace stack page
- *
- * The userspace trampoline is used to setup a new userspace process in start_userspace() after it was clone()'ed.
- * This function will run on a temporary stack page.
- * It ptrace()'es itself, then
- * Two pages are mapped into the userspace address space:
- * - STUB_CODE (with EXEC), which contains the skas stub code
- * - STUB_DATA (with R/W), which contains a data page that is used to transfer certain data between the UML userspace process and the UML kernel.
- * Also for the userspace process a SIGSEGV handler is installed to catch pagefaults in the userspace process.
- * And last the process stops itself to give control to the UML kernel for this userspace process.
- *
- * Return: Always zero, otherwise the current userspace process is ended with non null exit() call
- */
+static int stub_exe_fd;
+
static int userspace_tramp(void *stack)
{
- struct sigaction sa;
- void *addr;
- int fd;
+ char *const argv[] = { "uml-userspace", NULL };
+ int pipe_fds[2];
unsigned long long offset;
- unsigned long segv_handler = STUB_CODE +
- (unsigned long) stub_segv_handler -
- (unsigned long) __syscall_stub_start;
-
- ptrace(PTRACE_TRACEME, 0, 0, 0);
-
- signal(SIGTERM, SIG_DFL);
- signal(SIGWINCH, SIG_IGN);
-
- fd = phys_mapping(uml_to_phys(__syscall_stub_start), &offset);
- addr = mmap64((void *) STUB_CODE, UM_KERN_PAGE_SIZE,
- PROT_EXEC, MAP_FIXED | MAP_PRIVATE, fd, offset);
- if (addr == MAP_FAILED) {
- os_info("mapping mmap stub at 0x%lx failed, errno = %d\n",
- STUB_CODE, errno);
- exit(1);
+ struct stub_init_data init_data = {
+ .stub_start = STUB_START,
+ .segv_handler = STUB_CODE +
+ (unsigned long) stub_segv_handler -
+ (unsigned long) __syscall_stub_start,
+ };
+ struct iomem_region *iomem;
+ int ret;
+
+ init_data.stub_code_fd = phys_mapping(uml_to_phys(__syscall_stub_start),
+ &offset);
+ init_data.stub_code_offset = MMAP_OFFSET(offset);
+
+ init_data.stub_data_fd = phys_mapping(uml_to_phys(stack), &offset);
+ init_data.stub_data_offset = MMAP_OFFSET(offset);
+
+ /* Set CLOEXEC on all FDs and then unset on all memory related FDs */
+ close_range(0, ~0U, CLOSE_RANGE_CLOEXEC);
+
+ fcntl(init_data.stub_data_fd, F_SETFD, 0);
+ for (iomem = iomem_regions; iomem; iomem = iomem->next)
+ fcntl(iomem->fd, F_SETFD, 0);
+
+ /* Create a pipe for init_data (no CLOEXEC) and dup2 to STDIN */
+ if (pipe2(pipe_fds, 0))
+ exit(2);
+
+ close(0);
+ if (dup2(pipe_fds[0], 0) < 0) {
+ close(pipe_fds[0]);
+ close(pipe_fds[1]);
+ exit(3);
}
+ close(pipe_fds[0]);
+
+ /* Write init_data and close write side */
+ ret = write(pipe_fds[1], &init_data, sizeof(init_data));
+ close(pipe_fds[1]);
+
+ if (ret != sizeof(init_data))
+ exit(4);
+
+ execveat(stub_exe_fd, "", argv, NULL, AT_EMPTY_PATH);
- fd = phys_mapping(uml_to_phys(stack), &offset);
- addr = mmap((void *) STUB_DATA,
- STUB_DATA_PAGES * UM_KERN_PAGE_SIZE, PROT_READ | PROT_WRITE,
- MAP_FIXED | MAP_SHARED, fd, offset);
- if (addr == MAP_FAILED) {
- os_info("mapping segfault stack at 0x%lx failed, errno = %d\n",
- STUB_DATA, errno);
- exit(1);
+ exit(5);
+}
+
+extern char stub_exe_start[];
+extern char stub_exe_end[];
+
+extern char *tempdir;
+
+#define STUB_EXE_NAME_TEMPLATE "/uml-userspace-XXXXXX"
+
+#ifndef MFD_EXEC
+#define MFD_EXEC 0x0010U
+#endif
+
+static int __init init_stub_exe_fd(void)
+{
+ size_t written = 0;
+ char *tmpfile = NULL;
+
+ stub_exe_fd = memfd_create("uml-userspace",
+ MFD_EXEC | MFD_CLOEXEC | MFD_ALLOW_SEALING);
+
+ if (stub_exe_fd < 0) {
+ printk(UM_KERN_INFO "Could not create executable memfd, using temporary file!");
+
+ tmpfile = malloc(strlen(tempdir) +
+ strlen(STUB_EXE_NAME_TEMPLATE) + 1);
+ if (tmpfile == NULL)
+ panic("Failed to allocate memory for stub binary name");
+
+ strcpy(tmpfile, tempdir);
+ strcat(tmpfile, STUB_EXE_NAME_TEMPLATE);
+
+ stub_exe_fd = mkstemp(tmpfile);
+ if (stub_exe_fd < 0)
+ panic("Could not create temporary file for stub binary: %d",
+ -errno);
}
- set_sigstack((void *) STUB_DATA, STUB_DATA_PAGES * UM_KERN_PAGE_SIZE);
- sigemptyset(&sa.sa_mask);
- sa.sa_flags = SA_ONSTACK | SA_NODEFER | SA_SIGINFO;
- sa.sa_sigaction = (void *) segv_handler;
- sa.sa_restorer = NULL;
- if (sigaction(SIGSEGV, &sa, NULL) < 0) {
- os_info("%s - setting SIGSEGV handler failed - errno = %d\n",
- __func__, errno);
- exit(1);
+ while (written < stub_exe_end - stub_exe_start) {
+ ssize_t res = write(stub_exe_fd, stub_exe_start + written,
+ stub_exe_end - stub_exe_start - written);
+ if (res < 0) {
+ if (errno == EINTR)
+ continue;
+
+ if (tmpfile)
+ unlink(tmpfile);
+ panic("Failed write stub binary: %d", -errno);
+ }
+
+ written += res;
+ }
+
+ if (!tmpfile) {
+ fcntl(stub_exe_fd, F_ADD_SEALS,
+ F_SEAL_WRITE | F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_SEAL);
+ } else {
+ if (fchmod(stub_exe_fd, 00500) < 0) {
+ unlink(tmpfile);
+ panic("Could not make stub binary executable: %d",
+ -errno);
+ }
+
+ close(stub_exe_fd);
+ stub_exe_fd = open(tmpfile, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
+ if (stub_exe_fd < 0) {
+ unlink(tmpfile);
+ panic("Could not reopen stub binary: %d", -errno);
+ }
+
+ unlink(tmpfile);
+ free(tmpfile);
}
- kill(os_getpid(), SIGSTOP);
return 0;
}
+__initcall(init_stub_exe_fd);
int userspace_pid[NR_CPUS];
@@ -270,7 +339,7 @@ int start_userspace(unsigned long stub_stack)
{
void *stack;
unsigned long sp;
- int pid, status, n, flags, err;
+ int pid, status, n, err;
/* setup a temporary stack page */
stack = mmap(NULL, UM_KERN_PAGE_SIZE,
@@ -286,10 +355,10 @@ int start_userspace(unsigned long stub_stack)
/* set stack pointer to the end of the stack page, so it can grow downwards */
sp = (unsigned long)stack + UM_KERN_PAGE_SIZE;
- flags = CLONE_FILES | SIGCHLD;
-
/* clone into new userspace process */
- pid = clone(userspace_tramp, (void *) sp, flags, (void *) stub_stack);
+ pid = clone(userspace_tramp, (void *) sp,
+ CLONE_VFORK | CLONE_VM | SIGCHLD,
+ (void *)stub_stack);
if (pid < 0) {
err = -errno;
printk(UM_KERN_ERR "%s : clone failed, errno = %d\n",
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 03/10] um: Set parent death signal for userspace process
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 01/10] um: Add generic stub_syscall1 function Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 02/10] um: use execveat to create userspace MMs Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 04/10] um: Set parent death signal for winch thread/process Benjamin Berg
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
Enable PR_SET_PDEATHSIG so that the UML userspace process will be killed
when the kernel exits unexpectedly.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/kernel/skas/stub_exe.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/um/kernel/skas/stub_exe.c b/arch/um/kernel/skas/stub_exe.c
index bc6ba2e4d805..04f75c577f1a 100644
--- a/arch/um/kernel/skas/stub_exe.c
+++ b/arch/um/kernel/skas/stub_exe.c
@@ -32,6 +32,9 @@ noinline static void real_init(void)
/* set a nice name */
stub_syscall2(__NR_prctl, PR_SET_NAME, (unsigned long)"uml-userspace");
+ /* Make sure this process dies if the kernel dies */
+ stub_syscall2(__NR_prctl, PR_SET_PDEATHSIG, SIGKILL);
+
/* read information from STDIN and close it */
res = stub_syscall3(__NR_read, 0,
(unsigned long)&init_data, sizeof(init_data));
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 04/10] um: Set parent death signal for winch thread/process
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (2 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 03/10] um: Set parent death signal for userspace process Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 05/10] um: Add compile time assert that stub fits on a page Benjamin Berg
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
The winch "thread" is really a separate process. Using prctl to set
PR_SET_PDEATHSIG ensures that this separate thread will be killed if the
UML kernel itself dies unexpectedly and does not perform proper cleanup.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/drivers/chan_user.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
index a66e556012c4..1434114b2f34 100644
--- a/arch/um/drivers/chan_user.c
+++ b/arch/um/drivers/chan_user.c
@@ -10,6 +10,7 @@
#include <signal.h>
#include <termios.h>
#include <sys/ioctl.h>
+#include <sys/prctl.h>
#include "chan_user.h"
#include <os.h>
#include <um_malloc.h>
@@ -161,6 +162,8 @@ static __noreturn int winch_thread(void *arg)
int count;
char c = 1;
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+
pty_fd = data->pty_fd;
pipe_fd = data->pipe_fd;
count = write(pipe_fd, &c, sizeof(c));
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 05/10] um: Add compile time assert that stub fits on a page
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (3 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 04/10] um: Set parent death signal for winch thread/process Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 06/10] um: Calculate stub data address relative to stub code Benjamin Berg
` (4 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
The code assumes that the stub code can fit into a single page. This is
unlikely to ever change, but add a link time assert instead so that
there will be no hard to debug error.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/kernel/dyn.lds.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S
index 3385d653ebd0..dc9d9a68af55 100644
--- a/arch/um/kernel/dyn.lds.S
+++ b/arch/um/kernel/dyn.lds.S
@@ -178,3 +178,6 @@ SECTIONS
DISCARDS
}
+
+ASSERT(__syscall_stub_end - __syscall_stub_start <= PAGE_SIZE,
+ "STUB code must not be larger than one page");
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 06/10] um: Calculate stub data address relative to stub code
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (4 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 05/10] um: Add compile time assert that stub fits on a page Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 07/10] um: Limit TASK_SIZE to the addressable range Benjamin Berg
` (3 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
Instead of using the current stack pointer, we can also use the current
instruction to calculate where the stub data is. With this the stub data
only needs to be aligned to a full page boundary.
Changing this has the advantage that we do not have a hole in the memory
space above the stub data (which would need to be explicitly cleared).
Another motivation to do this is that with the planned addition of a
SECCOMP based userspace the stack pointer may not be fully trustworthy.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/kernel/um_arch.c | 6 ++----
arch/x86/um/shared/sysdep/stub_32.h | 10 +++++++---
arch/x86/um/shared/sysdep/stub_64.h | 8 +++++---
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 8e594cda6d77..44589cbd4174 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -325,10 +325,8 @@ int __init linux_main(int argc, char **argv)
add_arg(DEFAULT_COMMAND_LINE_CONSOLE);
host_task_size = os_get_top_address();
- /* reserve a few pages for the stubs (taking care of data alignment) */
- /* align the data portion */
- BUILD_BUG_ON(!is_power_of_2(STUB_DATA_PAGES));
- stub_start = (host_task_size - 1) & ~(STUB_DATA_PAGES * PAGE_SIZE - 1);
+ /* reserve a few pages for the stubs */
+ stub_start = host_task_size - STUB_DATA_PAGES * PAGE_SIZE;
/* another page for the code portion */
stub_start -= PAGE_SIZE;
host_task_size = stub_start;
diff --git a/arch/x86/um/shared/sysdep/stub_32.h b/arch/x86/um/shared/sysdep/stub_32.h
index 0b44a86dd346..631a18d0ff44 100644
--- a/arch/x86/um/shared/sysdep/stub_32.h
+++ b/arch/x86/um/shared/sysdep/stub_32.h
@@ -112,10 +112,14 @@ static __always_inline void *get_stub_data(void)
unsigned long ret;
asm volatile (
- "movl %%esp,%0 ;"
- "andl %1,%0"
+ "call _here_%=;"
+ "_here_%=:"
+ "popl %0;"
+ "andl %1, %0 ;"
+ "addl %2, %0 ;"
: "=a" (ret)
- : "g" (~(STUB_DATA_PAGES * UM_KERN_PAGE_SIZE - 1)));
+ : "g" (~(UM_KERN_PAGE_SIZE - 1)),
+ "g" (UM_KERN_PAGE_SIZE));
return (void *)ret;
}
diff --git a/arch/x86/um/shared/sysdep/stub_64.h b/arch/x86/um/shared/sysdep/stub_64.h
index 8e4ff39dcade..17153dfd780a 100644
--- a/arch/x86/um/shared/sysdep/stub_64.h
+++ b/arch/x86/um/shared/sysdep/stub_64.h
@@ -117,10 +117,12 @@ static __always_inline void *get_stub_data(void)
unsigned long ret;
asm volatile (
- "movq %%rsp,%0 ;"
- "andq %1,%0"
+ "lea 0(%%rip), %0;"
+ "andq %1, %0 ;"
+ "addq %2, %0 ;"
: "=a" (ret)
- : "g" (~(STUB_DATA_PAGES * UM_KERN_PAGE_SIZE - 1)));
+ : "g" (~(UM_KERN_PAGE_SIZE - 1)),
+ "g" (UM_KERN_PAGE_SIZE));
return (void *)ret;
}
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 07/10] um: Limit TASK_SIZE to the addressable range
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (5 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 06/10] um: Calculate stub data address relative to stub code Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 08/10] um: Discover host_task_size from envp Benjamin Berg
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
We may have a TASK_SIZE from the host that is bigger than UML is able to
address with a three-level pagetable on 64-bit. Guard against that by
clipping the maximum TASK_SIZE to the maximum addressable area.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
v9: This patch is technically not needed anymore, but does not hurt
v7: Fix integer overflow on 32 bit with 3-level page tables
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/kernel/um_arch.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 44589cbd4174..6f0bedbf048a 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -331,11 +331,16 @@ int __init linux_main(int argc, char **argv)
stub_start -= PAGE_SIZE;
host_task_size = stub_start;
+ /* Limit TASK_SIZE to what is addressable by the page table */
+ task_size = host_task_size;
+ if (task_size > (unsigned long long) PTRS_PER_PGD * PGDIR_SIZE)
+ task_size = PTRS_PER_PGD * PGDIR_SIZE;
+
/*
* TASK_SIZE needs to be PGDIR_SIZE aligned or else exit_mmap craps
* out
*/
- task_size = host_task_size & PGDIR_MASK;
+ task_size = task_size & PGDIR_MASK;
/* OS sanity checks that need to happen before the kernel runs */
os_early_checks();
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 08/10] um: Discover host_task_size from envp
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (6 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 07/10] um: Limit TASK_SIZE to the addressable range Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 09/10] um: clear all memory in new userspace processes Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 10/10] um: Switch to 4 level page tables on 64 bit Benjamin Berg
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
When loading the UML binary, the host kernel will place the stack at the
highest possible address. It will then map the program name and
environment variables onto the start of the stack.
As such, an easy way to figure out the host_task_size is to use the
highest pointer to an environment variable as a reference.
Ensure that this works by disabling address layout randomization and
re-executing UML in case it was enabled.
This increases the available TASK_SIZE for 64 bit UML considerably.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
v9: Move the implementation as it is architecture independent now
v7: Also use the same logic on 32bit
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/include/shared/as-layout.h | 2 +-
arch/um/include/shared/os.h | 3 -
arch/um/kernel/um_arch.c | 21 +++-
arch/um/os-Linux/main.c | 9 +-
arch/x86/um/os-Linux/Makefile | 2 +-
arch/x86/um/os-Linux/task_size.c | 151 -----------------------------
6 files changed, 29 insertions(+), 159 deletions(-)
delete mode 100644 arch/x86/um/os-Linux/task_size.c
diff --git a/arch/um/include/shared/as-layout.h b/arch/um/include/shared/as-layout.h
index 06292fca5a4d..b69cb8dcfeed 100644
--- a/arch/um/include/shared/as-layout.h
+++ b/arch/um/include/shared/as-layout.h
@@ -48,7 +48,7 @@ extern unsigned long brk_start;
extern unsigned long host_task_size;
extern unsigned long stub_start;
-extern int linux_main(int argc, char **argv);
+extern int linux_main(int argc, char **argv, char **envp);
extern void uml_finishsetup(void);
struct siginfo;
diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 9a039d6f1f74..73b5fe1542d2 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -329,9 +329,6 @@ extern int __ignore_sigio_fd(int fd);
/* tty.c */
extern int get_pty(void);
-/* sys-$ARCH/task_size.c */
-extern unsigned long os_get_top_address(void);
-
long syscall(long number, ...);
/* irqflags tracing */
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 6f0bedbf048a..285aa6fed188 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -302,7 +302,24 @@ static void parse_cache_line(char *line)
}
}
-int __init linux_main(int argc, char **argv)
+static unsigned long get_top_address(char **envp)
+{
+ unsigned long top_addr = (unsigned long) &top_addr;
+ int i;
+
+ /* The earliest variable should be after the program name in ELF */
+ for (i = 0; envp[i]; i++) {
+ if ((unsigned long) envp[i] > top_addr)
+ top_addr = (unsigned long) envp[i];
+ }
+
+ top_addr &= ~(UM_KERN_PAGE_SIZE - 1);
+ top_addr += UM_KERN_PAGE_SIZE;
+
+ return top_addr;
+}
+
+int __init linux_main(int argc, char **argv, char **envp)
{
unsigned long avail, diff;
unsigned long virtmem_size, max_physmem;
@@ -324,7 +341,7 @@ int __init linux_main(int argc, char **argv)
if (have_console == 0)
add_arg(DEFAULT_COMMAND_LINE_CONSOLE);
- host_task_size = os_get_top_address();
+ host_task_size = get_top_address(envp);
/* reserve a few pages for the stubs */
stub_start = host_task_size - STUB_DATA_PAGES * PAGE_SIZE;
/* another page for the code portion */
diff --git a/arch/um/os-Linux/main.c b/arch/um/os-Linux/main.c
index f98ff79cdbf7..9a61b1767795 100644
--- a/arch/um/os-Linux/main.c
+++ b/arch/um/os-Linux/main.c
@@ -11,6 +11,7 @@
#include <signal.h>
#include <string.h>
#include <sys/resource.h>
+#include <sys/personality.h>
#include <as-layout.h>
#include <init.h>
#include <kern_util.h>
@@ -108,6 +109,12 @@ int __init main(int argc, char **argv, char **envp)
char **new_argv;
int ret, i, err;
+ /* Disable randomization and re-exec if it was changed successfully */
+ ret = personality(PER_LINUX | ADDR_NO_RANDOMIZE);
+ if (ret >= 0 && (ret & (PER_LINUX | ADDR_NO_RANDOMIZE)) !=
+ (PER_LINUX | ADDR_NO_RANDOMIZE))
+ execve("/proc/self/exe", argv, envp);
+
set_stklim();
setup_env_path();
@@ -140,7 +147,7 @@ int __init main(int argc, char **argv, char **envp)
#endif
change_sig(SIGPIPE, 0);
- ret = linux_main(argc, argv);
+ ret = linux_main(argc, argv, envp);
/*
* Disable SIGPROF - I have no idea why libc doesn't do this or turn
diff --git a/arch/x86/um/os-Linux/Makefile b/arch/x86/um/os-Linux/Makefile
index 5249bbc30dcd..77a308aaa5ec 100644
--- a/arch/x86/um/os-Linux/Makefile
+++ b/arch/x86/um/os-Linux/Makefile
@@ -3,7 +3,7 @@
# Licensed under the GPL
#
-obj-y = registers.o task_size.o mcontext.o
+obj-y = registers.o mcontext.o
obj-$(CONFIG_X86_32) += tls.o
diff --git a/arch/x86/um/os-Linux/task_size.c b/arch/x86/um/os-Linux/task_size.c
deleted file mode 100644
index 1dc9adc20b1c..000000000000
--- a/arch/x86/um/os-Linux/task_size.c
+++ /dev/null
@@ -1,151 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <stdio.h>
-#include <stdlib.h>
-#include <signal.h>
-#include <sys/mman.h>
-#include <longjmp.h>
-
-#ifdef __i386__
-
-static jmp_buf buf;
-
-static void segfault(int sig)
-{
- longjmp(buf, 1);
-}
-
-static int page_ok(unsigned long page)
-{
- unsigned long *address = (unsigned long *) (page << UM_KERN_PAGE_SHIFT);
- unsigned long n = ~0UL;
- void *mapped = NULL;
- int ok = 0;
-
- /*
- * First see if the page is readable. If it is, it may still
- * be a VDSO, so we go on to see if it's writable. If not
- * then try mapping memory there. If that fails, then we're
- * still in the kernel area. As a sanity check, we'll fail if
- * the mmap succeeds, but gives us an address different from
- * what we wanted.
- */
- if (setjmp(buf) == 0)
- n = *address;
- else {
- mapped = mmap(address, UM_KERN_PAGE_SIZE,
- PROT_READ | PROT_WRITE,
- MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
- if (mapped == MAP_FAILED)
- return 0;
- if (mapped != address)
- goto out;
- }
-
- /*
- * Now, is it writeable? If so, then we're in user address
- * space. If not, then try mprotecting it and try the write
- * again.
- */
- if (setjmp(buf) == 0) {
- *address = n;
- ok = 1;
- goto out;
- } else if (mprotect(address, UM_KERN_PAGE_SIZE,
- PROT_READ | PROT_WRITE) != 0)
- goto out;
-
- if (setjmp(buf) == 0) {
- *address = n;
- ok = 1;
- }
-
- out:
- if (mapped != NULL)
- munmap(mapped, UM_KERN_PAGE_SIZE);
- return ok;
-}
-
-unsigned long os_get_top_address(void)
-{
- struct sigaction sa, old;
- unsigned long bottom = 0;
- /*
- * A 32-bit UML on a 64-bit host gets confused about the VDSO at
- * 0xffffe000. It is mapped, is readable, can be reprotected writeable
- * and written. However, exec discovers later that it can't be
- * unmapped. So, just set the highest address to be checked to just
- * below it. This might waste some address space on 4G/4G 32-bit
- * hosts, but shouldn't hurt otherwise.
- */
- unsigned long top = 0xffffd000 >> UM_KERN_PAGE_SHIFT;
- unsigned long test, original;
-
- printf("Locating the bottom of the address space ... ");
- fflush(stdout);
-
- /*
- * We're going to be longjmping out of the signal handler, so
- * SA_DEFER needs to be set.
- */
- sa.sa_handler = segfault;
- sigemptyset(&sa.sa_mask);
- sa.sa_flags = SA_NODEFER;
- if (sigaction(SIGSEGV, &sa, &old)) {
- perror("os_get_top_address");
- exit(1);
- }
-
- /* Manually scan the address space, bottom-up, until we find
- * the first valid page (or run out of them).
- */
- for (bottom = 0; bottom < top; bottom++) {
- if (page_ok(bottom))
- break;
- }
-
- /* If we've got this far, we ran out of pages. */
- if (bottom == top) {
- fprintf(stderr, "Unable to determine bottom of address "
- "space.\n");
- exit(1);
- }
-
- printf("0x%lx\n", bottom << UM_KERN_PAGE_SHIFT);
- printf("Locating the top of the address space ... ");
- fflush(stdout);
-
- original = bottom;
-
- /* This could happen with a 4G/4G split */
- if (page_ok(top))
- goto out;
-
- do {
- test = bottom + (top - bottom) / 2;
- if (page_ok(test))
- bottom = test;
- else
- top = test;
- } while (top - bottom > 1);
-
-out:
- /* Restore the old SIGSEGV handling */
- if (sigaction(SIGSEGV, &old, NULL)) {
- perror("os_get_top_address");
- exit(1);
- }
- top <<= UM_KERN_PAGE_SHIFT;
- printf("0x%lx\n", top);
-
- return top;
-}
-
-#else
-
-unsigned long os_get_top_address(void)
-{
- /* The old value of CONFIG_TOP_ADDR */
- return 0x7fc0002000;
-}
-
-#endif
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 09/10] um: clear all memory in new userspace processes
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (7 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 08/10] um: Discover host_task_size from envp Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 10/10] um: Switch to 4 level page tables on 64 bit Benjamin Berg
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
With the change to use execve() we can now safely clear the memory up to
STUB_START as rseq will not be trying to use memory in that region. Also,
on 64 bit the previous changes should mean that there is no usable
memory range above the stub.
Make the change and remove the comment as it is not needed anymore.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/kernel/skas/mmu.c | 25 ++-----------------------
1 file changed, 2 insertions(+), 23 deletions(-)
diff --git a/arch/um/kernel/skas/mmu.c b/arch/um/kernel/skas/mmu.c
index 886ed5e65674..d3fb506d5bd6 100644
--- a/arch/um/kernel/skas/mmu.c
+++ b/arch/um/kernel/skas/mmu.c
@@ -40,29 +40,8 @@ int init_new_context(struct task_struct *task, struct mm_struct *mm)
goto out_free;
}
- /*
- * Ensure the new MM is clean and nothing unwanted is mapped.
- *
- * TODO: We should clear the memory up to STUB_START to ensure there is
- * nothing mapped there, i.e. we (currently) have:
- *
- * |- user memory -|- unused -|- stub -|- unused -|
- * ^ TASK_SIZE ^ STUB_START
- *
- * Meaning we have two unused areas where we may still have valid
- * mappings from our internal clone(). That isn't really a problem as
- * userspace is not going to access them, but it is definitely not
- * correct.
- *
- * However, we are "lucky" and if rseq is configured, then on 32 bit
- * it will fall into the first empty range while on 64 bit it is going
- * to use an anonymous mapping in the second range. As such, things
- * continue to work for now as long as we don't start unmapping these
- * areas.
- *
- * Change this to STUB_START once we have a clean userspace.
- */
- unmap(new_id, 0, TASK_SIZE);
+ /* Ensure the new MM is clean and nothing unwanted is mapped */
+ unmap(new_id, 0, STUB_START);
return 0;
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v9 10/10] um: Switch to 4 level page tables on 64 bit
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
` (8 preceding siblings ...)
2024-09-19 12:45 ` [PATCH v9 09/10] um: clear all memory in new userspace processes Benjamin Berg
@ 2024-09-19 12:45 ` Benjamin Berg
9 siblings, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-09-19 12:45 UTC (permalink / raw)
To: linux-um; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
The larger memory space is useful to support more applications inside
UML. One example for this is ASAN instrumentation of userspace
applications which requires addresses that would otherwise not be
available.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
v9:
- Drop support for 3 level page tables
v7:
- Reword options and fix documentation of x86-64 default
v2:
- Do not hide option behind the EXPERT flag
- Fix typo in new "Two-level pagetables" option
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
arch/um/Kconfig | 4 +-
arch/um/include/asm/page.h | 14 +++++--
arch/um/include/asm/pgalloc.h | 11 ++++-
.../{pgtable-3level.h => pgtable-4level.h} | 40 ++++++++++++++++---
arch/um/include/asm/pgtable.h | 8 ++--
arch/um/kernel/mem.c | 17 +++++++-
arch/x86/um/Kconfig | 3 --
7 files changed, 78 insertions(+), 19 deletions(-)
rename arch/um/include/asm/{pgtable-3level.h => pgtable-4level.h} (66%)
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index c89575d05021..48db1c99bd46 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -209,8 +209,8 @@ config MMAPPER
config PGTABLE_LEVELS
int
- default 3 if 3_LEVEL_PGTABLES
- default 2
+ default 4 if 64BIT
+ default 2 if !64BIT
config UML_TIME_TRAVEL_SUPPORT
bool
diff --git a/arch/um/include/asm/page.h b/arch/um/include/asm/page.h
index 8d2ac5e86cf5..f0ad80fc8c10 100644
--- a/arch/um/include/asm/page.h
+++ b/arch/um/include/asm/page.h
@@ -35,14 +35,22 @@ struct page;
typedef struct { unsigned long pte; } pte_t;
typedef struct { unsigned long pgd; } pgd_t;
-#ifdef CONFIG_3_LEVEL_PGTABLES
+#if CONFIG_PGTABLE_LEVELS > 2
+
typedef struct { unsigned long pmd; } pmd_t;
#define pmd_val(x) ((x).pmd)
#define __pmd(x) ((pmd_t) { (x) } )
-#endif
-#define pte_val(x) ((x).pte)
+#if CONFIG_PGTABLE_LEVELS > 3
+typedef struct { unsigned long pud; } pud_t;
+#define pud_val(x) ((x).pud)
+#define __pud(x) ((pud_t) { (x) } )
+
+#endif /* CONFIG_PGTABLE_LEVELS > 3 */
+#endif /* CONFIG_PGTABLE_LEVELS > 2 */
+
+#define pte_val(x) ((x).pte)
#define pte_get_bits(p, bits) ((p).pte & (bits))
#define pte_set_bits(p, bits) ((p).pte |= (bits))
diff --git a/arch/um/include/asm/pgalloc.h b/arch/um/include/asm/pgalloc.h
index de5e31c64793..04fb4e6969a4 100644
--- a/arch/um/include/asm/pgalloc.h
+++ b/arch/um/include/asm/pgalloc.h
@@ -31,7 +31,7 @@ do { \
tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte))); \
} while (0)
-#ifdef CONFIG_3_LEVEL_PGTABLES
+#if CONFIG_PGTABLE_LEVELS > 2
#define __pmd_free_tlb(tlb, pmd, address) \
do { \
@@ -39,6 +39,15 @@ do { \
tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pmd)); \
} while (0)
+#if CONFIG_PGTABLE_LEVELS > 3
+
+#define __pud_free_tlb(tlb, pud, address) \
+do { \
+ pagetable_pud_dtor(virt_to_ptdesc(pud)); \
+ tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pud)); \
+} while (0)
+
+#endif
#endif
#endif
diff --git a/arch/um/include/asm/pgtable-3level.h b/arch/um/include/asm/pgtable-4level.h
similarity index 66%
rename from arch/um/include/asm/pgtable-3level.h
rename to arch/um/include/asm/pgtable-4level.h
index 3504a92dc485..f912fcc16b7a 100644
--- a/arch/um/include/asm/pgtable-3level.h
+++ b/arch/um/include/asm/pgtable-4level.h
@@ -4,17 +4,25 @@
* Derived from include/asm-i386/pgtable.h
*/
-#ifndef __UM_PGTABLE_3LEVEL_H
-#define __UM_PGTABLE_3LEVEL_H
+#ifndef __UM_PGTABLE_4LEVEL_H
+#define __UM_PGTABLE_4LEVEL_H
-#include <asm-generic/pgtable-nopud.h>
+#include <asm-generic/pgtable-nop4d.h>
-/* PGDIR_SHIFT determines what a third-level page table entry can map */
+/* PGDIR_SHIFT determines what a fourth-level page table entry can map */
-#define PGDIR_SHIFT 30
+#define PGDIR_SHIFT 39
#define PGDIR_SIZE (1UL << PGDIR_SHIFT)
#define PGDIR_MASK (~(PGDIR_SIZE-1))
+/* PUD_SHIFT determines the size of the area a third-level page table can
+ * map
+ */
+
+#define PUD_SHIFT 30
+#define PUD_SIZE (1UL << PUD_SHIFT)
+#define PUD_MASK (~(PUD_SIZE-1))
+
/* PMD_SHIFT determines the size of the area a second-level page table can
* map
*/
@@ -29,6 +37,7 @@
#define PTRS_PER_PTE 512
#define PTRS_PER_PMD 512
+#define PTRS_PER_PUD 512
#define PTRS_PER_PGD 512
#define USER_PTRS_PER_PGD ((TASK_SIZE + (PGDIR_SIZE - 1)) / PGDIR_SIZE)
@@ -39,6 +48,9 @@
#define pmd_ERROR(e) \
printk("%s:%d: bad pmd %p(%016lx).\n", __FILE__, __LINE__, &(e), \
pmd_val(e))
+#define pud_ERROR(e) \
+ printk("%s:%d: bad pud %p(%016lx).\n", __FILE__, __LINE__, &(e), \
+ pud_val(e))
#define pgd_ERROR(e) \
printk("%s:%d: bad pgd %p(%016lx).\n", __FILE__, __LINE__, &(e), \
pgd_val(e))
@@ -51,6 +63,15 @@
#define set_pud(pudptr, pudval) (*(pudptr) = (pudval))
+#define p4d_none(x) (!(p4d_val(x) & ~_PAGE_NEWPAGE))
+#define p4d_bad(x) ((p4d_val(x) & (~PAGE_MASK & ~_PAGE_USER)) != _KERNPG_TABLE)
+#define p4d_present(x) (p4d_val(x) & _PAGE_PRESENT)
+#define p4d_populate(mm, p4d, pud) \
+ set_p4d(p4d, __p4d(_PAGE_TABLE + __pa(pud)))
+
+#define set_p4d(p4dptr, p4dval) (*(p4dptr) = (p4dval))
+
+
static inline int pgd_newpage(pgd_t pgd)
{
return(pgd_val(pgd) & _PAGE_NEWPAGE);
@@ -65,9 +86,17 @@ static inline void pud_clear (pud_t *pud)
set_pud(pud, __pud(_PAGE_NEWPAGE));
}
+static inline void p4d_clear (p4d_t *p4d)
+{
+ set_p4d(p4d, __p4d(_PAGE_NEWPAGE));
+}
+
#define pud_page(pud) phys_to_page(pud_val(pud) & PAGE_MASK)
#define pud_pgtable(pud) ((pmd_t *) __va(pud_val(pud) & PAGE_MASK))
+#define p4d_page(p4d) phys_to_page(p4d_val(p4d) & PAGE_MASK)
+#define p4d_pgtable(p4d) ((pud_t *) __va(p4d_val(p4d) & PAGE_MASK))
+
static inline unsigned long pte_pfn(pte_t pte)
{
return phys_to_pfn(pte_val(pte));
@@ -88,4 +117,3 @@ static inline pmd_t pfn_pmd(unsigned long page_nr, pgprot_t pgprot)
}
#endif
-
diff --git a/arch/um/include/asm/pgtable.h b/arch/um/include/asm/pgtable.h
index 83373c9963e7..bd7a9593705f 100644
--- a/arch/um/include/asm/pgtable.h
+++ b/arch/um/include/asm/pgtable.h
@@ -24,10 +24,12 @@
/* We borrow bit 10 to store the exclusive marker in swap PTEs. */
#define _PAGE_SWP_EXCLUSIVE 0x400
-#ifdef CONFIG_3_LEVEL_PGTABLES
-#include <asm/pgtable-3level.h>
-#else
+#if CONFIG_PGTABLE_LEVELS == 4
+#include <asm/pgtable-4level.h>
+#elif CONFIG_PGTABLE_LEVELS == 2
#include <asm/pgtable-2level.h>
+#else
+#error "Unsupported number of page table levels"
#endif
extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c
index a5b4fe2ad931..e7c262265c31 100644
--- a/arch/um/kernel/mem.c
+++ b/arch/um/kernel/mem.c
@@ -98,7 +98,7 @@ static void __init one_page_table_init(pmd_t *pmd)
static void __init one_md_table_init(pud_t *pud)
{
-#ifdef CONFIG_3_LEVEL_PGTABLES
+#if CONFIG_PGTABLE_LEVELS > 2
pmd_t *pmd_table = (pmd_t *) memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
if (!pmd_table)
panic("%s: Failed to allocate %lu bytes align=%lx\n",
@@ -109,6 +109,19 @@ static void __init one_md_table_init(pud_t *pud)
#endif
}
+static void __init one_ud_table_init(p4d_t *p4d)
+{
+#if CONFIG_PGTABLE_LEVELS > 3
+ pud_t *pud_table = (pud_t *) memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
+ if (!pud_table)
+ panic("%s: Failed to allocate %lu bytes align=%lx\n",
+ __func__, PAGE_SIZE, PAGE_SIZE);
+
+ set_p4d(p4d, __p4d(_KERNPG_TABLE + (unsigned long) __pa(pud_table)));
+ BUG_ON(pud_table != pud_offset(p4d, 0));
+#endif
+}
+
static void __init fixrange_init(unsigned long start, unsigned long end,
pgd_t *pgd_base)
{
@@ -126,6 +139,8 @@ static void __init fixrange_init(unsigned long start, unsigned long end,
for ( ; (i < PTRS_PER_PGD) && (vaddr < end); pgd++, i++) {
p4d = p4d_offset(pgd, vaddr);
+ if (p4d_none(*p4d))
+ one_ud_table_init(p4d);
pud = pud_offset(p4d, vaddr);
if (pud_none(*pud))
one_md_table_init(pud);
diff --git a/arch/x86/um/Kconfig b/arch/x86/um/Kconfig
index 7d16baba8f5f..41bc9221c454 100644
--- a/arch/x86/um/Kconfig
+++ b/arch/x86/um/Kconfig
@@ -28,9 +28,6 @@ config X86_64
def_bool 64BIT
select MODULES_USE_ELF_RELA
-config 3_LEVEL_PGTABLES
- def_bool 64BIT
-
config ARCH_HAS_SC_SIGNALS
def_bool !64BIT
--
2.46.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-09-19 12:45 ` [PATCH v9 02/10] um: use execveat to create userspace MMs Benjamin Berg
@ 2024-10-16 2:34 ` Nathan Chancellor
2024-10-16 8:19 ` Berg, Benjamin
2024-10-17 7:17 ` David Gow
1 sibling, 1 reply; 18+ messages in thread
From: Nathan Chancellor @ 2024-10-16 2:34 UTC (permalink / raw)
To: Benjamin Berg; +Cc: linux-um, Benjamin Berg, llvm
Hi Benjamin,
On Thu, Sep 19, 2024 at 02:45:03PM +0200, Benjamin Berg wrote:
...
> diff --git a/arch/um/kernel/skas/Makefile b/arch/um/kernel/skas/Makefile
> index 6f86d53e3d69..fbb61968055f 100644
> --- a/arch/um/kernel/skas/Makefile
> +++ b/arch/um/kernel/skas/Makefile
> @@ -3,14 +3,43 @@
> # Copyright (C) 2002 - 2007 Jeff Dike (jdike@{addtoit,linux.intel}.com)
> #
>
> -obj-y := stub.o mmu.o process.o syscall.o uaccess.o
> +obj-y := stub.o mmu.o process.o syscall.o uaccess.o \
> + stub_exe_embed.o
> +
> +# Stub executable
> +
> +stub_exe_objs-y := stub_exe.o
> +
> +stub_exe_objs := $(foreach F,$(stub_exe_objs-y),$(obj)/$F)
> +
> +# Object file containing the ELF executable
> +$(obj)/stub_exe_embed.o: $(src)/stub_exe_embed.S $(obj)/stub_exe
> +
> +$(obj)/stub_exe.dbg: $(stub_exe_objs) FORCE
> + $(call if_changed,stub_exe)
> +
> +$(obj)/stub_exe: OBJCOPYFLAGS := -S
> +$(obj)/stub_exe: $(obj)/stub_exe.dbg FORCE
> + $(call if_changed,objcopy)
> +
> +quiet_cmd_stub_exe = STUB_EXE $@
> + cmd_stub_exe = $(CC) -nostdlib -o $@ \
> + $(KBUILD_CFLAGS) $(STUB_EXE_LDFLAGS) \
> + $(filter %.o,$^)
> +
> +STUB_EXE_LDFLAGS = -n -static
After this change in -next as commit 32e8eaf263d9 ("um: use execveat to
create userspace MMs"), I am seeing an error when building ARCH=um
defconfig with clang.
$ make -skj"$(nproc)" ARCH=um LLVM=1 mrproper defconfig linux
clang: error: unknown argument: '-n'
make[7]: *** [arch/um/kernel/skas/Makefile:19: arch/um/kernel/skas/stub_exe.dbg] Error 1
...
I have looked through the GCC manual and options index but I do not see
what '-n' is. Was this intended to be passed to the linker like -Wl,-n
or was there some other option that this should be? If I try dropping
it, I end up running into another issue, which could be entirely
unrelated.
$ make -skj"$(nproc)" ARCH=um LLVM=1 mrproper defconfig arch/um/kernel/skas/
/usr/bin/ld: arch/um/kernel/skas/stub_exe.o: in function `_start':
arch/um/kernel/skas/stub_exe.c:83:(.ltext+0x15): undefined reference to `memset'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[7]: *** [arch/um/kernel/skas/Makefile:19: arch/um/kernel/skas/stub_exe.dbg] Error 1
Cheers,
Nathan
> +targets += stub_exe.dbg stub_exe $(stub_exe_objs-y)
> +
> +# end
>
> # stub.o is in the stub, so it can't be built with profiling
> # GCC hardened also auto-enables -fpic, but we need %ebx so it can't work ->
> # disable it
>
> CFLAGS_stub.o := $(CFLAGS_NO_HARDENING)
> -UNPROFILE_OBJS := stub.o
> +CFLAGS_stub_exe.o := $(CFLAGS_NO_HARDENING)
> +UNPROFILE_OBJS := stub.o stub_exe.o
> KCOV_INSTRUMENT := n
>
> include $(srctree)/arch/um/scripts/Makefile.rules
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-10-16 2:34 ` Nathan Chancellor
@ 2024-10-16 8:19 ` Berg, Benjamin
0 siblings, 0 replies; 18+ messages in thread
From: Berg, Benjamin @ 2024-10-16 8:19 UTC (permalink / raw)
To: nathan@kernel.org; +Cc: llvm@lists.linux.dev, linux-um@lists.infradead.org
Hi Nathan,
On Tue, 2024-10-15 at 19:34 -0700, Nathan Chancellor wrote:
> On Thu, Sep 19, 2024 at 02:45:03PM +0200, Benjamin Berg wrote:
> ...
> > diff --git a/arch/um/kernel/skas/Makefile
> > b/arch/um/kernel/skas/Makefile
> > index 6f86d53e3d69..fbb61968055f 100644
> > --- a/arch/um/kernel/skas/Makefile
> > +++ b/arch/um/kernel/skas/Makefile
> > @@ -3,14 +3,43 @@
> > # Copyright (C) 2002 - 2007 Jeff Dike
> > (jdike@{addtoit,linux.intel}.com)
> > #
> >
> > -obj-y := stub.o mmu.o process.o syscall.o uaccess.o
> > +obj-y := stub.o mmu.o process.o syscall.o uaccess.o \
> > + stub_exe_embed.o
> > +
> > +# Stub executable
> > +
> > +stub_exe_objs-y := stub_exe.o
> > +
> > +stub_exe_objs := $(foreach F,$(stub_exe_objs-y),$(obj)/$F)
> > +
> > +# Object file containing the ELF executable
> > +$(obj)/stub_exe_embed.o: $(src)/stub_exe_embed.S $(obj)/stub_exe
> > +
> > +$(obj)/stub_exe.dbg: $(stub_exe_objs) FORCE
> > + $(call if_changed,stub_exe)
> > +
> > +$(obj)/stub_exe: OBJCOPYFLAGS := -S
> > +$(obj)/stub_exe: $(obj)/stub_exe.dbg FORCE
> > + $(call if_changed,objcopy)
> > +
> > +quiet_cmd_stub_exe = STUB_EXE $@
> > + cmd_stub_exe = $(CC) -nostdlib -o $@ \
> > + $(KBUILD_CFLAGS) $(STUB_EXE_LDFLAGS) \
> > + $(filter %.o,$^)
> > +
> > +STUB_EXE_LDFLAGS = -n -static
>
> After this change in -next as commit 32e8eaf263d9 ("um: use execveat
> to
> create userspace MMs"), I am seeing an error when building ARCH=um
> defconfig with clang.
>
> $ make -skj"$(nproc)" ARCH=um LLVM=1 mrproper defconfig linux
> clang: error: unknown argument: '-n'
> make[7]: *** [arch/um/kernel/skas/Makefile:19:
> arch/um/kernel/skas/stub_exe.dbg] Error 1
> ...
>
> I have looked through the GCC manual and options index but I do not
> see
> what '-n' is. Was this intended to be passed to the linker like -Wl,-
> n
> or was there some other option that this should be? If I try dropping
> it, I end up running into another issue, which could be entirely
> unrelated.
Yes, you are right, it was supposed to be passed to the linker and the
only point is to make the elf binary a bit smaller that will be
embedded (the --nmagic ld option).
Anything works for me. We can change it to -Wl,-n if it works or even
just drop it entirely.
Benjamin
> $ make -skj"$(nproc)" ARCH=um LLVM=1 mrproper defconfig
> arch/um/kernel/skas/
> /usr/bin/ld: arch/um/kernel/skas/stub_exe.o: in function `_start':
> arch/um/kernel/skas/stub_exe.c:83:(.ltext+0x15): undefined
> reference to `memset'
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> make[7]: *** [arch/um/kernel/skas/Makefile:19:
> arch/um/kernel/skas/stub_exe.dbg] Error 1
>
> Cheers,
> Nathan
>
> > +targets += stub_exe.dbg stub_exe $(stub_exe_objs-y)
> > +
> > +# end
> >
> > # stub.o is in the stub, so it can't be built with profiling
> > # GCC hardened also auto-enables -fpic, but we need %ebx so it
> > can't work ->
> > # disable it
> >
> > CFLAGS_stub.o := $(CFLAGS_NO_HARDENING)
> > -UNPROFILE_OBJS := stub.o
> > +CFLAGS_stub_exe.o := $(CFLAGS_NO_HARDENING)
> > +UNPROFILE_OBJS := stub.o stub_exe.o
> > KCOV_INSTRUMENT := n
> >
> > include $(srctree)/arch/um/scripts/Makefile.rules
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-09-19 12:45 ` [PATCH v9 02/10] um: use execveat to create userspace MMs Benjamin Berg
2024-10-16 2:34 ` Nathan Chancellor
@ 2024-10-17 7:17 ` David Gow
2024-10-17 8:18 ` Johannes Berg
2024-10-17 9:01 ` Benjamin Berg
1 sibling, 2 replies; 18+ messages in thread
From: David Gow @ 2024-10-17 7:17 UTC (permalink / raw)
To: Benjamin Berg; +Cc: linux-um, Benjamin Berg, KUnit Development, Kees Cook
[-- Attachment #1: Type: text/plain, Size: 19996 bytes --]
On Thu, 19 Sept 2024 at 20:45, Benjamin Berg <benjamin@sipsolutions.net> wrote:
>
> From: Benjamin Berg <benjamin.berg@intel.com>
>
> Using clone will not undo features that have been enabled by libc. An
> example of this already happening is rseq, which could cause the kernel
> to read/write memory of the userspace process. In the future the
> standard library might also use mseal by default to protect itself,
> which would also thwart our attempts at unmapping everything.
>
> Solve all this by taking a step back and doing an execve into a tiny
> static binary that sets up the minimal environment required for the
> stub without using any standard library. That way we have a clean
> execution environment that is fully under the control of UML.
>
> Note that this changes things a bit as the FDs are not anymore shared
> with the kernel. Instead, we explicitly share the FDs for the physical
> memory and all existing iomem regions. Doing this is fine, as iomem
> regions cannot be added at runtime.
>
> Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
>
> ---
>
> v9:
> - Avoid clash of libc __close_range with the kernel version,
> thanks to Tiwei Bie for pointing this out
>
> v8:
> - Make changes suggested by Johannes Berg
>
> v7:
> - Rename stub_elf to stub_exe
> - Move into architecture independent directory
> - Fix 32 bit issues
> - Improve tempfile logic
> - Other cleanups
>
> v6:
> - Apply fixes pointed out by Tiwei Bie
> - Add temporary file fallback as memfd is not always supported
>
> Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> ---
It turns out that this breaks the KUnit user alloc helpers on x86_64,
at least on my machine.
This can be reproduced with:
./tools/testing/kunit/kunit.py run usercopy
Though the 32-bit version works:
./tools/testing/kunit/kunit.py run usercopy --kconfig_add CONFIG_64BIT=n
The error we're getting is:
start_userspace : expected SIGSTOP, got status = 139
Could not create userspace mm
This basically is the result of the stub_exe segfaulting very early on
in its execution.
It seems that this is due to the stack being misaligned, and so the
generated SSE instructions are faulting. The workarounds I've tested
here include:
a) Build the stub with -mno-sse
b) Decorate real_init() with __attribute__((force_align_arg_pointer))
c) Decorate __start() with __attribute__((naked))
The last one seems to validate my theory as to why this is occurring:
__start's prologue is misaligning the stack, as __start is not
actually _called_ from anything, so there's no 8-byte misalignment to
hold the return address.
If this makes sense, I'll send a patch out with whichever the
preferred fix(es) are. My guess is that (c) is the "proper" fix,
though I'd not _miss_ SSE if we chose to disable it for the handful of
instructions here anyway.
Cheers,
-- David
> arch/um/Makefile | 3 +-
> arch/um/include/shared/skas/stub-data.h | 11 ++
> arch/um/kernel/skas/.gitignore | 2 +
> arch/um/kernel/skas/Makefile | 33 ++++-
> arch/um/kernel/skas/stub_exe.c | 88 ++++++++++++
> arch/um/kernel/skas/stub_exe_embed.S | 11 ++
> arch/um/os-Linux/mem.c | 2 +-
> arch/um/os-Linux/skas/process.c | 181 ++++++++++++++++--------
> 8 files changed, 271 insertions(+), 60 deletions(-)
> create mode 100644 arch/um/kernel/skas/.gitignore
> create mode 100644 arch/um/kernel/skas/stub_exe.c
> create mode 100644 arch/um/kernel/skas/stub_exe_embed.S
>
> diff --git a/arch/um/Makefile b/arch/um/Makefile
> index 00b63bac5eff..31e367e8ab4d 100644
> --- a/arch/um/Makefile
> +++ b/arch/um/Makefile
> @@ -61,7 +61,8 @@ KBUILD_CFLAGS += $(CFLAGS) $(CFLAGS-y) -D__arch_um__ \
> $(ARCH_INCLUDE) $(MODE_INCLUDE) -Dvmap=kernel_vmap \
> -Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
> -Din6addr_loopback=kernel_in6addr_loopback \
> - -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr
> + -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr \
> + -D__close_range=kernel__close_range
>
> KBUILD_RUSTFLAGS += -Crelocation-model=pie
>
> diff --git a/arch/um/include/shared/skas/stub-data.h b/arch/um/include/shared/skas/stub-data.h
> index 2b6b44759dfa..3fbdda727373 100644
> --- a/arch/um/include/shared/skas/stub-data.h
> +++ b/arch/um/include/shared/skas/stub-data.h
> @@ -12,6 +12,17 @@
> #include <as-layout.h>
> #include <sysdep/tls.h>
>
> +struct stub_init_data {
> + unsigned long stub_start;
> +
> + int stub_code_fd;
> + unsigned long stub_code_offset;
> + int stub_data_fd;
> + unsigned long stub_data_offset;
> +
> + unsigned long segv_handler;
> +};
> +
> #define STUB_NEXT_SYSCALL(s) \
> ((struct stub_syscall *) (((unsigned long) s) + (s)->cmd_len))
>
> diff --git a/arch/um/kernel/skas/.gitignore b/arch/um/kernel/skas/.gitignore
> new file mode 100644
> index 000000000000..c3409ced0f38
> --- /dev/null
> +++ b/arch/um/kernel/skas/.gitignore
> @@ -0,0 +1,2 @@
> +stub_exe
> +stub_exe.dbg
> diff --git a/arch/um/kernel/skas/Makefile b/arch/um/kernel/skas/Makefile
> index 6f86d53e3d69..fbb61968055f 100644
> --- a/arch/um/kernel/skas/Makefile
> +++ b/arch/um/kernel/skas/Makefile
> @@ -3,14 +3,43 @@
> # Copyright (C) 2002 - 2007 Jeff Dike (jdike@{addtoit,linux.intel}.com)
> #
>
> -obj-y := stub.o mmu.o process.o syscall.o uaccess.o
> +obj-y := stub.o mmu.o process.o syscall.o uaccess.o \
> + stub_exe_embed.o
> +
> +# Stub executable
> +
> +stub_exe_objs-y := stub_exe.o
> +
> +stub_exe_objs := $(foreach F,$(stub_exe_objs-y),$(obj)/$F)
> +
> +# Object file containing the ELF executable
> +$(obj)/stub_exe_embed.o: $(src)/stub_exe_embed.S $(obj)/stub_exe
> +
> +$(obj)/stub_exe.dbg: $(stub_exe_objs) FORCE
> + $(call if_changed,stub_exe)
> +
> +$(obj)/stub_exe: OBJCOPYFLAGS := -S
> +$(obj)/stub_exe: $(obj)/stub_exe.dbg FORCE
> + $(call if_changed,objcopy)
> +
> +quiet_cmd_stub_exe = STUB_EXE $@
> + cmd_stub_exe = $(CC) -nostdlib -o $@ \
> + $(KBUILD_CFLAGS) $(STUB_EXE_LDFLAGS) \
> + $(filter %.o,$^)
> +
> +STUB_EXE_LDFLAGS = -n -static
> +
> +targets += stub_exe.dbg stub_exe $(stub_exe_objs-y)
> +
> +# end
>
> # stub.o is in the stub, so it can't be built with profiling
> # GCC hardened also auto-enables -fpic, but we need %ebx so it can't work ->
> # disable it
>
> CFLAGS_stub.o := $(CFLAGS_NO_HARDENING)
> -UNPROFILE_OBJS := stub.o
> +CFLAGS_stub_exe.o := $(CFLAGS_NO_HARDENING)
> +UNPROFILE_OBJS := stub.o stub_exe.o
> KCOV_INSTRUMENT := n
>
> include $(srctree)/arch/um/scripts/Makefile.rules
> diff --git a/arch/um/kernel/skas/stub_exe.c b/arch/um/kernel/skas/stub_exe.c
> new file mode 100644
> index 000000000000..bc6ba2e4d805
> --- /dev/null
> +++ b/arch/um/kernel/skas/stub_exe.c
> @@ -0,0 +1,88 @@
> +#include <sys/ptrace.h>
> +#include <sys/prctl.h>
> +#include <asm/unistd.h>
> +#include <sysdep/stub.h>
> +#include <stub-data.h>
> +
> +void _start(void);
> +
> +noinline static void real_init(void)
> +{
> + struct stub_init_data init_data;
> + unsigned long res;
> + struct {
> + void *ss_sp;
> + int ss_flags;
> + size_t ss_size;
> + } stack = {
> + .ss_size = STUB_DATA_PAGES * UM_KERN_PAGE_SIZE,
> + };
> + struct {
> + void *sa_handler_;
> + unsigned long sa_flags;
> + void *sa_restorer;
> + unsigned long long sa_mask;
> + } sa = {
> + /* Need to set SA_RESTORER (but the handler never returns) */
> + .sa_flags = SA_ONSTACK | SA_NODEFER | SA_SIGINFO | 0x04000000,
> + /* no need to mask any signals */
> + .sa_mask = 0,
> + };
> +
> + /* set a nice name */
> + stub_syscall2(__NR_prctl, PR_SET_NAME, (unsigned long)"uml-userspace");
> +
> + /* read information from STDIN and close it */
> + res = stub_syscall3(__NR_read, 0,
> + (unsigned long)&init_data, sizeof(init_data));
> + if (res != sizeof(init_data))
> + stub_syscall1(__NR_exit, 10);
> +
> + stub_syscall1(__NR_close, 0);
> +
> + /* map stub code + data */
> + res = stub_syscall6(STUB_MMAP_NR,
> + init_data.stub_start, UM_KERN_PAGE_SIZE,
> + PROT_READ | PROT_EXEC, MAP_FIXED | MAP_SHARED,
> + init_data.stub_code_fd, init_data.stub_code_offset);
> + if (res != init_data.stub_start)
> + stub_syscall1(__NR_exit, 11);
> +
> + res = stub_syscall6(STUB_MMAP_NR,
> + init_data.stub_start + UM_KERN_PAGE_SIZE,
> + STUB_DATA_PAGES * UM_KERN_PAGE_SIZE,
> + PROT_READ | PROT_WRITE, MAP_FIXED | MAP_SHARED,
> + init_data.stub_data_fd, init_data.stub_data_offset);
> + if (res != init_data.stub_start + UM_KERN_PAGE_SIZE)
> + stub_syscall1(__NR_exit, 12);
> +
> + /* setup signal stack inside stub data */
> + stack.ss_sp = (void *)init_data.stub_start + UM_KERN_PAGE_SIZE;
> + stub_syscall2(__NR_sigaltstack, (unsigned long)&stack, 0);
> +
> + /* register SIGSEGV handler */
> + sa.sa_handler_ = (void *) init_data.segv_handler;
> + res = stub_syscall4(__NR_rt_sigaction, SIGSEGV, (unsigned long)&sa, 0,
> + sizeof(sa.sa_mask));
> + if (res != 0)
> + stub_syscall1(__NR_exit, 13);
> +
> + stub_syscall4(__NR_ptrace, PTRACE_TRACEME, 0, 0, 0);
> +
> + stub_syscall2(__NR_kill, stub_syscall0(__NR_getpid), SIGSTOP);
> +
> + stub_syscall1(__NR_exit, 14);
> +
> + __builtin_unreachable();
> +}
> +
> +void _start(void)
> +{
> + char *alloc;
> +
> + /* Make enough space for the stub (including space for alignment) */
> + alloc = __builtin_alloca((1 + 2 * STUB_DATA_PAGES - 1) * UM_KERN_PAGE_SIZE);
> + asm volatile("" : "+r,m"(alloc) : : "memory");
> +
> + real_init();
> +}
> diff --git a/arch/um/kernel/skas/stub_exe_embed.S b/arch/um/kernel/skas/stub_exe_embed.S
> new file mode 100644
> index 000000000000..6d8914fbe8f1
> --- /dev/null
> +++ b/arch/um/kernel/skas/stub_exe_embed.S
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <linux/init.h>
> +#include <linux/linkage.h>
> +
> +__INITDATA
> +
> +SYM_DATA_START(stub_exe_start)
> + .incbin "arch/um/kernel/skas/stub_exe"
> +SYM_DATA_END_LABEL(stub_exe_start, SYM_L_GLOBAL, stub_exe_end)
> +
> +__FINIT
> diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c
> index cf44d386f23c..857e3deab293 100644
> --- a/arch/um/os-Linux/mem.c
> +++ b/arch/um/os-Linux/mem.c
> @@ -42,7 +42,7 @@ void kasan_map_memory(void *start, size_t len)
> }
>
> /* Set by make_tempfile() during early boot. */
> -static char *tempdir = NULL;
> +char *tempdir = NULL;
>
> /* Check if dir is on tmpfs. Return 0 if yes, -1 if no or error. */
> static int __init check_tmpfs(const char *dir)
> diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
> index b6f656bcffb1..2caaa9e85752 100644
> --- a/arch/um/os-Linux/skas/process.c
> +++ b/arch/um/os-Linux/skas/process.c
> @@ -10,8 +10,11 @@
> #include <sched.h>
> #include <errno.h>
> #include <string.h>
> +#include <fcntl.h>
> +#include <mem_user.h>
> #include <sys/mman.h>
> #include <sys/wait.h>
> +#include <sys/stat.h>
> #include <asm/unistd.h>
> #include <as-layout.h>
> #include <init.h>
> @@ -189,69 +192,135 @@ static void handle_trap(int pid, struct uml_pt_regs *regs)
>
> extern char __syscall_stub_start[];
>
> -/**
> - * userspace_tramp() - userspace trampoline
> - * @stack: pointer to the new userspace stack page
> - *
> - * The userspace trampoline is used to setup a new userspace process in start_userspace() after it was clone()'ed.
> - * This function will run on a temporary stack page.
> - * It ptrace()'es itself, then
> - * Two pages are mapped into the userspace address space:
> - * - STUB_CODE (with EXEC), which contains the skas stub code
> - * - STUB_DATA (with R/W), which contains a data page that is used to transfer certain data between the UML userspace process and the UML kernel.
> - * Also for the userspace process a SIGSEGV handler is installed to catch pagefaults in the userspace process.
> - * And last the process stops itself to give control to the UML kernel for this userspace process.
> - *
> - * Return: Always zero, otherwise the current userspace process is ended with non null exit() call
> - */
> +static int stub_exe_fd;
> +
> static int userspace_tramp(void *stack)
> {
> - struct sigaction sa;
> - void *addr;
> - int fd;
> + char *const argv[] = { "uml-userspace", NULL };
> + int pipe_fds[2];
> unsigned long long offset;
> - unsigned long segv_handler = STUB_CODE +
> - (unsigned long) stub_segv_handler -
> - (unsigned long) __syscall_stub_start;
> -
> - ptrace(PTRACE_TRACEME, 0, 0, 0);
> -
> - signal(SIGTERM, SIG_DFL);
> - signal(SIGWINCH, SIG_IGN);
> -
> - fd = phys_mapping(uml_to_phys(__syscall_stub_start), &offset);
> - addr = mmap64((void *) STUB_CODE, UM_KERN_PAGE_SIZE,
> - PROT_EXEC, MAP_FIXED | MAP_PRIVATE, fd, offset);
> - if (addr == MAP_FAILED) {
> - os_info("mapping mmap stub at 0x%lx failed, errno = %d\n",
> - STUB_CODE, errno);
> - exit(1);
> + struct stub_init_data init_data = {
> + .stub_start = STUB_START,
> + .segv_handler = STUB_CODE +
> + (unsigned long) stub_segv_handler -
> + (unsigned long) __syscall_stub_start,
> + };
> + struct iomem_region *iomem;
> + int ret;
> +
> + init_data.stub_code_fd = phys_mapping(uml_to_phys(__syscall_stub_start),
> + &offset);
> + init_data.stub_code_offset = MMAP_OFFSET(offset);
> +
> + init_data.stub_data_fd = phys_mapping(uml_to_phys(stack), &offset);
> + init_data.stub_data_offset = MMAP_OFFSET(offset);
> +
> + /* Set CLOEXEC on all FDs and then unset on all memory related FDs */
> + close_range(0, ~0U, CLOSE_RANGE_CLOEXEC);
> +
> + fcntl(init_data.stub_data_fd, F_SETFD, 0);
> + for (iomem = iomem_regions; iomem; iomem = iomem->next)
> + fcntl(iomem->fd, F_SETFD, 0);
> +
> + /* Create a pipe for init_data (no CLOEXEC) and dup2 to STDIN */
> + if (pipe2(pipe_fds, 0))
> + exit(2);
> +
> + close(0);
> + if (dup2(pipe_fds[0], 0) < 0) {
> + close(pipe_fds[0]);
> + close(pipe_fds[1]);
> + exit(3);
> }
> + close(pipe_fds[0]);
> +
> + /* Write init_data and close write side */
> + ret = write(pipe_fds[1], &init_data, sizeof(init_data));
> + close(pipe_fds[1]);
> +
> + if (ret != sizeof(init_data))
> + exit(4);
> +
> + execveat(stub_exe_fd, "", argv, NULL, AT_EMPTY_PATH);
>
> - fd = phys_mapping(uml_to_phys(stack), &offset);
> - addr = mmap((void *) STUB_DATA,
> - STUB_DATA_PAGES * UM_KERN_PAGE_SIZE, PROT_READ | PROT_WRITE,
> - MAP_FIXED | MAP_SHARED, fd, offset);
> - if (addr == MAP_FAILED) {
> - os_info("mapping segfault stack at 0x%lx failed, errno = %d\n",
> - STUB_DATA, errno);
> - exit(1);
> + exit(5);
> +}
> +
> +extern char stub_exe_start[];
> +extern char stub_exe_end[];
> +
> +extern char *tempdir;
> +
> +#define STUB_EXE_NAME_TEMPLATE "/uml-userspace-XXXXXX"
> +
> +#ifndef MFD_EXEC
> +#define MFD_EXEC 0x0010U
> +#endif
> +
> +static int __init init_stub_exe_fd(void)
> +{
> + size_t written = 0;
> + char *tmpfile = NULL;
> +
> + stub_exe_fd = memfd_create("uml-userspace",
> + MFD_EXEC | MFD_CLOEXEC | MFD_ALLOW_SEALING);
> +
> + if (stub_exe_fd < 0) {
> + printk(UM_KERN_INFO "Could not create executable memfd, using temporary file!");
> +
> + tmpfile = malloc(strlen(tempdir) +
> + strlen(STUB_EXE_NAME_TEMPLATE) + 1);
> + if (tmpfile == NULL)
> + panic("Failed to allocate memory for stub binary name");
> +
> + strcpy(tmpfile, tempdir);
> + strcat(tmpfile, STUB_EXE_NAME_TEMPLATE);
> +
> + stub_exe_fd = mkstemp(tmpfile);
> + if (stub_exe_fd < 0)
> + panic("Could not create temporary file for stub binary: %d",
> + -errno);
> }
>
> - set_sigstack((void *) STUB_DATA, STUB_DATA_PAGES * UM_KERN_PAGE_SIZE);
> - sigemptyset(&sa.sa_mask);
> - sa.sa_flags = SA_ONSTACK | SA_NODEFER | SA_SIGINFO;
> - sa.sa_sigaction = (void *) segv_handler;
> - sa.sa_restorer = NULL;
> - if (sigaction(SIGSEGV, &sa, NULL) < 0) {
> - os_info("%s - setting SIGSEGV handler failed - errno = %d\n",
> - __func__, errno);
> - exit(1);
> + while (written < stub_exe_end - stub_exe_start) {
> + ssize_t res = write(stub_exe_fd, stub_exe_start + written,
> + stub_exe_end - stub_exe_start - written);
> + if (res < 0) {
> + if (errno == EINTR)
> + continue;
> +
> + if (tmpfile)
> + unlink(tmpfile);
> + panic("Failed write stub binary: %d", -errno);
> + }
> +
> + written += res;
> + }
> +
> + if (!tmpfile) {
> + fcntl(stub_exe_fd, F_ADD_SEALS,
> + F_SEAL_WRITE | F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_SEAL);
> + } else {
> + if (fchmod(stub_exe_fd, 00500) < 0) {
> + unlink(tmpfile);
> + panic("Could not make stub binary executable: %d",
> + -errno);
> + }
> +
> + close(stub_exe_fd);
> + stub_exe_fd = open(tmpfile, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
> + if (stub_exe_fd < 0) {
> + unlink(tmpfile);
> + panic("Could not reopen stub binary: %d", -errno);
> + }
> +
> + unlink(tmpfile);
> + free(tmpfile);
> }
>
> - kill(os_getpid(), SIGSTOP);
> return 0;
> }
> +__initcall(init_stub_exe_fd);
>
> int userspace_pid[NR_CPUS];
>
> @@ -270,7 +339,7 @@ int start_userspace(unsigned long stub_stack)
> {
> void *stack;
> unsigned long sp;
> - int pid, status, n, flags, err;
> + int pid, status, n, err;
>
> /* setup a temporary stack page */
> stack = mmap(NULL, UM_KERN_PAGE_SIZE,
> @@ -286,10 +355,10 @@ int start_userspace(unsigned long stub_stack)
> /* set stack pointer to the end of the stack page, so it can grow downwards */
> sp = (unsigned long)stack + UM_KERN_PAGE_SIZE;
>
> - flags = CLONE_FILES | SIGCHLD;
> -
> /* clone into new userspace process */
> - pid = clone(userspace_tramp, (void *) sp, flags, (void *) stub_stack);
> + pid = clone(userspace_tramp, (void *) sp,
> + CLONE_VFORK | CLONE_VM | SIGCHLD,
> + (void *)stub_stack);
> if (pid < 0) {
> err = -errno;
> printk(UM_KERN_ERR "%s : clone failed, errno = %d\n",
> --
> 2.46.0
>
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-10-17 7:17 ` David Gow
@ 2024-10-17 8:18 ` Johannes Berg
2024-10-17 12:05 ` Benjamin Berg
2024-10-17 9:01 ` Benjamin Berg
1 sibling, 1 reply; 18+ messages in thread
From: Johannes Berg @ 2024-10-17 8:18 UTC (permalink / raw)
To: David Gow, Benjamin Berg
Cc: linux-um, Benjamin Berg, KUnit Development, Kees Cook
On Thu, 2024-10-17 at 15:17 +0800, David Gow wrote:
> It turns out that this breaks the KUnit user alloc helpers on x86_64,
> at least on my machine.
Yay, second bug from this ;-)
> This can be reproduced with:
> ./tools/testing/kunit/kunit.py run usercopy
>
> Though the 32-bit version works:
> ./tools/testing/kunit/kunit.py run usercopy --kconfig_add CONFIG_64BIT=n
>
> The error we're getting is:
> start_userspace : expected SIGSTOP, got status = 139
> Could not create userspace mm
>
> This basically is the result of the stub_exe segfaulting very early on
> in its execution.
>
> It seems that this is due to the stack being misaligned, and so the
> generated SSE instructions are faulting. The workarounds I've tested
> here include:
> a) Build the stub with -mno-sse
> b) Decorate real_init() with __attribute__((force_align_arg_pointer))
> c) Decorate __start() with __attribute__((naked))
>
> The last one seems to validate my theory as to why this is occurring:
> __start's prologue is misaligning the stack, as __start is not
> actually _called_ from anything, so there's no 8-byte misalignment to
> hold the return address.
>
> If this makes sense, I'll send a patch out with whichever the
> preferred fix(es) are. My guess is that (c) is the "proper" fix,
> though I'd not _miss_ SSE if we chose to disable it for the handful of
> instructions here anyway.
Interesting. Actually somewhere here while reviewing this, though I
don't remember precisely if it was _start, I thought we might need
__attribute__((naked)) to ensure we don't get extra things, but I let
that thought go since it seemed to work and I didn't wrap my head around
it too much...
I wonder now if the SSE instructions generated are memset() and that
goes away with the patches that Nathan just sent to not have the memset
(which was due to -ftrivial-auto-var-init) in the first place?
But anyway all of these pretty much sound reasonable. We don't need much
to happen in the stub here, simpler is better.
johannes
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-10-17 7:17 ` David Gow
2024-10-17 8:18 ` Johannes Berg
@ 2024-10-17 9:01 ` Benjamin Berg
1 sibling, 0 replies; 18+ messages in thread
From: Benjamin Berg @ 2024-10-17 9:01 UTC (permalink / raw)
To: David Gow; +Cc: linux-um, KUnit Development, Kees Cook
Hi,
On Thu, 2024-10-17 at 15:17 +0800, David Gow wrote:
> On Thu, 19 Sept 2024 at 20:45, Benjamin Berg <benjamin@sipsolutions.net> wrote:
> >
> > [SNIP]
>
> It turns out that this breaks the KUnit user alloc helpers on x86_64,
> at least on my machine.
>
> This can be reproduced with:
> ./tools/testing/kunit/kunit.py run usercopy
>
> Though the 32-bit version works:
> ./tools/testing/kunit/kunit.py run usercopy --kconfig_add CONFIG_64BIT=n
>
> The error we're getting is:
> start_userspace : expected SIGSTOP, got status = 139
> Could not create userspace mm
>
> This basically is the result of the stub_exe segfaulting very early on
> in its execution.
>
> It seems that this is due to the stack being misaligned, and so the
> generated SSE instructions are faulting. The workarounds I've tested
> here include:
> a) Build the stub with -mno-sse
> b) Decorate real_init() with __attribute__((force_align_arg_pointer))
> c) Decorate __start() with __attribute__((naked))
>
> The last one seems to validate my theory as to why this is occurring:
> __start's prologue is misaligning the stack, as __start is not
> actually _called_ from anything, so there's no 8-byte misalignment to
> hold the return address.
>
> If this makes sense, I'll send a patch out with whichever the
> preferred fix(es) are. My guess is that (c) is the "proper" fix,
> though I'd not _miss_ SSE if we chose to disable it for the handful of
> instructions here anyway.
Yeah, SSE is pointless here, but we probably should ensure the
alignment is as expected.
So, c) seems like the right thing to do here. I am just a bit confused
about the alignment guarantees from the kernel. Are we correct to
assume a 16 byte alignment there or do we also need b) just to be on
the safe side?
Unfortunately I have not managed to reproduce the issue.
Benjamin
>
> Cheers,
> -- David
>
> > arch/um/Makefile | 3 +-
> > arch/um/include/shared/skas/stub-data.h | 11 ++
> > arch/um/kernel/skas/.gitignore | 2 +
> > arch/um/kernel/skas/Makefile | 33 ++++-
> > arch/um/kernel/skas/stub_exe.c | 88 ++++++++++++
> > arch/um/kernel/skas/stub_exe_embed.S | 11 ++
> > arch/um/os-Linux/mem.c | 2 +-
> > arch/um/os-Linux/skas/process.c | 181 ++++++++++++++++----
> > ----
> > 8 files changed, 271 insertions(+), 60 deletions(-)
> > create mode 100644 arch/um/kernel/skas/.gitignore
> > create mode 100644 arch/um/kernel/skas/stub_exe.c
> > create mode 100644 arch/um/kernel/skas/stub_exe_embed.S
> >
> > diff --git a/arch/um/Makefile b/arch/um/Makefile
> > index 00b63bac5eff..31e367e8ab4d 100644
> > --- a/arch/um/Makefile
> > +++ b/arch/um/Makefile
> > @@ -61,7 +61,8 @@ KBUILD_CFLAGS += $(CFLAGS) $(CFLAGS-y) -
> > D__arch_um__ \
> > $(ARCH_INCLUDE) $(MODE_INCLUDE) -Dvmap=kernel_vmap \
> > -Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
> > -Din6addr_loopback=kernel_in6addr_loopback \
> > - -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr
> > + -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr \
> > + -D__close_range=kernel__close_range
> >
> > KBUILD_RUSTFLAGS += -Crelocation-model=pie
> >
> > diff --git a/arch/um/include/shared/skas/stub-data.h
> > b/arch/um/include/shared/skas/stub-data.h
> > index 2b6b44759dfa..3fbdda727373 100644
> > --- a/arch/um/include/shared/skas/stub-data.h
> > +++ b/arch/um/include/shared/skas/stub-data.h
> > @@ -12,6 +12,17 @@
> > #include <as-layout.h>
> > #include <sysdep/tls.h>
> >
> > +struct stub_init_data {
> > + unsigned long stub_start;
> > +
> > + int stub_code_fd;
> > + unsigned long stub_code_offset;
> > + int stub_data_fd;
> > + unsigned long stub_data_offset;
> > +
> > + unsigned long segv_handler;
> > +};
> > +
> > #define STUB_NEXT_SYSCALL(s) \
> > ((struct stub_syscall *) (((unsigned long) s) + (s)-
> > >cmd_len))
> >
> > diff --git a/arch/um/kernel/skas/.gitignore
> > b/arch/um/kernel/skas/.gitignore
> > new file mode 100644
> > index 000000000000..c3409ced0f38
> > --- /dev/null
> > +++ b/arch/um/kernel/skas/.gitignore
> > @@ -0,0 +1,2 @@
> > +stub_exe
> > +stub_exe.dbg
> > diff --git a/arch/um/kernel/skas/Makefile
> > b/arch/um/kernel/skas/Makefile
> > index 6f86d53e3d69..fbb61968055f 100644
> > --- a/arch/um/kernel/skas/Makefile
> > +++ b/arch/um/kernel/skas/Makefile
> > @@ -3,14 +3,43 @@
> > # Copyright (C) 2002 - 2007 Jeff Dike
> > (jdike@{addtoit,linux.intel}.com)
> > #
> >
> > -obj-y := stub.o mmu.o process.o syscall.o uaccess.o
> > +obj-y := stub.o mmu.o process.o syscall.o uaccess.o \
> > + stub_exe_embed.o
> > +
> > +# Stub executable
> > +
> > +stub_exe_objs-y := stub_exe.o
> > +
> > +stub_exe_objs := $(foreach F,$(stub_exe_objs-y),$(obj)/$F)
> > +
> > +# Object file containing the ELF executable
> > +$(obj)/stub_exe_embed.o: $(src)/stub_exe_embed.S $(obj)/stub_exe
> > +
> > +$(obj)/stub_exe.dbg: $(stub_exe_objs) FORCE
> > + $(call if_changed,stub_exe)
> > +
> > +$(obj)/stub_exe: OBJCOPYFLAGS := -S
> > +$(obj)/stub_exe: $(obj)/stub_exe.dbg FORCE
> > + $(call if_changed,objcopy)
> > +
> > +quiet_cmd_stub_exe = STUB_EXE $@
> > + cmd_stub_exe = $(CC) -nostdlib -o $@ \
> > + $(KBUILD_CFLAGS) $(STUB_EXE_LDFLAGS) \
> > + $(filter %.o,$^)
> > +
> > +STUB_EXE_LDFLAGS = -n -static
> > +
> > +targets += stub_exe.dbg stub_exe $(stub_exe_objs-y)
> > +
> > +# end
> >
> > # stub.o is in the stub, so it can't be built with profiling
> > # GCC hardened also auto-enables -fpic, but we need %ebx so it
> > can't work ->
> > # disable it
> >
> > CFLAGS_stub.o := $(CFLAGS_NO_HARDENING)
> > -UNPROFILE_OBJS := stub.o
> > +CFLAGS_stub_exe.o := $(CFLAGS_NO_HARDENING)
> > +UNPROFILE_OBJS := stub.o stub_exe.o
> > KCOV_INSTRUMENT := n
> >
> > include $(srctree)/arch/um/scripts/Makefile.rules
> > diff --git a/arch/um/kernel/skas/stub_exe.c
> > b/arch/um/kernel/skas/stub_exe.c
> > new file mode 100644
> > index 000000000000..bc6ba2e4d805
> > --- /dev/null
> > +++ b/arch/um/kernel/skas/stub_exe.c
> > @@ -0,0 +1,88 @@
> > +#include <sys/ptrace.h>
> > +#include <sys/prctl.h>
> > +#include <asm/unistd.h>
> > +#include <sysdep/stub.h>
> > +#include <stub-data.h>
> > +
> > +void _start(void);
> > +
> > +noinline static void real_init(void)
> > +{
> > + struct stub_init_data init_data;
> > + unsigned long res;
> > + struct {
> > + void *ss_sp;
> > + int ss_flags;
> > + size_t ss_size;
> > + } stack = {
> > + .ss_size = STUB_DATA_PAGES * UM_KERN_PAGE_SIZE,
> > + };
> > + struct {
> > + void *sa_handler_;
> > + unsigned long sa_flags;
> > + void *sa_restorer;
> > + unsigned long long sa_mask;
> > + } sa = {
> > + /* Need to set SA_RESTORER (but the handler never
> > returns) */
> > + .sa_flags = SA_ONSTACK | SA_NODEFER | SA_SIGINFO |
> > 0x04000000,
> > + /* no need to mask any signals */
> > + .sa_mask = 0,
> > + };
> > +
> > + /* set a nice name */
> > + stub_syscall2(__NR_prctl, PR_SET_NAME, (unsigned long)"uml-
> > userspace");
> > +
> > + /* read information from STDIN and close it */
> > + res = stub_syscall3(__NR_read, 0,
> > + (unsigned long)&init_data,
> > sizeof(init_data));
> > + if (res != sizeof(init_data))
> > + stub_syscall1(__NR_exit, 10);
> > +
> > + stub_syscall1(__NR_close, 0);
> > +
> > + /* map stub code + data */
> > + res = stub_syscall6(STUB_MMAP_NR,
> > + init_data.stub_start,
> > UM_KERN_PAGE_SIZE,
> > + PROT_READ | PROT_EXEC, MAP_FIXED |
> > MAP_SHARED,
> > + init_data.stub_code_fd,
> > init_data.stub_code_offset);
> > + if (res != init_data.stub_start)
> > + stub_syscall1(__NR_exit, 11);
> > +
> > + res = stub_syscall6(STUB_MMAP_NR,
> > + init_data.stub_start +
> > UM_KERN_PAGE_SIZE,
> > + STUB_DATA_PAGES * UM_KERN_PAGE_SIZE,
> > + PROT_READ | PROT_WRITE, MAP_FIXED |
> > MAP_SHARED,
> > + init_data.stub_data_fd,
> > init_data.stub_data_offset);
> > + if (res != init_data.stub_start + UM_KERN_PAGE_SIZE)
> > + stub_syscall1(__NR_exit, 12);
> > +
> > + /* setup signal stack inside stub data */
> > + stack.ss_sp = (void *)init_data.stub_start +
> > UM_KERN_PAGE_SIZE;
> > + stub_syscall2(__NR_sigaltstack, (unsigned long)&stack, 0);
> > +
> > + /* register SIGSEGV handler */
> > + sa.sa_handler_ = (void *) init_data.segv_handler;
> > + res = stub_syscall4(__NR_rt_sigaction, SIGSEGV, (unsigned
> > long)&sa, 0,
> > + sizeof(sa.sa_mask));
> > + if (res != 0)
> > + stub_syscall1(__NR_exit, 13);
> > +
> > + stub_syscall4(__NR_ptrace, PTRACE_TRACEME, 0, 0, 0);
> > +
> > + stub_syscall2(__NR_kill, stub_syscall0(__NR_getpid),
> > SIGSTOP);
> > +
> > + stub_syscall1(__NR_exit, 14);
> > +
> > + __builtin_unreachable();
> > +}
> > +
> > +void _start(void)
> > +{
> > + char *alloc;
> > +
> > + /* Make enough space for the stub (including space for
> > alignment) */
> > + alloc = __builtin_alloca((1 + 2 * STUB_DATA_PAGES - 1) *
> > UM_KERN_PAGE_SIZE);
> > + asm volatile("" : "+r,m"(alloc) : : "memory");
> > +
> > + real_init();
> > +}
> > diff --git a/arch/um/kernel/skas/stub_exe_embed.S
> > b/arch/um/kernel/skas/stub_exe_embed.S
> > new file mode 100644
> > index 000000000000..6d8914fbe8f1
> > --- /dev/null
> > +++ b/arch/um/kernel/skas/stub_exe_embed.S
> > @@ -0,0 +1,11 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <linux/init.h>
> > +#include <linux/linkage.h>
> > +
> > +__INITDATA
> > +
> > +SYM_DATA_START(stub_exe_start)
> > + .incbin "arch/um/kernel/skas/stub_exe"
> > +SYM_DATA_END_LABEL(stub_exe_start, SYM_L_GLOBAL, stub_exe_end)
> > +
> > +__FINIT
> > diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c
> > index cf44d386f23c..857e3deab293 100644
> > --- a/arch/um/os-Linux/mem.c
> > +++ b/arch/um/os-Linux/mem.c
> > @@ -42,7 +42,7 @@ void kasan_map_memory(void *start, size_t len)
> > }
> >
> > /* Set by make_tempfile() during early boot. */
> > -static char *tempdir = NULL;
> > +char *tempdir = NULL;
> >
> > /* Check if dir is on tmpfs. Return 0 if yes, -1 if no or error.
> > */
> > static int __init check_tmpfs(const char *dir)
> > diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-
> > Linux/skas/process.c
> > index b6f656bcffb1..2caaa9e85752 100644
> > --- a/arch/um/os-Linux/skas/process.c
> > +++ b/arch/um/os-Linux/skas/process.c
> > @@ -10,8 +10,11 @@
> > #include <sched.h>
> > #include <errno.h>
> > #include <string.h>
> > +#include <fcntl.h>
> > +#include <mem_user.h>
> > #include <sys/mman.h>
> > #include <sys/wait.h>
> > +#include <sys/stat.h>
> > #include <asm/unistd.h>
> > #include <as-layout.h>
> > #include <init.h>
> > @@ -189,69 +192,135 @@ static void handle_trap(int pid, struct
> > uml_pt_regs *regs)
> >
> > extern char __syscall_stub_start[];
> >
> > -/**
> > - * userspace_tramp() - userspace trampoline
> > - * @stack: pointer to the new userspace stack page
> > - *
> > - * The userspace trampoline is used to setup a new userspace
> > process in start_userspace() after it was clone()'ed.
> > - * This function will run on a temporary stack page.
> > - * It ptrace()'es itself, then
> > - * Two pages are mapped into the userspace address space:
> > - * - STUB_CODE (with EXEC), which contains the skas stub code
> > - * - STUB_DATA (with R/W), which contains a data page that is used
> > to transfer certain data between the UML userspace process and the
> > UML kernel.
> > - * Also for the userspace process a SIGSEGV handler is installed
> > to catch pagefaults in the userspace process.
> > - * And last the process stops itself to give control to the UML
> > kernel for this userspace process.
> > - *
> > - * Return: Always zero, otherwise the current userspace process is
> > ended with non null exit() call
> > - */
> > +static int stub_exe_fd;
> > +
> > static int userspace_tramp(void *stack)
> > {
> > - struct sigaction sa;
> > - void *addr;
> > - int fd;
> > + char *const argv[] = { "uml-userspace", NULL };
> > + int pipe_fds[2];
> > unsigned long long offset;
> > - unsigned long segv_handler = STUB_CODE +
> > - (unsigned long)
> > stub_segv_handler -
> > - (unsigned long)
> > __syscall_stub_start;
> > -
> > - ptrace(PTRACE_TRACEME, 0, 0, 0);
> > -
> > - signal(SIGTERM, SIG_DFL);
> > - signal(SIGWINCH, SIG_IGN);
> > -
> > - fd = phys_mapping(uml_to_phys(__syscall_stub_start),
> > &offset);
> > - addr = mmap64((void *) STUB_CODE, UM_KERN_PAGE_SIZE,
> > - PROT_EXEC, MAP_FIXED | MAP_PRIVATE, fd,
> > offset);
> > - if (addr == MAP_FAILED) {
> > - os_info("mapping mmap stub at 0x%lx failed, errno =
> > %d\n",
> > - STUB_CODE, errno);
> > - exit(1);
> > + struct stub_init_data init_data = {
> > + .stub_start = STUB_START,
> > + .segv_handler = STUB_CODE +
> > + (unsigned long) stub_segv_handler -
> > + (unsigned long)
> > __syscall_stub_start,
> > + };
> > + struct iomem_region *iomem;
> > + int ret;
> > +
> > + init_data.stub_code_fd =
> > phys_mapping(uml_to_phys(__syscall_stub_start),
> > + &offset);
> > + init_data.stub_code_offset = MMAP_OFFSET(offset);
> > +
> > + init_data.stub_data_fd = phys_mapping(uml_to_phys(stack),
> > &offset);
> > + init_data.stub_data_offset = MMAP_OFFSET(offset);
> > +
> > + /* Set CLOEXEC on all FDs and then unset on all memory
> > related FDs */
> > + close_range(0, ~0U, CLOSE_RANGE_CLOEXEC);
> > +
> > + fcntl(init_data.stub_data_fd, F_SETFD, 0);
> > + for (iomem = iomem_regions; iomem; iomem = iomem->next)
> > + fcntl(iomem->fd, F_SETFD, 0);
> > +
> > + /* Create a pipe for init_data (no CLOEXEC) and dup2 to
> > STDIN */
> > + if (pipe2(pipe_fds, 0))
> > + exit(2);
> > +
> > + close(0);
> > + if (dup2(pipe_fds[0], 0) < 0) {
> > + close(pipe_fds[0]);
> > + close(pipe_fds[1]);
> > + exit(3);
> > }
> > + close(pipe_fds[0]);
> > +
> > + /* Write init_data and close write side */
> > + ret = write(pipe_fds[1], &init_data, sizeof(init_data));
> > + close(pipe_fds[1]);
> > +
> > + if (ret != sizeof(init_data))
> > + exit(4);
> > +
> > + execveat(stub_exe_fd, "", argv, NULL, AT_EMPTY_PATH);
> >
> > - fd = phys_mapping(uml_to_phys(stack), &offset);
> > - addr = mmap((void *) STUB_DATA,
> > - STUB_DATA_PAGES * UM_KERN_PAGE_SIZE, PROT_READ
> > | PROT_WRITE,
> > - MAP_FIXED | MAP_SHARED, fd, offset);
> > - if (addr == MAP_FAILED) {
> > - os_info("mapping segfault stack at 0x%lx failed,
> > errno = %d\n",
> > - STUB_DATA, errno);
> > - exit(1);
> > + exit(5);
> > +}
> > +
> > +extern char stub_exe_start[];
> > +extern char stub_exe_end[];
> > +
> > +extern char *tempdir;
> > +
> > +#define STUB_EXE_NAME_TEMPLATE "/uml-userspace-XXXXXX"
> > +
> > +#ifndef MFD_EXEC
> > +#define MFD_EXEC 0x0010U
> > +#endif
> > +
> > +static int __init init_stub_exe_fd(void)
> > +{
> > + size_t written = 0;
> > + char *tmpfile = NULL;
> > +
> > + stub_exe_fd = memfd_create("uml-userspace",
> > + MFD_EXEC | MFD_CLOEXEC |
> > MFD_ALLOW_SEALING);
> > +
> > + if (stub_exe_fd < 0) {
> > + printk(UM_KERN_INFO "Could not create executable
> > memfd, using temporary file!");
> > +
> > + tmpfile = malloc(strlen(tempdir) +
> > + strlen(STUB_EXE_NAME_TEMPLATE) +
> > 1);
> > + if (tmpfile == NULL)
> > + panic("Failed to allocate memory for stub
> > binary name");
> > +
> > + strcpy(tmpfile, tempdir);
> > + strcat(tmpfile, STUB_EXE_NAME_TEMPLATE);
> > +
> > + stub_exe_fd = mkstemp(tmpfile);
> > + if (stub_exe_fd < 0)
> > + panic("Could not create temporary file for
> > stub binary: %d",
> > + -errno);
> > }
> >
> > - set_sigstack((void *) STUB_DATA, STUB_DATA_PAGES *
> > UM_KERN_PAGE_SIZE);
> > - sigemptyset(&sa.sa_mask);
> > - sa.sa_flags = SA_ONSTACK | SA_NODEFER | SA_SIGINFO;
> > - sa.sa_sigaction = (void *) segv_handler;
> > - sa.sa_restorer = NULL;
> > - if (sigaction(SIGSEGV, &sa, NULL) < 0) {
> > - os_info("%s - setting SIGSEGV handler failed -
> > errno = %d\n",
> > - __func__, errno);
> > - exit(1);
> > + while (written < stub_exe_end - stub_exe_start) {
> > + ssize_t res = write(stub_exe_fd, stub_exe_start +
> > written,
> > + stub_exe_end - stub_exe_start -
> > written);
> > + if (res < 0) {
> > + if (errno == EINTR)
> > + continue;
> > +
> > + if (tmpfile)
> > + unlink(tmpfile);
> > + panic("Failed write stub binary: %d", -
> > errno);
> > + }
> > +
> > + written += res;
> > + }
> > +
> > + if (!tmpfile) {
> > + fcntl(stub_exe_fd, F_ADD_SEALS,
> > + F_SEAL_WRITE | F_SEAL_SHRINK | F_SEAL_GROW |
> > F_SEAL_SEAL);
> > + } else {
> > + if (fchmod(stub_exe_fd, 00500) < 0) {
> > + unlink(tmpfile);
> > + panic("Could not make stub binary
> > executable: %d",
> > + -errno);
> > + }
> > +
> > + close(stub_exe_fd);
> > + stub_exe_fd = open(tmpfile, O_RDONLY | O_CLOEXEC |
> > O_NOFOLLOW);
> > + if (stub_exe_fd < 0) {
> > + unlink(tmpfile);
> > + panic("Could not reopen stub binary: %d", -
> > errno);
> > + }
> > +
> > + unlink(tmpfile);
> > + free(tmpfile);
> > }
> >
> > - kill(os_getpid(), SIGSTOP);
> > return 0;
> > }
> > +__initcall(init_stub_exe_fd);
> >
> > int userspace_pid[NR_CPUS];
> >
> > @@ -270,7 +339,7 @@ int start_userspace(unsigned long stub_stack)
> > {
> > void *stack;
> > unsigned long sp;
> > - int pid, status, n, flags, err;
> > + int pid, status, n, err;
> >
> > /* setup a temporary stack page */
> > stack = mmap(NULL, UM_KERN_PAGE_SIZE,
> > @@ -286,10 +355,10 @@ int start_userspace(unsigned long stub_stack)
> > /* set stack pointer to the end of the stack page, so it
> > can grow downwards */
> > sp = (unsigned long)stack + UM_KERN_PAGE_SIZE;
> >
> > - flags = CLONE_FILES | SIGCHLD;
> > -
> > /* clone into new userspace process */
> > - pid = clone(userspace_tramp, (void *) sp, flags, (void *)
> > stub_stack);
> > + pid = clone(userspace_tramp, (void *) sp,
> > + CLONE_VFORK | CLONE_VM | SIGCHLD,
> > + (void *)stub_stack);
> > if (pid < 0) {
> > err = -errno;
> > printk(UM_KERN_ERR "%s : clone failed, errno =
> > %d\n",
> > --
> > 2.46.0
> >
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-10-17 8:18 ` Johannes Berg
@ 2024-10-17 12:05 ` Benjamin Berg
2024-10-17 23:17 ` David Gow
0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Berg @ 2024-10-17 12:05 UTC (permalink / raw)
To: Johannes Berg, David Gow; +Cc: linux-um, KUnit Development, Kees Cook
Hi,
On Thu, 2024-10-17 at 10:18 +0200, Johannes Berg wrote:
> [SNIP]
>
> I wonder now if the SSE instructions generated are memset() and that
> goes away with the patches that Nathan just sent to not have the memset
> (which was due to -ftrivial-auto-var-init) in the first place?
I am guessing it would not fix it. I can reproduce the problem if I add
__attribute__((aligned(16)))
to the "stack" variable. That is sufficient for GCC to emit an
offending movaps instruction for me. The -ftrivial-auto-var-init=
option will not affect that initialization.
> But anyway all of these pretty much sound reasonable. We don't need much
> to happen in the stub here, simpler is better.
Yeah, and I guess c) really is sufficient by itself.
Benjamin
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v9 02/10] um: use execveat to create userspace MMs
2024-10-17 12:05 ` Benjamin Berg
@ 2024-10-17 23:17 ` David Gow
0 siblings, 0 replies; 18+ messages in thread
From: David Gow @ 2024-10-17 23:17 UTC (permalink / raw)
To: Benjamin Berg; +Cc: Johannes Berg, linux-um, KUnit Development, Kees Cook
[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]
On Thu, 17 Oct 2024 at 20:05, Benjamin Berg <benjamin@sipsolutions.net> wrote:
>
> Hi,
>
> On Thu, 2024-10-17 at 10:18 +0200, Johannes Berg wrote:
> > [SNIP]
> >
> > I wonder now if the SSE instructions generated are memset() and that
> > goes away with the patches that Nathan just sent to not have the memset
> > (which was due to -ftrivial-auto-var-init) in the first place?
>
> I am guessing it would not fix it. I can reproduce the problem if I add
> __attribute__((aligned(16)))
> to the "stack" variable. That is sufficient for GCC to emit an
> offending movaps instruction for me. The -ftrivial-auto-var-init=
> option will not affect that initialization.
>
Yeah, that seems to be what's happening here: the SSE use is all in
the 'stack' and 'sa' variable initialisation. Nathan's patch doesn't
fix it for me, but then again, I can't reproduce it with clang, just
gcc 13.2.0.
> > But anyway all of these pretty much sound reasonable. We don't need much
> > to happen in the stub here, simpler is better.
>
> Yeah, and I guess c) really is sufficient by itself.
I've sent (c) out here:
https://lore.kernel.org/linux-um/20241017231007.1500497-2-davidgow@google.com/T/#u
Thanks,
-- David
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2024-10-17 23:17 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-19 12:45 [PATCH v9 00/10] Increased address space for 64 bit Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 01/10] um: Add generic stub_syscall1 function Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 02/10] um: use execveat to create userspace MMs Benjamin Berg
2024-10-16 2:34 ` Nathan Chancellor
2024-10-16 8:19 ` Berg, Benjamin
2024-10-17 7:17 ` David Gow
2024-10-17 8:18 ` Johannes Berg
2024-10-17 12:05 ` Benjamin Berg
2024-10-17 23:17 ` David Gow
2024-10-17 9:01 ` Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 03/10] um: Set parent death signal for userspace process Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 04/10] um: Set parent death signal for winch thread/process Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 05/10] um: Add compile time assert that stub fits on a page Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 06/10] um: Calculate stub data address relative to stub code Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 07/10] um: Limit TASK_SIZE to the addressable range Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 08/10] um: Discover host_task_size from envp Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 09/10] um: clear all memory in new userspace processes Benjamin Berg
2024-09-19 12:45 ` [PATCH v9 10/10] um: Switch to 4 level page tables on 64 bit Benjamin Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox