From 3d729deaf287c43e415c5d791c9ac8414dbeff70 Mon Sep 17 00:00:00 2001 From: James Hogan Date: Fri, 11 Aug 2017 21:56:50 +0100 Subject: MIPS: seccomp: Fix indirect syscall args Since commit 669c4092225f ("MIPS: Give __secure_computing() access to syscall arguments."), upon syscall entry when seccomp is enabled, syscall_trace_enter() passes a carefully prepared struct seccomp_data containing syscall arguments to __secure_computing(). Unfortunately it directly uses mips_get_syscall_arg() and fails to take into account the indirect O32 system calls (i.e. syscall(2)) which put the system call number in a0 and have the arguments shifted up by one entry. We can't just revert that commit as samples/bpf/tracex5 would break again, so use syscall_get_arguments() which already takes indirect syscalls into account instead of directly using mips_get_syscall_arg(), similar to what populate_seccomp_data() does. This also removes the redundant error checking of the mips_get_syscall_arg() return value (get_user() already zeroes the result if an argument from the stack can't be loaded). Reported-by: James Cowgill Fixes: 669c4092225f ("MIPS: Give __secure_computing() access to syscall arguments.") Signed-off-by: James Hogan Reviewed-by: Kees Cook Cc: David Daney Cc: Andy Lutomirski Cc: Will Drewry Cc: Oleg Nesterov Cc: Alexei Starovoitov Cc: Daniel Borkmann Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-mips@linux-mips.org Patchwork: https://patchwork.linux-mips.org/patch/16994/ Signed-off-by: Ralf Baechle --- arch/mips/kernel/ptrace.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c index 6dd13641a418..1395654cfc8d 100644 --- a/arch/mips/kernel/ptrace.c +++ b/arch/mips/kernel/ptrace.c @@ -872,15 +872,13 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall) if (unlikely(test_thread_flag(TIF_SECCOMP))) { int ret, i; struct seccomp_data sd; + unsigned long args[6]; sd.nr = syscall; sd.arch = syscall_get_arch(); - for (i = 0; i < 6; i++) { - unsigned long v, r; - - r = mips_get_syscall_arg(&v, current, regs, i); - sd.args[i] = r ? 0 : v; - } + syscall_get_arguments(current, regs, 0, 6, args); + for (i = 0; i < 6; i++) + sd.args[i] = args[i]; sd.instruction_pointer = KSTK_EIP(current); ret = __secure_computing(&sd); -- cgit From 5af2ed36697e2a48cd7d36232212caa6240fe9bb Mon Sep 17 00:00:00 2001 From: James Cowgill Date: Fri, 31 Mar 2017 17:09:59 +0100 Subject: MIPS: Remove pt_regs adjustments in indirect syscall handler If a restartable syscall is called using the indirect o32 syscall handler - eg: syscall(__NR_waitid, ...), then it is possible for the incorrect arguments to be passed to the syscall after it has been restarted. This is because the syscall handler tries to shift all the registers down one place in pt_regs so that when the syscall is restarted, the "real" syscall is called instead. Unfortunately it only shifts the arguments passed in registers, not the arguments on the user stack. This causes the 4th argument to be duplicated when the syscall is restarted. Fix by removing all the pt_regs shifting so that the indirect syscall handler is called again when the syscall is restarted. The comment "some syscalls like execve get their arguments from struct pt_regs" is long out of date so this should now be safe. Signed-off-by: James Cowgill Reviewed-by: James Hogan Tested-by: James Hogan Cc: linux-mips@linux-mips.org Patchwork: https://patchwork.linux-mips.org/patch/15856/ Signed-off-by: Ralf Baechle --- arch/mips/kernel/scall32-o32.S | 11 ----------- arch/mips/kernel/scall64-o32.S | 6 ------ 2 files changed, 17 deletions(-) diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S index 27c2f90eeb21..a9a7d78803cd 100644 --- a/arch/mips/kernel/scall32-o32.S +++ b/arch/mips/kernel/scall32-o32.S @@ -190,12 +190,6 @@ illegal_syscall: sll t1, t0, 2 beqz v0, einval lw t2, sys_call_table(t1) # syscall routine - sw a0, PT_R2(sp) # call routine directly on restart - - /* Some syscalls like execve get their arguments from struct pt_regs - and claim zero arguments in the syscall table. Thus we have to - assume the worst case and shuffle around all potential arguments. - If you want performance, don't use indirect syscalls. */ move a0, a1 # shift argument registers move a1, a2 @@ -207,11 +201,6 @@ illegal_syscall: sw t4, 16(sp) sw t5, 20(sp) sw t6, 24(sp) - sw a0, PT_R4(sp) # .. and push back a0 - a3, some - sw a1, PT_R5(sp) # syscalls expect them there - sw a2, PT_R6(sp) - sw a3, PT_R7(sp) - sw a3, PT_R26(sp) # update a3 for syscall restarting jr t2 /* Unreached */ diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S index c30bc520885f..9ebe3e2403b1 100644 --- a/arch/mips/kernel/scall64-o32.S +++ b/arch/mips/kernel/scall64-o32.S @@ -198,7 +198,6 @@ LEAF(sys32_syscall) dsll t1, t0, 3 beqz v0, einval ld t2, sys32_call_table(t1) # syscall routine - sd a0, PT_R2(sp) # call routine directly on restart move a0, a1 # shift argument registers move a1, a2 @@ -207,11 +206,6 @@ LEAF(sys32_syscall) move a4, a5 move a5, a6 move a6, a7 - sd a0, PT_R4(sp) # ... and push back a0 - a3, some - sd a1, PT_R5(sp) # syscalls expect them there - sd a2, PT_R6(sp) - sd a3, PT_R7(sp) - sd a3, PT_R26(sp) # update a3 for syscall restarting jr t2 /* Unreached */ -- cgit From 2c0e8382386f618c85d20cb05e7cf7df8cdd382c Mon Sep 17 00:00:00 2001 From: James Hogan Date: Sat, 12 Aug 2017 21:36:09 -0700 Subject: irqchip: mips-gic: SYNC after enabling GIC region A SYNC is required between enabling the GIC region and actually trying to use it, even if the first access is a read, otherwise its possible depending on the timing (and in my case depending on the precise alignment of certain kernel code) to hit CM bus errors on that first access. Add the SYNC straight after setting the GIC base. [paul.burton@imgtec.com: Changes later in this series increase our likelihood of hitting this by reducing the amount of code that runs between enabling the GIC & accessing it.] Fixes: a7057270c280 ("irqchip: mips-gic: Add device-tree support") Signed-off-by: James Hogan Signed-off-by: Paul Burton Acked-by: Marc Zyngier Cc: Thomas Gleixner Cc: Jason Cooper Cc: James Hogan Cc: linux-kernel@vger.kernel.org Cc: linux-mips@linux-mips.org Cc: # 3.19.x- Patchwork: https://patchwork.linux-mips.org/patch/17019/ Signed-off-by: Ralf Baechle --- drivers/irqchip/irq-mips-gic.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c index 6ab1d3afec02..48ee1bad473f 100644 --- a/drivers/irqchip/irq-mips-gic.c +++ b/drivers/irqchip/irq-mips-gic.c @@ -1020,8 +1020,11 @@ static int __init gic_of_init(struct device_node *node, gic_len = resource_size(&res); } - if (mips_cm_present()) + if (mips_cm_present()) { write_gcr_gic_base(gic_base | CM_GCR_GIC_BASE_GICEN_MSK); + /* Ensure GIC region is enabled before trying to access it */ + __sync(); + } gic_present = true; __gic_init(gic_base, gic_len, cpu_vec, 0, node); -- cgit