| Message ID | 1452209679-19445-15-git-send-email-ynorov@caviumnetworks.com |
|---|---|
| State | Superseded |
| Headers | show |
On Friday 08 January 2016 02:34:32 Yury Norov wrote: > +asmlinkage long sys_mmap(unsigned long addr, unsigned long len, > + unsigned long prot, unsigned long flags, > + unsigned long fd, off_t off); > +#define sys_mmap2 sys_mmap > Why do you need a separate declaration for sys_mmap here? There is one in include/asm-generic/syscalls.h that should be visible in this file. Arnd
On Friday 08 January 2016 02:34:32 Yury Norov wrote: > @@ -688,6 +692,12 @@ ni_sys: > b ret_fast_syscall > ENDPROC(el0_svc) > > +#ifdef CONFIG_ARM64_ILP32 > +el0_ilp32_svc: > + adrp stbl, sys_call_ilp32_table // load syscall table pointer > + b el0_svc_naked > +#endif Don't we still need some code that clears the top halves of the 32-bit arguments? That thread has taken so many turns now that I'm confused about what we actually need, but I thought we had concluded that your current approach has at some some problems. > +#include <asm/syscall.h> > + > +#undef __SYSCALL > +#undef __SC_COMP > +#undef __SC_3264 > +#undef __SC_COMP_3264 The four #undef are not needed, right? Arnd
On Fri, Jan 08, 2016 at 10:21:06AM +0100, Arnd Bergmann wrote: > On Friday 08 January 2016 02:34:32 Yury Norov wrote: > > > @@ -688,6 +692,12 @@ ni_sys: > > b ret_fast_syscall > > ENDPROC(el0_svc) > > > > +#ifdef CONFIG_ARM64_ILP32 > > +el0_ilp32_svc: > > + adrp stbl, sys_call_ilp32_table // load syscall table pointer > > + b el0_svc_naked > > +#endif > > Don't we still need some code that clears the top halves of the 32-bit > arguments? That thread has taken so many turns now that I'm confused > about what we actually need, but I thought we had concluded that your > current approach has at some some problems. We are discussing how to do it better - make a generic solution from s390 with individual syscall handling, or reproduce s390 solution for ILP32, or zero top-half registers and not use top half of register at all. As I understand, we stand on 1st option, and agreed to introduce it separately. > > > +#include <asm/syscall.h> > > + > > +#undef __SYSCALL > > +#undef __SC_COMP > > +#undef __SC_3264 > > +#undef __SC_COMP_3264 > > The four #undef are not needed, right? > > Arnd
On Friday 08 January 2016 14:13:18 Yury Norov wrote: > On Fri, Jan 08, 2016 at 10:21:06AM +0100, Arnd Bergmann wrote: > > On Friday 08 January 2016 02:34:32 Yury Norov wrote: > > > > > @@ -688,6 +692,12 @@ ni_sys: > > > b ret_fast_syscall > > > ENDPROC(el0_svc) > > > > > > +#ifdef CONFIG_ARM64_ILP32 > > > +el0_ilp32_svc: > > > + adrp stbl, sys_call_ilp32_table // load syscall table pointer > > > + b el0_svc_naked > > > +#endif > > > > Don't we still need some code that clears the top halves of the 32-bit > > arguments? That thread has taken so many turns now that I'm confused > > about what we actually need, but I thought we had concluded that your > > current approach has at some some problems. > > We are discussing how to do it better - make a generic solution from > s390 with individual syscall handling, or reproduce s390 solution for > ILP32, or zero top-half registers and not use top half of register at > all. As I understand, we stand on 1st option, and agreed to introduce > it separately. Ok. At some point I thought there was a security hole if we don't do the explicit expansion, but now I can't remember what I was thinking of or if that was actually real. Let me try to recall my understanding: We know that the existing DEFINE_SYSCALL wrappers work fine for 32-bit (__u32, int, unsigned int, ...) or smaller (char, short, __u8, __u16, ...) arguments as well as the explicitly 64-bit arguments (loff_t, __u64, __s64). Entering a syscall with a 'long' (or size_t, pointer, ...) argument means that user space expects the kernel to ignore the upper half, but the kernel will treat it as part of the register. This means anything we pass into the kernel will follow the ELF ABI for function calls, and I don't see a security problem here either, just the question of how the ABI should be defined. (sorry for the excursion, I needed to write that down to get my own thoughts sorted) I still think that the first option from Catalin's email is best here, and it would be good if you could implement that so we can see if there are any complications we have not thought of yet. Arnd
On Fri, Jan 08, 2016 at 10:21:06AM +0100, Arnd Bergmann wrote: > On Friday 08 January 2016 02:34:32 Yury Norov wrote: > > > @@ -688,6 +692,12 @@ ni_sys: > > b ret_fast_syscall > > ENDPROC(el0_svc) > > > > +#ifdef CONFIG_ARM64_ILP32 > > +el0_ilp32_svc: > > + adrp stbl, sys_call_ilp32_table // load syscall table pointer > > + b el0_svc_naked > > +#endif > > Don't we still need some code that clears the top halves of the 32-bit > arguments? That thread has taken so many turns now that I'm confused > about what we actually need, but I thought we had concluded that your > current approach has at some some problems. > > > +#include <asm/syscall.h> > > + > > +#undef __SYSCALL > > +#undef __SC_COMP > > +#undef __SC_3264 > > +#undef __SC_COMP_3264 > > The four #undef are not needed, right? > > Arnd No, removing any of them follows compilation problems.
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 ad7158c..a35f2f8 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_PERF_EVENTS) += perf_regs.o perf_callchain.o diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index 5eb1bb7..f348f58 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -666,9 +666,13 @@ ENDPROC(ret_from_fork) */ .align 6 el0_svc: - adrp stbl, sys_call_table // load syscall table pointer uxtw scno, w8 // syscall number in w8 mov sc_nr, #__NR_syscalls +#ifdef CONFIG_ARM64_ILP32 + ldr x16, [tsk, #TI_FLAGS] + tbnz x16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32 +#endif + adrp stbl, sys_call_table // load syscall table pointer el0_svc_naked: // compat entry point stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number enable_dbg_and_irq @@ -688,6 +692,12 @@ ni_sys: b ret_fast_syscall ENDPROC(el0_svc) +#ifdef CONFIG_ARM64_ILP32 +el0_ilp32_svc: + adrp stbl, sys_call_ilp32_table // load syscall table pointer + b el0_svc_naked +#endif + /* * This is the really slow path. We're going to be doing context * switches, and waiting for our parent to respond. diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c new file mode 100644 index 0000000..71912b0 --- /dev/null +++ b/arch/arm64/kernel/sys_ilp32.c @@ -0,0 +1,66 @@ +/* + * 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/>. + */ + +#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> + +/* 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_pread64 sys_pread64 +#define compat_sys_pwrite64 sys_pwrite64 +#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 + +asmlinkage long sys_mmap(unsigned long addr, unsigned long len, + unsigned long prot, unsigned long flags, + unsigned long fd, off_t off); +#define sys_mmap2 sys_mmap + +#include <asm/syscall.h> + +#undef __SYSCALL +#undef __SC_COMP +#undef __SC_3264 +#undef __SC_COMP_3264 + +#define __SYSCALL_COMPAT +#define __SYSCALL(nr, sym) [nr] = 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> +};