From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:21:31
Currently on powerpc we have our own #define for the highest (negative)
errno value, called _LAST_ERRNO. This is defined to be 516, for reasons
which are not clear.
The generic code, and x86, use MAX_ERRNO, which is defined to be 4095.
In particular seccomp uses MAX_ERRNO to restrict the value that a
seccomp filter can return.
Currently with the mismatch between _LAST_ERRNO and MAX_ERRNO, a seccomp
tracer wanting to return 600, expecting it to be seen as an error, would
instead find on powerpc that userspace sees a successful syscall with a
return value of 600.
To avoid this inconsistency, switch powerpc to use MAX_ERRNO.
We are somewhat confident that generic syscalls that can return a
non-error value above negative MAX_ERRNO have already been updated to
use force_successful_syscall_return().
I have also checked all the powerpc specific syscalls, and believe that
none of them expect to return a non-error value between -MAX_ERRNO and
-516. So this change should be safe ...
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/uapi/asm/errno.h | 2 --
arch/powerpc/kernel/entry_32.S | 3 ++-
arch/powerpc/kernel/entry_64.S | 5 +++--
3 files changed, 5 insertions(+), 5 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:21:41
The API for calling do_syscall_trace_enter() is currently sensible
enough, it just returns the (modified) syscall number.
However once we enable seccomp filter it will get more complicated. When
seccomp filter runs, the seccomp kernel code (via SECCOMP_RET_ERRNO), or
a ptracer (via SECCOMP_RET_TRACE), may reject the syscall and *may* or may
*not* set a return value in r3.
That means the assembler that calls do_syscall_trace_enter() can not
blindly return ENOSYS, it needs to only return ENOSYS if a return value
has not already been set.
There is no way to implement that logic with the current API. So change
the do_syscall_trace_enter() API to make it deal with the return code
juggling, and the assembler can then just return whatever return code it
is given.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/entry_32.S | 4 ++++
arch/powerpc/kernel/entry_64.S | 23 ++++++++++++++------
arch/powerpc/kernel/ptrace.c | 48 ++++++++++++++++++++++++++++++++----------
3 files changed, 58 insertions(+), 17 deletions(-)
@@ -1762,26 +1762,42 @@ long arch_ptrace(struct task_struct *child, long request,returnret;}-/*-*Wemustreturnthesyscallnumbertoactuallylookupinthetable.-*Thiscanbe-1Ltoskiprunninganysyscallatall.+/**+*do_syscall_trace_enter()-Dosyscalltracingonkernelentry.+*@regs:thept_regsofthetasktotrace(current)+*+*Performsvarioustypesoftracingonsyscallentry.Thisincludesseccomp,+*ptrace,syscalltracepointsandaudit.+*+*Thept_regsarepotentiallyvisibletouserspaceviaptrace,sotheir+*contentsisABI.+*+*Oneormoreofthetracersmaymodifythecontentsofpt_regs,inparticular+*tomodifyargumentsoreventhesyscallnumberitself.+*+*It'salsopossiblethatatracercanchoosetorejectthesystemcall.In+*thatcasethisfunctionwillreturnanillegalsyscallnumber,andwillput+*anappropriatereturnvalueinregs->r3.+*+*Return:the(possiblychanged)syscallnumber.*/longdo_syscall_trace_enter(structpt_regs*regs){-longret=0;+boolabort=false;user_exit();secure_computing_strict(regs->gpr[0]);-if(test_thread_flag(TIF_SYSCALL_TRACE)&&-tracehook_report_syscall_entry(regs))+if(test_thread_flag(TIF_SYSCALL_TRACE)){/*-*Tracingdecidedthissyscallshouldnothappen.-*We'llreturnaboguscallnumbertogetanENOSYS-*error,butleavetheoriginalnumberinregs->gpr[0].+*Thetracermaydecidetoabortthesyscall,ifsotracehook+*willreturn!0.Notethatthetracermayalsojustchange+*regs->gpr[0]toaninvalidsyscallnumber,thatishandled+*belowontheexitpath.*/-ret=-1L;+abort=tracehook_report_syscall_entry(regs)!=0;+}if(unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))trace_sys_enter(regs,regs->gpr[0]);
@@ -1798,7 +1814,17 @@ long do_syscall_trace_enter(struct pt_regs *regs)regs->gpr[5]&0xffffffff,regs->gpr[6]&0xffffffff);-returnret?:regs->gpr[0];+if(abort||regs->gpr[0]>=NR_syscalls){+/*+*Ifweareabortingexplicitly,orifthesyscallnumberis+*nowinvalid,setthereturnvalueto-ENOSYS.+*/+regs->gpr[3]=-ENOSYS;+return-1;+}++/* Return the possibly modified but valid syscall number */+returnregs->gpr[0];}voiddo_syscall_trace_leave(structpt_regs*regs)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:21:54
syscall_get_error() is unused, and never has been.
It's also probably wrong, as it negates r3 before returning it, but that
depends on what the caller is expecting.
It also doesn't deal with compat, and doesn't deal with TIF_NOERROR.
Although we could fix those, until it has a caller and it's clear what
semantics the caller wants it's just untested code. So drop it.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/syscall.h | 6 ------
1 file changed, 6 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:02
Currently the only caller of syscall_set_return_value() is seccomp
filter, which is not enabled on powerpc.
This means we have not noticed that our implementation of
syscall_set_return_value() negates error, even though the value passed
in is already negative.
So remove the negation in syscall_set_return_value(), and expect the
caller to do it like all other implementations do.
Also add a comment about the ccr handling.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/syscall.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:09
The documentation for syscall_get_nr() in asm-generic says:
Note this returns int even on 64-bit machines. Only 32 bits of
system call number can be meaningful. If the actual arch value
is 64 bits, this truncates to 32 bits so 0xffffffff means -1.
However our implementation was never updated to reflect this.
Generally it's not important, but there is once case where it matters.
For seccomp filter with SECCOMP_RET_TRACE, the tracer will set
regs->gpr[0] to -1 to reject the syscall. When the task is a compat
task, this means we end up with 0xffffffff in r0 because ptrace will
zero extend the 32-bit value.
If syscall_get_nr() returns an unsigned long, then a 64-bit kernel will
see a positive value in r0 and will incorrectly allow the syscall
through seccomp.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/syscall.h | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:14
Currently syscall_get_arguments() is used by syscall tracepoints, and
collect_syscall() which is used in some debugging as well as
/proc/pid/syscall.
The current implementation just copies regs->gpr[3 .. 5] out, which is
fine for all the current use cases.
When we enable seccomp filter, that will also start using
syscall_get_arguments(). However for seccomp filter we want to use r3
as the return value of the syscall, and orig_gpr3 as the first
parameter. This will allow seccomp to modify the return value in r3.
To support this we need to modify syscall_get_arguments() to return
orig_gpr3 instead of r3. This is safe for all uses because orig_gpr3
always contains the r3 value that was passed to the syscall. We store it
in the syscall entry path and never modify it.
Update syscall_set_arguments() while we're here, even though it's never
used.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/syscall.h | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
@@ -83,6 +89,10 @@ static inline void syscall_set_arguments(struct task_struct *task,{BUG_ON(i+n>6);memcpy(®s->gpr[3+i],args,n*sizeof(args[0]));++/* Also copy the first argument into orig_gpr3 */+if(i==0&&n>0)+regs->orig_gpr3=args[0];}staticinlineintsyscall_get_arch(void)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:18
The seccomp_bpf test uses BPF_LD|BPF_W|BPF_ABS to load 32-bit values
from seccomp_data->args. On big endian machines this will load the high
word of the argument, which is not what the test wants.
Borrow a hack from samples/seccomp/bpf-helper.h which changes the offset
on big endian to account for this.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/seccomp/seccomp_bpf.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:32
SIG_SYS was added in commit a0727e8ce513 "signal, x86: add SIGSYS info
and make it synchronous."
Because we use the asm-generic struct siginfo, we got support for
SIG_SYS for free as part of that commit.
However there was no compat handling added for powerpc. That means we've
been advertising the existence of signfo._sifields._sigsys to compat
tasks, but not actually filling in the fields correctly.
Luckily it looks like no one has noticed, presumably because the only
user of SIGSYS in the kernel is seccomp filter, which we don't support
yet.
So before we enable seccomp filter, add compat handling for SIGSYS.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/compat.h | 7 +++++++
arch/powerpc/kernel/signal_32.c | 5 +++++
2 files changed, 12 insertions(+)
@@ -949,6 +949,11 @@ int copy_siginfo_to_user32(struct compat_siginfo __user *d, const siginfo_t *s)err|=__put_user(s->si_overrun,&d->si_overrun);err|=__put_user(s->si_int,&d->si_int);break;+case__SI_SYS>>16:+err|=__put_user(ptr_to_compat(s->si_call_addr),&d->si_call_addr);+err|=__put_user(s->si_syscall,&d->si_syscall);+err|=__put_user(s->si_arch,&d->si_arch);+break;case__SI_RT>>16:/* This is not generated by the kernel as of now. */case__SI_MESGQ>>16:err|=__put_user(s->si_int,&d->si_int);
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:37
Currently syscall_get_arguments() has two loops, one for compat and one
for regular tasks. In prepartion for the next patch, which changes which
registers we use, switch it to only have one loop, so we only have one
place to update.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/syscall.h | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:44
This commit enables seccomp filter on powerpc, now that we have all the
necessary pieces in place.
To support seccomp's desire to modify the syscall return value under
some circumstances, we use a different ABI to the ptrace ABI. That is we
use r3 as the syscall return value, and orig_gpr3 is the first syscall
parameter.
This means the seccomp code, or a ptracer via SECCOMP_RET_TRACE, will
see -ENOSYS preloaded in r3. This is identical to the behaviour on x86,
and allows seccomp or the ptracer to either leave the -ENOSYS or change
it to something else, as well as rejecting or not the syscall by
modifying r0.
If seccomp does not reject the syscall, we restore the register state to
match what ptrace and audit expect, ie. r3 is the first syscall
parameter again. We do this restore using orig_gpr3, which may have been
modified by seccomp, which allows seccomp to modify the first syscall
paramater and allow the syscall to proceed.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/ptrace.c | 28 +++++++++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-23 10:22:50
Wire up the syscall number and regs so the tests work on powerpc.
Acked-by: Kees Cook <redacted>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/seccomp/seccomp_bpf.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
Currently on powerpc we have our own #define for the highest (negative)
errno value, called _LAST_ERRNO. This is defined to be 516, for reasons
which are not clear.
The generic code, and x86, use MAX_ERRNO, which is defined to be 4095.
In particular seccomp uses MAX_ERRNO to restrict the value that a
seccomp filter can return.
Currently with the mismatch between _LAST_ERRNO and MAX_ERRNO, a seccomp
tracer wanting to return 600, expecting it to be seen as an error, would
instead find on powerpc that userspace sees a successful syscall with a
return value of 600.
To avoid this inconsistency, switch powerpc to use MAX_ERRNO.
We are somewhat confident that generic syscalls that can return a
non-error value above negative MAX_ERRNO have already been updated to
use force_successful_syscall_return().
I have also checked all the powerpc specific syscalls, and believe that
none of them expect to return a non-error value between -MAX_ERRNO and
-516. So this change should be safe ...
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
The API for calling do_syscall_trace_enter() is currently sensible
enough, it just returns the (modified) syscall number.
However once we enable seccomp filter it will get more complicated. When
seccomp filter runs, the seccomp kernel code (via SECCOMP_RET_ERRNO), or
a ptracer (via SECCOMP_RET_TRACE), may reject the syscall and *may* or may
*not* set a return value in r3.
That means the assembler that calls do_syscall_trace_enter() can not
blindly return ENOSYS, it needs to only return ENOSYS if a return value
has not already been set.
There is no way to implement that logic with the current API. So change
the do_syscall_trace_enter() API to make it deal with the return code
juggling, and the assembler can then just return whatever return code it
is given.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
@@ -1762,26 +1762,42 @@ long arch_ptrace(struct task_struct *child, long request,returnret;}-/*-*Wemustreturnthesyscallnumbertoactuallylookupinthetable.-*Thiscanbe-1Ltoskiprunninganysyscallatall.+/**+*do_syscall_trace_enter()-Dosyscalltracingonkernelentry.+*@regs:thept_regsofthetasktotrace(current)+*+*Performsvarioustypesoftracingonsyscallentry.Thisincludesseccomp,+*ptrace,syscalltracepointsandaudit.+*+*Thept_regsarepotentiallyvisibletouserspaceviaptrace,sotheir+*contentsisABI.+*+*Oneormoreofthetracersmaymodifythecontentsofpt_regs,inparticular+*tomodifyargumentsoreventhesyscallnumberitself.+*+*It'salsopossiblethatatracercanchoosetorejectthesystemcall.In+*thatcasethisfunctionwillreturnanillegalsyscallnumber,andwillput+*anappropriatereturnvalueinregs->r3.+*+*Return:the(possiblychanged)syscallnumber.*/longdo_syscall_trace_enter(structpt_regs*regs){-longret=0;+boolabort=false;user_exit();secure_computing_strict(regs->gpr[0]);-if(test_thread_flag(TIF_SYSCALL_TRACE)&&-tracehook_report_syscall_entry(regs))+if(test_thread_flag(TIF_SYSCALL_TRACE)){/*-*Tracingdecidedthissyscallshouldnothappen.-*We'llreturnaboguscallnumbertogetanENOSYS-*error,butleavetheoriginalnumberinregs->gpr[0].+*Thetracermaydecidetoabortthesyscall,ifsotracehook+*willreturn!0.Notethatthetracermayalsojustchange+*regs->gpr[0]toaninvalidsyscallnumber,thatishandled+*belowontheexitpath.*/-ret=-1L;+abort=tracehook_report_syscall_entry(regs)!=0;+}if(unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))trace_sys_enter(regs,regs->gpr[0]);
@@ -1798,7 +1814,17 @@ long do_syscall_trace_enter(struct pt_regs *regs)regs->gpr[5]&0xffffffff,regs->gpr[6]&0xffffffff);-returnret?:regs->gpr[0];+if(abort||regs->gpr[0]>=NR_syscalls){+/*+*Ifweareabortingexplicitly,orifthesyscallnumberis+*nowinvalid,setthereturnvalueto-ENOSYS.+*/+regs->gpr[3]=-ENOSYS;+return-1;+}++/* Return the possibly modified but valid syscall number */+returnregs->gpr[0];}voiddo_syscall_trace_leave(structpt_regs*regs)--
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
syscall_get_error() is unused, and never has been.
It's also probably wrong, as it negates r3 before returning it, but that
depends on what the caller is expecting.
It also doesn't deal with compat, and doesn't deal with TIF_NOERROR.
Although we could fix those, until it has a caller and it's clear what
semantics the caller wants it's just untested code. So drop it.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
Currently the only caller of syscall_set_return_value() is seccomp
filter, which is not enabled on powerpc.
This means we have not noticed that our implementation of
syscall_set_return_value() negates error, even though the value passed
in is already negative.
So remove the negation in syscall_set_return_value(), and expect the
caller to do it like all other implementations do.
Also add a comment about the ccr handling.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
Currently syscall_get_arguments() has two loops, one for compat and one
for regular tasks. In prepartion for the next patch, which changes which
registers we use, switch it to only have one loop, so we only have one
place to update.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
Currently syscall_get_arguments() is used by syscall tracepoints, and
collect_syscall() which is used in some debugging as well as
/proc/pid/syscall.
The current implementation just copies regs->gpr[3 .. 5] out, which is
fine for all the current use cases.
When we enable seccomp filter, that will also start using
syscall_get_arguments(). However for seccomp filter we want to use r3
as the return value of the syscall, and orig_gpr3 as the first
parameter. This will allow seccomp to modify the return value in r3.
To support this we need to modify syscall_get_arguments() to return
orig_gpr3 instead of r3. This is safe for all uses because orig_gpr3
always contains the r3 value that was passed to the syscall. We store it
in the syscall entry path and never modify it.
Update syscall_set_arguments() while we're here, even though it's never
used.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
@@ -83,6 +89,10 @@ static inline void syscall_set_arguments(struct task_struct *task,{BUG_ON(i+n>6);memcpy(®s->gpr[3+i],args,n*sizeof(args[0]));++/* Also copy the first argument into orig_gpr3 */+if(i==0&&n>0)+regs->orig_gpr3=args[0];}staticinlineintsyscall_get_arch(void)--
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
The documentation for syscall_get_nr() in asm-generic says:
Note this returns int even on 64-bit machines. Only 32 bits of
system call number can be meaningful. If the actual arch value
is 64 bits, this truncates to 32 bits so 0xffffffff means -1.
However our implementation was never updated to reflect this.
Generally it's not important, but there is once case where it matters.
For seccomp filter with SECCOMP_RET_TRACE, the tracer will set
regs->gpr[0] to -1 to reject the syscall. When the task is a compat
task, this means we end up with 0xffffffff in r0 because ptrace will
zero extend the 32-bit value.
If syscall_get_nr() returns an unsigned long, then a 64-bit kernel will
see a positive value in r0 and will incorrectly allow the syscall
through seccomp.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
SIG_SYS was added in commit a0727e8ce513 "signal, x86: add SIGSYS info
and make it synchronous."
Because we use the asm-generic struct siginfo, we got support for
SIG_SYS for free as part of that commit.
However there was no compat handling added for powerpc. That means we've
been advertising the existence of signfo._sifields._sigsys to compat
tasks, but not actually filling in the fields correctly.
Luckily it looks like no one has noticed, presumably because the only
user of SIGSYS in the kernel is seccomp filter, which we don't support
yet.
So before we enable seccomp filter, add compat handling for SIGSYS.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
@@ -949,6 +949,11 @@ int copy_siginfo_to_user32(struct compat_siginfo __user *d, const siginfo_t *s)err|=__put_user(s->si_overrun,&d->si_overrun);err|=__put_user(s->si_int,&d->si_int);break;+case__SI_SYS>>16:+err|=__put_user(ptr_to_compat(s->si_call_addr),&d->si_call_addr);+err|=__put_user(s->si_syscall,&d->si_syscall);+err|=__put_user(s->si_arch,&d->si_arch);+break;case__SI_RT>>16:/* This is not generated by the kernel as of now. */case__SI_MESGQ>>16:err|=__put_user(s->si_int,&d->si_int);--
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
This commit enables seccomp filter on powerpc, now that we have all the
necessary pieces in place.
To support seccomp's desire to modify the syscall return value under
some circumstances, we use a different ABI to the ptrace ABI. That is we
use r3 as the syscall return value, and orig_gpr3 is the first syscall
parameter.
This means the seccomp code, or a ptracer via SECCOMP_RET_TRACE, will
see -ENOSYS preloaded in r3. This is identical to the behaviour on x86,
and allows seccomp or the ptracer to either leave the -ENOSYS or change
it to something else, as well as rejecting or not the syscall by
modifying r0.
If seccomp does not reject the syscall, we restore the register state to
match what ptrace and audit expect, ie. r3 is the first syscall
parameter again. We do this restore using orig_gpr3, which may have been
modified by seccomp, which allows seccomp to modify the first syscall
paramater and allow the syscall to proceed.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
The seccomp_bpf test uses BPF_LD|BPF_W|BPF_ABS to load 32-bit values
from seccomp_data->args. On big endian machines this will load the high
word of the argument, which is not what the test wants.
Borrow a hack from samples/seccomp/bpf-helper.h which changes the offset
on big endian to account for this.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Acked-by: Kees Cook <redacted>
Thanks for fixing the error message! :)
-Kees
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
Wire up the syscall number and regs so the tests work on powerpc.
This patch begs the question: this passes everything? (I was reminded
about syscall_restart while reading the patches. If everything passes,
it looks like powerpc exposes syscall_restart? Seems like ARM remains
the odd-arch-out on this. :P)
Thanks!
-Kees
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-28 12:18:51
On Mon, 2015-07-27 at 11:59 -0700, Kees Cook wrote:
On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman [off-list ref] wrote:
quoted
Wire up the syscall number and regs so the tests work on powerpc.
This patch begs the question: this passes everything?
Doh, yes that would have been worth mentioning! :)
I'll update it before I commit it.
Yes everything passes, thanks very much for the tests BTW.
And for the record I've tested on: ppc64, ppc64 (compat), ppc64le,
ppc, ppc64e and ppc64e (compat).
(I was reminded about syscall_restart while reading the patches. If
everything passes, it looks like powerpc exposes syscall_restart? Seems like
ARM remains the odd-arch-out on this. :P)
Yeah I noticed that and though "oh jeez not syscall_restart", but it seems to
pass, so I guess we behave in a standard fasion for once. I'll have a closer
look just to be sure before I merge.
cheers
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-07-30 06:08:11
This commit enables seccomp filter on powerpc, now that we have all the
necessary pieces in place.
To support seccomp's desire to modify the syscall return value under
some circumstances, we use a different ABI to the ptrace ABI. That is we
use r3 as the syscall return value, and orig_gpr3 is the first syscall
parameter.
This means the seccomp code, or a ptracer via SECCOMP_RET_TRACE, will
see -ENOSYS preloaded in r3. This is identical to the behaviour on x86,
and allows seccomp or the ptracer to either leave the -ENOSYS or change
it to something else, as well as rejecting or not the syscall by
modifying r0.
If seccomp does not reject the syscall, we restore the register state to
match what ptrace and audit expect, ie. r3 is the first syscall
parameter again. We do this restore using orig_gpr3, which may have been
modified by seccomp, which allows seccomp to modify the first syscall
paramater and allow the syscall to proceed.
We need to #ifdef the the additional handling of r3 for seccomp, so move
it all out of line.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Kees Cook <redacted>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/ptrace.c | 41 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 1 deletion(-)
v2: The previous version didn't compile for CONFIG_SECCOMP=n. To fix it up I
moved the logic out of line and #ifdef'ed that. It gets inlined by the compiler
so the end result is the same.
Kees I kept your Reviewed-by on the basis that the interesting logic is the
same, hope that's OK by you.
On Wed, Jul 29, 2015 at 10:56 PM, Michael Ellerman [off-list ref] wrote:
This commit enables seccomp filter on powerpc, now that we have all the
necessary pieces in place.
To support seccomp's desire to modify the syscall return value under
some circumstances, we use a different ABI to the ptrace ABI. That is we
use r3 as the syscall return value, and orig_gpr3 is the first syscall
parameter.
This means the seccomp code, or a ptracer via SECCOMP_RET_TRACE, will
see -ENOSYS preloaded in r3. This is identical to the behaviour on x86,
and allows seccomp or the ptracer to either leave the -ENOSYS or change
it to something else, as well as rejecting or not the syscall by
modifying r0.
If seccomp does not reject the syscall, we restore the register state to
match what ptrace and audit expect, ie. r3 is the first syscall
parameter again. We do this restore using orig_gpr3, which may have been
modified by seccomp, which allows seccomp to modify the first syscall
paramater and allow the syscall to proceed.
We need to #ifdef the the additional handling of r3 for seccomp, so move
it all out of line.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Kees Cook <redacted>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/ptrace.c | 41 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 1 deletion(-)
v2: The previous version didn't compile for CONFIG_SECCOMP=n. To fix it up I
moved the logic out of line and #ifdef'ed that. It gets inlined by the compiler
so the end result is the same.
Kees I kept your Reviewed-by on the basis that the interesting logic is the
same, hope that's OK by you.
Totally fine. Thanks!
-Kees
--
Kees Cook
Chrome OS Security