| Message ID | 20160514150352.GA30533@yury-N73SV (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote: > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > + unsigned long, prot, unsigned long, flags, unsigned long, fd, > + unsigned long, pgoff) To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.). > +{ > + if (pgoff & (~PAGE_MASK >> 12)) > + return -EINVAL; > + > + return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len, > + (int) prot, (int) flags, (int) fd, > + pgoff >> (PAGE_SHIFT-12)); Then we wouldn't need the explicit casting here. > +} > + > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf, > + compat_size_t, count, off_t, offset) > +{ > + return sys_pread64(fd, (char *) ubuf, count, offset); > +} > + > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf, > + compat_size_t, count, off_t, offset) > +{ > + return sys_pwrite64(fd, (char *) ubuf, count, offset); Nitpick: no space between cast type and variable name: (char *)ubuf, ... We can also make these functions static as they are not used outside this file.
On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote: > On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote: > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > > + unsigned long, prot, unsigned long, flags, unsigned long, fd, > > + unsigned long, pgoff) > > To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h > and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.). > > > +{ > > + if (pgoff & (~PAGE_MASK >> 12)) > > + return -EINVAL; > > + > > + return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len, > > + (int) prot, (int) flags, (int) fd, > > + pgoff >> (PAGE_SHIFT-12)); > > Then we wouldn't need the explicit casting here. See below > > > +} > > + > > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf, > > + compat_size_t, count, off_t, offset) > > +{ > > + return sys_pread64(fd, (char *) ubuf, count, offset); > > +} > > + > > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf, > > + compat_size_t, count, off_t, offset) > > +{ > > + return sys_pwrite64(fd, (char *) ubuf, count, offset); > > Nitpick: no space between cast type and variable name: (char *)ubuf, ... I think it's really a matter of taste. I prefer to have a space, and there's no solid rule in coding style. And there are 13032 insertions of my version vs 35030 of yours: ~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l 35030 ~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l 13032 Of course, I will change it if you insist. > We can also make these functions static as they are not used outside > this file. If it's OK, we can use just compat_sys_xxx() instead of COMPAT_SYSCALL_DEFINEx(xxx), and for mmap2 we'll not need to change _SYSCALL to _SC_WRAP in unistd.h that way. > > -- > Catalin
On Tue, May 17, 2016 at 10:05:26PM +0300, Yury Norov wrote: > On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote: > > On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote: > > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > > > + unsigned long, prot, unsigned long, flags, unsigned long, fd, > > > + unsigned long, pgoff) > > > > To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h > > and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.). > > > > > +{ > > > + if (pgoff & (~PAGE_MASK >> 12)) > > > + return -EINVAL; > > > + > > > + return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len, > > > + (int) prot, (int) flags, (int) fd, > > > + pgoff >> (PAGE_SHIFT-12)); > > > > Then we wouldn't need the explicit casting here. > > See below > > > > +} > > > + > > > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf, > > > + compat_size_t, count, off_t, offset) > > > +{ > > > + return sys_pread64(fd, (char *) ubuf, count, offset); > > > +} > > > + > > > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf, > > > + compat_size_t, count, off_t, offset) > > > +{ > > > + return sys_pwrite64(fd, (char *) ubuf, count, offset); > > > > Nitpick: no space between cast type and variable name: (char *)ubuf, ... > > I think it's really a matter of taste. I prefer to have a space, and > there's no solid rule in coding style. > > And there are 13032 insertions of my version vs 35030 of yours: > ~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l > 35030 > ~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l > 13032 > > Of course, I will change it if you insist. Not really, I thought it's covered by CodingStyle but it doesn't seem to. > > We can also make these functions static as they are not used outside > > this file. > > If it's OK, we can use just compat_sys_xxx() instead of > COMPAT_SYSCALL_DEFINEx(xxx), I got lost in macros, what difference would COMPAT_SYSCALL_DEFINE vs compat_sys_*() make? Is this the delouse stuff? > and for mmap2 we'll not need to change _SYSCALL to _SC_WRAP in > unistd.h that way. Looking at the generic unistd.h, adding _SC_WRAP for sys_mmap2 is indeed not easy: #define __NR3264_mmap 222 __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) So defining a compat_sys_mmap2() would work but I think you'd also need: #define sys_mmap2 compat_sys_mmap2()
On Wed, May 18, 2016 at 12:21:46PM +0100, Catalin Marinas wrote: > On Tue, May 17, 2016 at 10:05:26PM +0300, Yury Norov wrote: > > On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote: > > > On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote: > > > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > > > > + unsigned long, prot, unsigned long, flags, unsigned long, fd, > > > > + unsigned long, pgoff) > > > > > > To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h > > > and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.). > > > > > > > +{ > > > > + if (pgoff & (~PAGE_MASK >> 12)) > > > > + return -EINVAL; > > > > + > > > > + return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len, > > > > + (int) prot, (int) flags, (int) fd, > > > > + pgoff >> (PAGE_SHIFT-12)); > > > > > > Then we wouldn't need the explicit casting here. > > > > See below > > > > > > +} > > > > + > > > > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf, > > > > + compat_size_t, count, off_t, offset) > > > > +{ > > > > + return sys_pread64(fd, (char *) ubuf, count, offset); > > > > +} > > > > + > > > > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf, > > > > + compat_size_t, count, off_t, offset) > > > > +{ > > > > + return sys_pwrite64(fd, (char *) ubuf, count, offset); > > > > > > Nitpick: no space between cast type and variable name: (char *)ubuf, ... > > > > I think it's really a matter of taste. I prefer to have a space, and > > there's no solid rule in coding style. > > > > And there are 13032 insertions of my version vs 35030 of yours: > > ~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l > > 35030 > > ~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l > > 13032 > > > > Of course, I will change it if you insist. > > Not really, I thought it's covered by CodingStyle but it doesn't seem > to. > > > > We can also make these functions static as they are not used outside > > > this file. > > > > If it's OK, we can use just compat_sys_xxx() instead of > > COMPAT_SYSCALL_DEFINEx(xxx), > > I got lost in macros, what difference would COMPAT_SYSCALL_DEFINE vs > compat_sys_*() make? Is this the delouse stuff? > Hmm... I looked again. It seems that COMPAT delouses all arguments, including off_t, and that's what we try to avoid. So we *should* use naked functions. include/linux/compat.h: 53 #define COMPAT_SYSCALL_DEFINEx(x, name, ...) \ 54 asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\ 55 __attribute__((alias(__stringify(compat_SyS##name)))); \ 56 static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));\ 57 asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));\ 58 asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\ 59 { \ 60 return C_SYSC##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__)); \ 61 } \ 62 static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__)) > > and for mmap2 we'll not need to change _SYSCALL to _SC_WRAP in > > unistd.h that way. > > Looking at the generic unistd.h, adding _SC_WRAP for sys_mmap2 is > indeed not easy: > > #define __NR3264_mmap 222 > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) > > So defining a compat_sys_mmap2() would work but I think you'd also need: > > #define sys_mmap2 compat_sys_mmap2() OK. > > -- > Catalin
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h index 2971dea..5ea18ef 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -13,9 +13,18 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#ifdef CONFIG_COMPAT +#define __ARCH_WANT_COMPAT_STAT64 +#endif + +#ifdef CONFIG_ARM64_ILP32 +#define __ARCH_WANT_COMPAT_SYS_PREADV64 +#define __ARCH_WANT_COMPAT_SYS_PWRITEV64 +#endif + #ifdef CONFIG_AARCH32_EL0 #define __ARCH_WANT_COMPAT_SYS_GETDENTS64 -#define __ARCH_WANT_COMPAT_STAT64 #define __ARCH_WANT_SYS_GETHOSTNAME #define __ARCH_WANT_SYS_PAUSE #define __ARCH_WANT_SYS_GETPGRP diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 9dfdf86..7aa65ea 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -28,7 +28,7 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE arm64-obj-$(CONFIG_AARCH32_EL0) += sys32.o kuser32.o signal32.o \ sys_compat.o entry32.o \ ../../arm/kernel/opcodes.o binfmt_elf32.o -arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o +arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o sys_ilp32.o arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o arm64-obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index cf4d1ae..0f651bc 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -500,6 +500,7 @@ el0_svc_compat: * AArch32 syscall handling */ adrp stbl, compat_sys_call_table // load compat syscall table pointer + ldr x16, [tsk, #TI_FLAGS] uxtw scno, w7 // syscall number in w7 (r7) mov sc_nr, #__NR_compat_syscalls b el0_svc_naked @@ -716,15 +717,20 @@ ENDPROC(ret_from_fork) .align 6 el0_svc: adrp stbl, sys_call_table // load syscall table pointer + ldr x16, [tsk, #TI_FLAGS] uxtw scno, w8 // syscall number in w8 mov sc_nr, #__NR_syscalls +#ifdef CONFIG_ARM64_ILP32 + adrp x17, sys_call_ilp32_table // load ilp32 syscall table pointer + tst x16, #_TIF_32BIT_AARCH64 + csel stbl, stbl, x17, eq // We are using ILP32 +#endif el0_svc_naked: // compat entry point stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number enable_dbg_and_irq ct_user_exit 1 - ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks - tst x16, #_TIF_SYSCALL_WORK + tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks b.ne __sys_trace cmp scno, sc_nr // check upper syscall limit b.hs ni_sys diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c new file mode 100644 index 0000000..64db612 --- /dev/null +++ b/arch/arm64/kernel/sys_ilp32.c @@ -0,0 +1,84 @@ +/* + * AArch64- ILP32 specific system calls implementation + * + * Copyright (C) 2016 Cavium Inc. + * Author: Andrew Pinski <apinski@cavium.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#define __SYSCALL_COMPAT + +#include <linux/compiler.h> +#include <linux/errno.h> +#include <linux/fs.h> +#include <linux/mm.h> +#include <linux/msg.h> +#include <linux/export.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/syscalls.h> +#include <linux/compat.h> +#include <asm-generic/syscalls.h> + +/* Using non-compat syscalls where necessary */ +#define compat_sys_fadvise64_64 sys_fadvise64_64 +#define compat_sys_fallocate sys_fallocate +#define compat_sys_ftruncate64 sys_ftruncate +#define compat_sys_lookup_dcookie sys_lookup_dcookie +#define compat_sys_readahead sys_readahead +#define compat_sys_shmat sys_shmat +#define compat_sys_sync_file_range sys_sync_file_range +#define compat_sys_truncate64 sys_truncate +#define sys_llseek sys_lseek + +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, + unsigned long, prot, unsigned long, flags, unsigned long, fd, + unsigned long, pgoff) +{ + if (pgoff & (~PAGE_MASK >> 12)) + return -EINVAL; + + return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len, + (int) prot, (int) flags, (int) fd, + pgoff >> (PAGE_SHIFT-12)); +} + +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf, + compat_size_t, count, off_t, offset) +{ + return sys_pread64(fd, (char *) ubuf, count, offset); +} + +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf, + compat_size_t, count, off_t, offset) +{ + return sys_pwrite64(fd, (char *) ubuf, count, offset); +} + +#include <asm/syscall.h> + +#undef __SYSCALL +#undef __SC_WRAP + +#define __SYSCALL(nr, sym) [nr] = sym, +#define __SC_WRAP(nr, sym) [nr] = compat_##sym, + +/* + * The sys_call_ilp32_table array must be 4K aligned to be accessible from + * kernel/entry.S. + */ +void *sys_call_ilp32_table[__NR_syscalls] __aligned(4096) = { + [0 ... __NR_syscalls - 1] = sys_ni_syscall, +#include <asm/unistd.h> +};