[PATCH v6 20/21] all: s390: make compat wrappers the generic solution
From: Heiko Carstens <hidden>
Date: 2016-01-15 12:46:57
Also in:
lkml
On Thu, Jan 14, 2016 at 08:23:17PM +0300, Yury Norov wrote:
The problem that makes us to use wrappers is that some compat architectures allows user code to access top halves of registers. This is not a problem for syscalls that are already handled by compat code, or for that who has types of the same size in kernel and userspace. In case of lp64/ilp32 the problem is in pointer types, long, unsigned long. S390 folks already have the solution for it. In this patch, it is turned to be general, as arm64/ilp32 needs it too. Build-tested on s390.
If you want to make this generic then I think the footprint of the compat wrapper infrastructure should be as small as possible:
quoted hunk ↗ jump to hunk
diff --git a/arch/s390/include/asm/compat_wrapper.h b/arch/s390/include/asm/compat_wrapper.h new file mode 100644 index 0000000..d5a5c36 --- /dev/null +++ b/arch/s390/include/asm/compat_wrapper.h@@ -0,0 +1,25 @@ +#ifndef __ASM_COMPAT_WRAPPER +#define __ASM_COMPAT_WRAPPER + +/* + * Compat system call wrappers. + * + * Copyright IBM Corp. 2014 + */ + +#define __SC_COMPAT_CAST(t, a) \ +({ \ + long __ReS = a; \ + \ + BUILD_BUG_ON((sizeof(t) > 4) && !__TYPE_IS_L(t) && \ + !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t)); \ + if (__TYPE_IS_L(t)) \ + __ReS = (s32)a; \ + if (__TYPE_IS_UL(t)) \ + __ReS = (u32)a; \ + if (__TYPE_IS_PTR(t)) \ + __ReS = a & 0x7fffffff; \ + (t)__ReS; \ +}) + +#endif /* __ASM_COMPAT_WRAPPER */
This should go to arch/s390/include/asm/compat.h right below Al's __SC_DELOUSE. (no new header file please)
quoted hunk ↗ jump to hunk
diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c index 437e611..22416a8 100644 --- a/arch/s390/kernel/compat_linux.c +++ b/arch/s390/kernel/compat_linux.c@@ -44,6 +44,7 @@ #include <linux/binfmts.h> #include <linux/capability.h> #include <linux/compat.h> +#include <linux/compat_wrapper.h> #include <linux/vfs.h> #include <linux/ptrace.h> #include <linux/fadvise.h>@@ -86,6 +87,11 @@ #define SET_STAT_UID(stat, uid) (stat).st_uid = high2lowuid(uid) #define SET_STAT_GID(stat, gid) (stat).st_gid = high2lowgid(gid) +COMPAT_SYSCALL_WRAP3(s390_pci_mmio_write, const unsigned long, + mmio_addr, const void __user *, user_buffer, const size_t, length); +COMPAT_SYSCALL_WRAP3(s390_pci_mmio_read, const unsigned long, + mmio_addr, void __user *, user_buffer, const size_t, length); +
I'd really prefer if the existing common SYSCALL_DEFINE macros could be extended to also generate the compat wrappers, if needed. This probably means that you need another set of SYSCALL_DEFINE macros, e.g. something like SYSCALL_COMPAT, which is defined the same like SYSCALL_DEFINE, except that it also generates the compat_wrapper functions. Since the list of system calls that need a wrapper is already present in s390's compat_wrapper.c file it's "only" a matter of writing a script that converts the required SYSCALL_DEFINEs to SYSYCALL_COMPATs.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/compat_wrapper.h b/include/linux/compat_wrapper.h new file mode 100644 index 0000000..6f22732 --- /dev/null +++ b/include/linux/compat_wrapper.h@@ -0,0 +1,270 @@ +#ifndef __COMPAT_WRAPPER +#define __COMPAT_WRAPPER + +#include <asm/compat_wrapper.h> + +#define COMPAT_SYSCALL_WRAP1(name, ...) \ + COMPAT_SYSCALL_WRAPx(1, _##name, __VA_ARGS__) +#define COMPAT_SYSCALL_WRAP2(name, ...) \ + COMPAT_SYSCALL_WRAPx(2, _##name, __VA_ARGS__) +#define COMPAT_SYSCALL_WRAP3(name, ...) \ + COMPAT_SYSCALL_WRAPx(3, _##name, __VA_ARGS__) +#define COMPAT_SYSCALL_WRAP4(name, ...) \ + COMPAT_SYSCALL_WRAPx(4, _##name, __VA_ARGS__) +#define COMPAT_SYSCALL_WRAP5(name, ...) \ + COMPAT_SYSCALL_WRAPx(5, _##name, __VA_ARGS__) +#define COMPAT_SYSCALL_WRAP6(name, ...) \ + COMPAT_SYSCALL_WRAPx(6, _##name, __VA_ARGS__) + +#ifndef __SC_COMPAT_TYPE +#define __SC_COMPAT_TYPE(t, a) \ + __typeof(__builtin_choose_expr(sizeof(t) > 4, 0L, (t)0)) a +#endif + +#ifndef __SC_COMPAT_CAST +#define __SC_COMPAT_CAST(t, a) ((t)(long) ((t)(-1) < 0 ? (s32)(a) : (u32)(a))) +#endif +/* + * The COMPAT_SYSCALL_WRAP macro generates system call wrappers to be used by + * compat tasks. These wrappers will only be used for system calls where only + * the system call arguments need sign or zero extension or zeroing of the upper + * 33 bits of pointers. + * Note: since the wrapper function will afterwards call a system call which + * again performs zero and sign extension for all system call arguments with + * a size of less than eight bytes, these compat wrappers only touch those + * system call arguments with a size of eight bytes ((unsigned) long and + * pointers). Zero and sign extension for e.g. int parameters will be done by + * the regular system call wrappers. + */ +#ifndef COMPAT_SYSCALL_WRAPx +#define COMPAT_SYSCALL_WRAPx(x, name, ...) \ +asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)); \ +asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ + __attribute__((alias(__stringify(compat_SyS##name)))); \ +asmlinkage long notrace compat_SyS##name(__MAP(x,__SC_COMPAT_TYPE,__VA_ARGS__)); \ +asmlinkage long notrace compat_SyS##name(__MAP(x,__SC_COMPAT_TYPE,__VA_ARGS__)) \ +{ \ + return sys##name(__MAP(x,__SC_COMPAT_CAST,__VA_ARGS__)); \ +} +#endif
All of this should go to include/linux/compat.h
+asmlinkage long compat_sys_creat(const char __user *pathname, umode_t mode); +asmlinkage long compat_sys_link(const char __user *oldname, + const char __user *newname); +asmlinkage long compat_sys_chdir(const char __user *filename); +asmlinkage long compat_sys_mknod(const char __user *filename, umode_t mode, + unsigned dev);
Are these really needed?
quoted hunk ↗ jump to hunk
diff --git a/kernel/compat_wrapper.c b/kernel/compat_wrapper.c new file mode 100644 index 0000000..4a99811 --- /dev/null +++ b/kernel/compat_wrapper.c@@ -0,0 +1,167 @@ +/* + * Compat system call wrappers. + * + * Copyright IBM Corp. 2014 + */ + +#include <linux/syscalls.h> +#include <linux/compat.h> +#include <linux/compat_wrapper.h> + +COMPAT_SYSCALL_WRAP2(creat, const char __user *, pathname, umode_t, mode); +COMPAT_SYSCALL_WRAP2(link, const char __user *, oldname, const char __user *, newname); +COMPAT_SYSCALL_WRAP1(unlink, const char __user *, pathname); +COMPAT_SYSCALL_WRAP1(chdir, const char __user *, filename); +COMPAT_SYSCALL_WRAP3(mknod, const char __user *, filename, umode_t, mode, unsigned, dev); +COMPAT_SYSCALL_WRAP2(chmod, const char __user *, filename, umode_t, mode);
With all the above changes this file wouldn't be necessary, which I think would be good. Just my thoughts...