2021-03-26 14:40:55

by Christoph Hellwig

[permalink] [raw]
Subject: cleanup compat exec handling

Hi all,

this series cleans up the exec code by sharing the native vs compat
versions less awkwardly.

Diffstat:
arch/arm64/include/asm/unistd32.h | 4
arch/mips/kernel/syscalls/syscall_n32.tbl | 4
arch/mips/kernel/syscalls/syscall_o32.tbl | 4
arch/parisc/kernel/syscalls/syscall.tbl | 4
arch/powerpc/kernel/syscalls/syscall.tbl | 4
arch/s390/kernel/syscalls/syscall.tbl | 4
arch/sparc/kernel/syscalls.S | 4
arch/x86/entry/syscall_x32.c | 2
arch/x86/entry/syscalls/syscall_32.tbl | 4
arch/x86/entry/syscalls/syscall_64.tbl | 4
fs/exec.c | 136 +++------------------
include/linux/compat.h | 7 -
include/uapi/asm-generic/unistd.h | 4
tools/include/uapi/asm-generic/unistd.h | 4
tools/perf/arch/powerpc/entry/syscalls/syscall.tbl | 4
tools/perf/arch/s390/entry/syscalls/syscall.tbl | 4
tools/perf/arch/x86/entry/syscalls/syscall_64.tbl | 4
17 files changed, 53 insertions(+), 148 deletions(-)


2021-03-26 14:41:21

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/4] exec: simplify the compat syscall handling

The only differenence betweeen the compat exec* syscalls and their
native versions is the compat_ptr sign extension, and the fact that
the pointer arithmetics for the two dimensional arrays needs to use
the compat pointer size. Instead of the compat wrappers and the
struct user_arg_ptr machinery just use in_compat_syscall() to do the
right thing for the compat case deep inside get_user_arg_ptr().

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/arm64/include/asm/unistd32.h | 4 +-
arch/mips/kernel/syscalls/syscall_n32.tbl | 4 +-
arch/mips/kernel/syscalls/syscall_o32.tbl | 4 +-
arch/parisc/kernel/syscalls/syscall.tbl | 4 +-
arch/powerpc/kernel/syscalls/syscall.tbl | 4 +-
arch/s390/kernel/syscalls/syscall.tbl | 4 +-
arch/sparc/kernel/syscalls.S | 4 +-
arch/x86/entry/syscall_x32.c | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 4 +-
arch/x86/entry/syscalls/syscall_64.tbl | 4 +-
fs/exec.c | 101 ++++--------------
include/linux/compat.h | 7 --
include/uapi/asm-generic/unistd.h | 4 +-
tools/include/uapi/asm-generic/unistd.h | 4 +-
.../arch/powerpc/entry/syscalls/syscall.tbl | 4 +-
.../perf/arch/s390/entry/syscalls/syscall.tbl | 4 +-
.../arch/x86/entry/syscalls/syscall_64.tbl | 4 +-
17 files changed, 48 insertions(+), 118 deletions(-)

diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 3d874f624056b1..34360760773201 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -33,7 +33,7 @@ __SYSCALL(__NR_link, sys_link)
#define __NR_unlink 10
__SYSCALL(__NR_unlink, sys_unlink)
#define __NR_execve 11
-__SYSCALL(__NR_execve, compat_sys_execve)
+__SYSCALL(__NR_execve, sys_execve)
#define __NR_chdir 12
__SYSCALL(__NR_chdir, sys_chdir)
/* 13 was sys_time */
@@ -785,7 +785,7 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
#define __NR_bpf 386
__SYSCALL(__NR_bpf, sys_bpf)
#define __NR_execveat 387
-__SYSCALL(__NR_execveat, compat_sys_execveat)
+__SYSCALL(__NR_execveat, sys_execveat)
#define __NR_userfaultfd 388
__SYSCALL(__NR_userfaultfd, sys_userfaultfd)
#define __NR_membarrier 389
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 8fd8c1790941c6..4da26b7f95d172 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -64,7 +64,7 @@
54 n32 getsockopt sys_getsockopt
55 n32 clone __sys_clone
56 n32 fork __sys_fork
-57 n32 execve compat_sys_execve
+57 n32 execve sys_execve
58 n32 exit sys_exit
59 n32 wait4 compat_sys_wait4
60 n32 kill sys_kill
@@ -328,7 +328,7 @@
317 n32 getrandom sys_getrandom
318 n32 memfd_create sys_memfd_create
319 n32 bpf sys_bpf
-320 n32 execveat compat_sys_execveat
+320 n32 execveat sys_execveat
321 n32 userfaultfd sys_userfaultfd
322 n32 membarrier sys_membarrier
323 n32 mlock2 sys_mlock2
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 090d29ca80ff8f..33818dd2462090 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -18,7 +18,7 @@
8 o32 creat sys_creat
9 o32 link sys_link
10 o32 unlink sys_unlink
-11 o32 execve sys_execve compat_sys_execve
+11 o32 execve sys_execve
12 o32 chdir sys_chdir
13 o32 time sys_time32
14 o32 mknod sys_mknod
@@ -367,7 +367,7 @@
353 o32 getrandom sys_getrandom
354 o32 memfd_create sys_memfd_create
355 o32 bpf sys_bpf
-356 o32 execveat sys_execveat compat_sys_execveat
+356 o32 execveat sys_execveat
357 o32 userfaultfd sys_userfaultfd
358 o32 membarrier sys_membarrier
359 o32 mlock2 sys_mlock2
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 271a9251968345..81085f10db4777 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -18,7 +18,7 @@
8 common creat sys_creat
9 common link sys_link
10 common unlink sys_unlink
-11 common execve sys_execve compat_sys_execve
+11 common execve sys_execve
12 common chdir sys_chdir
13 32 time sys_time32
13 64 time sys_time
@@ -385,7 +385,7 @@
339 common getrandom sys_getrandom
340 common memfd_create sys_memfd_create
341 common bpf sys_bpf
-342 common execveat sys_execveat compat_sys_execveat
+342 common execveat sys_execveat
343 common membarrier sys_membarrier
344 common userfaultfd parisc_userfaultfd
345 common mlock2 sys_mlock2
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 0b2480cf3e4793..97e20641366d94 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -18,7 +18,7 @@
8 common creat sys_creat
9 common link sys_link
10 common unlink sys_unlink
-11 nospu execve sys_execve compat_sys_execve
+11 nospu execve sys_execve
12 common chdir sys_chdir
13 32 time sys_time32
13 64 time sys_time
@@ -452,7 +452,7 @@
359 common getrandom sys_getrandom
360 common memfd_create sys_memfd_create
361 common bpf sys_bpf
-362 nospu execveat sys_execveat compat_sys_execveat
+362 nospu execveat sys_execveat
363 32 switch_endian sys_ni_syscall
363 64 switch_endian sys_switch_endian
363 spu switch_endian sys_ni_syscall
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 3abef2144dac79..8d9f3ab509d583 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -18,7 +18,7 @@
8 common creat sys_creat sys_creat
9 common link sys_link sys_link
10 common unlink sys_unlink sys_unlink
-11 common execve sys_execve compat_sys_execve
+11 common execve sys_execve sys_execve
12 common chdir sys_chdir sys_chdir
13 32 time - sys_time32
14 common mknod sys_mknod sys_mknod
@@ -361,7 +361,7 @@
351 common bpf sys_bpf sys_bpf
352 common s390_pci_mmio_write sys_s390_pci_mmio_write sys_s390_pci_mmio_write
353 common s390_pci_mmio_read sys_s390_pci_mmio_read sys_s390_pci_mmio_read
-354 common execveat sys_execveat compat_sys_execveat
+354 common execveat sys_execveat sys_execveat
355 common userfaultfd sys_userfaultfd sys_userfaultfd
356 common membarrier sys_membarrier sys_membarrier
357 common recvmmsg sys_recvmmsg compat_sys_recvmmsg_time32
diff --git a/arch/sparc/kernel/syscalls.S b/arch/sparc/kernel/syscalls.S
index 0e8ab0602c360b..554a0dedc80ce5 100644
--- a/arch/sparc/kernel/syscalls.S
+++ b/arch/sparc/kernel/syscalls.S
@@ -16,12 +16,12 @@ sys64_execveat:
sunos_execv:
mov %g0, %o2
sys32_execve:
- set compat_sys_execve, %g1
+ set sys_execve, %g1
jmpl %g1, %g0
flushw

sys32_execveat:
- set compat_sys_execveat, %g1
+ set sys_execveat, %g1
jmpl %g1, %g0
flushw
#endif
diff --git a/arch/x86/entry/syscall_x32.c b/arch/x86/entry/syscall_x32.c
index f2fe0a33bcfdd5..510918355ea10a 100644
--- a/arch/x86/entry/syscall_x32.c
+++ b/arch/x86/entry/syscall_x32.c
@@ -12,6 +12,8 @@
* Reuse the 64-bit entry points for the x32 versions that occupy different
* slots in the syscall table.
*/
+#define __x32_sys_execve __x64_sys_execve
+#define __x32_sys_execveat __x64_sys_execveat
#define __x32_sys_readv __x64_sys_readv
#define __x32_sys_writev __x64_sys_writev
#define __x32_sys_getsockopt __x64_sys_getsockopt
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index a1c9f496fca6a2..539ab7d46a3d35 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -22,7 +22,7 @@
8 i386 creat sys_creat
9 i386 link sys_link
10 i386 unlink sys_unlink
-11 i386 execve sys_execve compat_sys_execve
+11 i386 execve sys_execve
12 i386 chdir sys_chdir
13 i386 time sys_time32
14 i386 mknod sys_mknod
@@ -369,7 +369,7 @@
355 i386 getrandom sys_getrandom
356 i386 memfd_create sys_memfd_create
357 i386 bpf sys_bpf
-358 i386 execveat sys_execveat compat_sys_execveat
+358 i386 execveat sys_execveat
359 i386 socket sys_socket
360 i386 socketpair sys_socketpair
361 i386 bind sys_bind
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7bf01cbe582f03..b121013fdb806a 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -379,7 +379,7 @@
517 x32 recvfrom compat_sys_recvfrom
518 x32 sendmsg compat_sys_sendmsg
519 x32 recvmsg compat_sys_recvmsg
-520 x32 execve compat_sys_execve
+520 x32 execve sys_execve
521 x32 ptrace compat_sys_ptrace
522 x32 rt_sigpending compat_sys_rt_sigpending
523 x32 rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
@@ -404,7 +404,7 @@
542 x32 getsockopt sys_getsockopt
543 x32 io_setup compat_sys_io_setup
544 x32 io_submit compat_sys_io_submit
-545 x32 execveat compat_sys_execveat
+545 x32 execveat sys_execveat
546 x32 preadv2 compat_sys_preadv64v2
547 x32 pwritev2 compat_sys_pwritev64v2
# This is the end of the legacy x32 range. Numbers 548 and above are
diff --git a/fs/exec.c b/fs/exec.c
index 06e07278b456fa..b34c1eb9e7ad8e 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -391,47 +391,34 @@ static int bprm_mm_init(struct linux_binprm *bprm)
return err;
}

-struct user_arg_ptr {
-#ifdef CONFIG_COMPAT
- bool is_compat;
-#endif
- union {
- const char __user *const __user *native;
-#ifdef CONFIG_COMPAT
- const compat_uptr_t __user *compat;
-#endif
- } ptr;
-};
-
-static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
+static const char __user *
+get_user_arg_ptr(const char __user *const __user *argv, int nr)
{
- const char __user *native;
-
-#ifdef CONFIG_COMPAT
- if (unlikely(argv.is_compat)) {
+ if (in_compat_syscall()) {
+ const compat_uptr_t __user *compat_argv =
+ compat_ptr((unsigned long)argv);
compat_uptr_t compat;

- if (get_user(compat, argv.ptr.compat + nr))
+ if (get_user(compat, compat_argv + nr))
return ERR_PTR(-EFAULT);
-
return compat_ptr(compat);
- }
-#endif
-
- if (get_user(native, argv.ptr.native + nr))
- return ERR_PTR(-EFAULT);
+ } else {
+ const char __user *native;

- return native;
+ if (get_user(native, argv + nr))
+ return ERR_PTR(-EFAULT);
+ return native;
+ }
}

/*
* count() counts the number of strings in array ARGV.
*/
-static int count(struct user_arg_ptr argv, int max)
+static int count(const char __user *const __user *argv, int max)
{
int i = 0;

- if (argv.ptr.native != NULL) {
+ if (argv) {
for (;;) {
const char __user *p = get_user_arg_ptr(argv, i);

@@ -510,7 +497,7 @@ static int bprm_stack_limits(struct linux_binprm *bprm)
* processes's memory to the new process's stack. The call to get_user_pages()
* ensures the destination page is created and not swapped out.
*/
-static int copy_strings(int argc, struct user_arg_ptr argv,
+static int copy_strings(int argc, const char __user *const __user *argv,
struct linux_binprm *bprm)
{
struct page *kmapped_page = NULL;
@@ -1856,10 +1843,9 @@ static int bprm_execve(struct linux_binprm *bprm,
return retval;
}

-static int do_execveat_common(int fd, struct filename *filename,
- struct user_arg_ptr argv,
- struct user_arg_ptr envp,
- int flags)
+static int do_execveat(int fd, struct filename *filename,
+ const char __user *const __user *argv,
+ const char __user *const __user *envp, int flags)
{
struct linux_binprm *bprm;
int retval;
@@ -1978,35 +1964,6 @@ int kernel_execve(const char *kernel_filename,
return retval;
}

-static int do_execveat(int fd, struct filename *filename,
- const char __user *const __user *__argv,
- const char __user *const __user *__envp,
- int flags)
-{
- struct user_arg_ptr argv = { .ptr.native = __argv };
- struct user_arg_ptr envp = { .ptr.native = __envp };
-
- return do_execveat_common(fd, filename, argv, envp, flags);
-}
-
-#ifdef CONFIG_COMPAT
-static int compat_do_execveat(int fd, struct filename *filename,
- const compat_uptr_t __user *__argv,
- const compat_uptr_t __user *__envp,
- int flags)
-{
- struct user_arg_ptr argv = {
- .is_compat = true,
- .ptr.compat = __argv,
- };
- struct user_arg_ptr envp = {
- .is_compat = true,
- .ptr.compat = __envp,
- };
- return do_execveat_common(fd, filename, argv, envp, flags);
-}
-#endif
-
void set_binfmt(struct linux_binfmt *new)
{
struct mm_struct *mm = current->mm;
@@ -2051,25 +2008,3 @@ SYSCALL_DEFINE5(execveat,
getname_flags(filename, lookup_flags, NULL),
argv, envp, flags);
}
-
-#ifdef CONFIG_COMPAT
-COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
- const compat_uptr_t __user *, argv,
- const compat_uptr_t __user *, envp)
-{
- return compat_do_execveat(AT_FDCWD, getname(filename), argv, envp, 0);
-}
-
-COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
- const char __user *, filename,
- const compat_uptr_t __user *, argv,
- const compat_uptr_t __user *, envp,
- int, flags)
-{
- int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
-
- return compat_do_execveat(fd,
- getname_flags(filename, lookup_flags, NULL),
- argv, envp, flags);
-}
-#endif
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6e65be75360321..894dfcf2dd4590 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -766,10 +766,6 @@ asmlinkage long compat_sys_recvmsg(int fd, struct compat_msghdr __user *msg,
asmlinkage long compat_sys_keyctl(u32 option,
u32 arg2, u32 arg3, u32 arg4, u32 arg5);

-/* arch/example/kernel/sys_example.c */
-asmlinkage long compat_sys_execve(const char __user *filename, const compat_uptr_t __user *argv,
- const compat_uptr_t __user *envp);
-
/* mm/fadvise.c: No generic prototype for fadvise64_64 */

/* mm/, CONFIG_MMU only */
@@ -812,9 +808,6 @@ asmlinkage long compat_sys_open_by_handle_at(int mountdirfd,
int flags);
asmlinkage long compat_sys_sendmmsg(int fd, struct compat_mmsghdr __user *mmsg,
unsigned vlen, unsigned int flags);
-asmlinkage long compat_sys_execveat(int dfd, const char __user *filename,
- const compat_uptr_t __user *argv,
- const compat_uptr_t __user *envp, int flags);
asmlinkage ssize_t compat_sys_preadv2(compat_ulong_t fd,
const struct iovec __user *vec,
compat_ulong_t vlen, u32 pos_low, u32 pos_high, rwf_t flags);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index ce58cff99b6653..74e9bc4f0ce569 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -640,7 +640,7 @@ __SC_COMP(__NR_keyctl, sys_keyctl, compat_sys_keyctl)
#define __NR_clone 220
__SYSCALL(__NR_clone, sys_clone)
#define __NR_execve 221
-__SC_COMP(__NR_execve, sys_execve, compat_sys_execve)
+__SYSCALL(__NR_execve, sys_execve)

#define __NR3264_mmap 222
__SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
@@ -749,7 +749,7 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
#define __NR_bpf 280
__SYSCALL(__NR_bpf, sys_bpf)
#define __NR_execveat 281
-__SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
+__SYSCALL(__NR_execveat, sys_execveat)
#define __NR_userfaultfd 282
__SYSCALL(__NR_userfaultfd, sys_userfaultfd)
#define __NR_membarrier 283
diff --git a/tools/include/uapi/asm-generic/unistd.h b/tools/include/uapi/asm-generic/unistd.h
index ce58cff99b6653..74e9bc4f0ce569 100644
--- a/tools/include/uapi/asm-generic/unistd.h
+++ b/tools/include/uapi/asm-generic/unistd.h
@@ -640,7 +640,7 @@ __SC_COMP(__NR_keyctl, sys_keyctl, compat_sys_keyctl)
#define __NR_clone 220
__SYSCALL(__NR_clone, sys_clone)
#define __NR_execve 221
-__SC_COMP(__NR_execve, sys_execve, compat_sys_execve)
+__SYSCALL(__NR_execve, sys_execve)

#define __NR3264_mmap 222
__SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
@@ -749,7 +749,7 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
#define __NR_bpf 280
__SYSCALL(__NR_bpf, sys_bpf)
#define __NR_execveat 281
-__SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
+__SYSCALL(__NR_execveat, sys_execveat)
#define __NR_userfaultfd 282
__SYSCALL(__NR_userfaultfd, sys_userfaultfd)
#define __NR_membarrier 283
diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
index 0b2480cf3e4793..44fec8c21082a4 100644
--- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
@@ -18,7 +18,7 @@
8 common creat sys_creat
9 common link sys_link
10 common unlink sys_unlink
-11 nospu execve sys_execve compat_sys_execve
+11 nospu execve sys_execve sys_execve
12 common chdir sys_chdir
13 32 time sys_time32
13 64 time sys_time
@@ -452,7 +452,7 @@
359 common getrandom sys_getrandom
360 common memfd_create sys_memfd_create
361 common bpf sys_bpf
-362 nospu execveat sys_execveat compat_sys_execveat
+362 nospu execveat sys_execveat sys_execveat
363 32 switch_endian sys_ni_syscall
363 64 switch_endian sys_switch_endian
363 spu switch_endian sys_ni_syscall
diff --git a/tools/perf/arch/s390/entry/syscalls/syscall.tbl b/tools/perf/arch/s390/entry/syscalls/syscall.tbl
index 3abef2144dac79..8d9f3ab509d583 100644
--- a/tools/perf/arch/s390/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/s390/entry/syscalls/syscall.tbl
@@ -18,7 +18,7 @@
8 common creat sys_creat sys_creat
9 common link sys_link sys_link
10 common unlink sys_unlink sys_unlink
-11 common execve sys_execve compat_sys_execve
+11 common execve sys_execve sys_execve
12 common chdir sys_chdir sys_chdir
13 32 time - sys_time32
14 common mknod sys_mknod sys_mknod
@@ -361,7 +361,7 @@
351 common bpf sys_bpf sys_bpf
352 common s390_pci_mmio_write sys_s390_pci_mmio_write sys_s390_pci_mmio_write
353 common s390_pci_mmio_read sys_s390_pci_mmio_read sys_s390_pci_mmio_read
-354 common execveat sys_execveat compat_sys_execveat
+354 common execveat sys_execveat sys_execveat
355 common userfaultfd sys_userfaultfd sys_userfaultfd
356 common membarrier sys_membarrier sys_membarrier
357 common recvmmsg sys_recvmmsg compat_sys_recvmmsg_time32
diff --git a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
index 7bf01cbe582f03..b121013fdb806a 100644
--- a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
@@ -379,7 +379,7 @@
517 x32 recvfrom compat_sys_recvfrom
518 x32 sendmsg compat_sys_sendmsg
519 x32 recvmsg compat_sys_recvmsg
-520 x32 execve compat_sys_execve
+520 x32 execve sys_execve
521 x32 ptrace compat_sys_ptrace
522 x32 rt_sigpending compat_sys_rt_sigpending
523 x32 rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
@@ -404,7 +404,7 @@
542 x32 getsockopt sys_getsockopt
543 x32 io_setup compat_sys_io_setup
544 x32 io_submit compat_sys_io_submit
-545 x32 execveat compat_sys_execveat
+545 x32 execveat sys_execveat
546 x32 preadv2 compat_sys_preadv64v2
547 x32 pwritev2 compat_sys_pwritev64v2
# This is the end of the legacy x32 range. Numbers 548 and above are
--
2.30.1

2021-03-26 14:43:05

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/4] exec: remove do_execve

Just call do_execveat instead.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/exec.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 18594f11c31fe1..b63fb020909075 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1978,15 +1978,6 @@ int kernel_execve(const char *kernel_filename,
return retval;
}

-static int do_execve(struct filename *filename,
- const char __user *const __user *__argv,
- const char __user *const __user *__envp)
-{
- struct user_arg_ptr argv = { .ptr.native = __argv };
- struct user_arg_ptr envp = { .ptr.native = __envp };
- return do_execveat_common(AT_FDCWD, filename, argv, envp, 0);
-}
-
static int do_execveat(int fd, struct filename *filename,
const char __user *const __user *__argv,
const char __user *const __user *__envp,
@@ -2060,7 +2051,7 @@ SYSCALL_DEFINE3(execve,
const char __user *const __user *, argv,
const char __user *const __user *, envp)
{
- return do_execve(getname(filename), argv, envp);
+ return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0);
}

SYSCALL_DEFINE5(execveat,
--
2.30.1

2021-03-26 14:43:21

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/4] exec: move the call to getname_flags into do_execveat

Remove the duplicated copying of the pathname into the common helper.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/exec.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index b34c1eb9e7ad8e..5c0dd8f85fe7b5 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1843,13 +1843,16 @@ static int bprm_execve(struct linux_binprm *bprm,
return retval;
}

-static int do_execveat(int fd, struct filename *filename,
+static int do_execveat(int fd, const char __user *pathname,
const char __user *const __user *argv,
const char __user *const __user *envp, int flags)
{
+ int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
+ struct filename *filename;
struct linux_binprm *bprm;
int retval;

+ filename = getname_flags(pathname, lookup_flags, NULL);
if (IS_ERR(filename))
return PTR_ERR(filename);

@@ -1993,7 +1996,7 @@ SYSCALL_DEFINE3(execve,
const char __user *const __user *, argv,
const char __user *const __user *, envp)
{
- return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0);
+ return do_execveat(AT_FDCWD, filename, argv, envp, 0);
}

SYSCALL_DEFINE5(execveat,
@@ -2002,9 +2005,5 @@ SYSCALL_DEFINE5(execveat,
const char __user *const __user *, envp,
int, flags)
{
- int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
-
- return do_execveat(fd,
- getname_flags(filename, lookup_flags, NULL),
- argv, envp, flags);
+ return do_execveat(fd, filename, argv, envp, flags);
}
--
2.30.1

2021-03-26 15:06:45

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/4] exec: simplify the compat syscall handling

On Fri, Mar 26, 2021 at 3:38 PM Christoph Hellwig <[email protected]> wrote:
>
> The only differenence betweeen the compat exec* syscalls and their
> native versions is the compat_ptr sign extension, and the fact that
> the pointer arithmetics for the two dimensional arrays needs to use
> the compat pointer size. Instead of the compat wrappers and the
> struct user_arg_ptr machinery just use in_compat_syscall() to do the
> right thing for the compat case deep inside get_user_arg_ptr().
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Nice cleanup!

Reviewed-by: Arnd Bergmann <[email protected]>

2021-03-26 15:09:02

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/4] exec: remove do_execve

On Fri, Mar 26, 2021 at 3:38 PM Christoph Hellwig <[email protected]> wrote:
>
> Just call do_execveat instead.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Reviewed-by: Arnd Bergmann <[email protected]>

2021-03-26 15:15:17

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 4/4] exec: move the call to getname_flags into do_execveat

On Fri, Mar 26, 2021 at 3:38 PM Christoph Hellwig <[email protected]> wrote:
>
> Remove the duplicated copying of the pathname into the common helper.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks correct, but

> -static int do_execveat(int fd, struct filename *filename,
> +static int do_execveat(int fd, const char __user *pathname,
> const char __user *const __user *argv,
> const char __user *const __user *envp, int flags)

Maybe rename this to ksys_execveat() for consistency now? I think that
is the current trend for functions that are essentially just the syscall.

With or without that change

Reviewed-by: Arnd Bergmann <[email protected]>

2021-03-26 16:14:35

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 3/4] exec: simplify the compat syscall handling

On Fri, Mar 26, 2021 at 03:38:30PM +0100, Christoph Hellwig wrote:

> -static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
> +static const char __user *
> +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> {
> - const char __user *native;
> -
> -#ifdef CONFIG_COMPAT
> - if (unlikely(argv.is_compat)) {
> + if (in_compat_syscall()) {
> + const compat_uptr_t __user *compat_argv =
> + compat_ptr((unsigned long)argv);
> compat_uptr_t compat;
>
> - if (get_user(compat, argv.ptr.compat + nr))
> + if (get_user(compat, compat_argv + nr))
> return ERR_PTR(-EFAULT);
> -
> return compat_ptr(compat);
> - }
> -#endif
> -
> - if (get_user(native, argv.ptr.native + nr))
> - return ERR_PTR(-EFAULT);
> + } else {
> + const char __user *native;
>
> - return native;
> + if (get_user(native, argv + nr))
> + return ERR_PTR(-EFAULT);
> + return native;
> + }
> }

Yecchhh.... So you have in_compat_syscall() called again and again, for
each argument in the list? I agree that current version is fucking ugly,
but I really hate that approach ;-/

2021-03-26 16:46:33

by David Laight

[permalink] [raw]
Subject: RE: [PATCH 3/4] exec: simplify the compat syscall handling

From: Al Viro
> Sent: 26 March 2021 16:12
>
> On Fri, Mar 26, 2021 at 03:38:30PM +0100, Christoph Hellwig wrote:
>
> > +static const char __user *
> > +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> > {
> > + if (in_compat_syscall()) {
> > + const compat_uptr_t __user *compat_argv =
> > + compat_ptr((unsigned long)argv);
> > compat_uptr_t compat;
> >
> > + if (get_user(compat, compat_argv + nr))
> > return ERR_PTR(-EFAULT);
> > return compat_ptr(compat);
> > + } else {
> > + const char __user *native;
> >
> > + if (get_user(native, argv + nr))
> > + return ERR_PTR(-EFAULT);
> > + return native;
> > + }
> > }
>
> Yecchhh.... So you have in_compat_syscall() called again and again, for
> each argument in the list? I agree that current version is fucking ugly,
> but I really hate that approach ;-/

Especially since in_compat_syscall() isn't entirely trivial on x86-64.
Probably all in the noise for 'exec', but all the bits do add up.

You may not want separate get_user() on some architectures either.
The user_access_begin/end aren't cheap.

OTOH if you call copy_from_user() you get hit by the stupid
additional costs of 'user copy hardening'.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2021-03-26 21:25:17

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH 3/4] exec: simplify the compat syscall handling

Christoph Hellwig <[email protected]> writes:


> diff --git a/fs/exec.c b/fs/exec.c
> index 06e07278b456fa..b34c1eb9e7ad8e 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -391,47 +391,34 @@ static int bprm_mm_init(struct linux_binprm *bprm)
> return err;
> }
>
> -struct user_arg_ptr {
> -#ifdef CONFIG_COMPAT
> - bool is_compat;
> -#endif
> - union {
> - const char __user *const __user *native;
> -#ifdef CONFIG_COMPAT
> - const compat_uptr_t __user *compat;
> -#endif
> - } ptr;
> -};
> -
> -static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
> +static const char __user *
> +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> {
> - const char __user *native;
> -
> -#ifdef CONFIG_COMPAT
> - if (unlikely(argv.is_compat)) {
> + if (in_compat_syscall()) {
> + const compat_uptr_t __user *compat_argv =
> + compat_ptr((unsigned long)argv);

Ouch! Passing a pointer around as the wrong type through the kernel!

Perhaps we should reduce everything to do_execveat and
do_execveat_compat. Then there would be no need for anything
to do anything odd with the pointer types.

I think the big change would be to factor out a copy_string out
of copy_strings, that performs all of the work once we know the proper
pointer value.

Casting pointers from one type to another scares me as one mistake means
we are doing something wrong and probably exploitable.


Eric




> compat_uptr_t compat;
>
> - if (get_user(compat, argv.ptr.compat + nr))
> + if (get_user(compat, compat_argv + nr))
> return ERR_PTR(-EFAULT);
> -
> return compat_ptr(compat);
> - }
> -#endif
> -
> - if (get_user(native, argv.ptr.native + nr))
> - return ERR_PTR(-EFAULT);
> + } else {
> + const char __user *native;
>
> - return native;
> + if (get_user(native, argv + nr))
> + return ERR_PTR(-EFAULT);
> + return native;
> + }
> }
>