| Message ID | 20201215173510.253770-1-ddstreet@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,Focal,PATCHv2] s390/ptrace: return -ENOSYS when invalid syscall is supplied | expand |
On 15.12.20 18:35, Dan Streetman wrote: > From: Sven Schnelle <svens@linux.ibm.com> > > BugLink: https://bugs.launchpad.net/bugs/1895132 > > The current code returns the syscall number which an invalid > syscall number is supplied and tracing is enabled. This makes > the strace testsuite fail. > > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> > (backported from commit cd29fa798001075a554b978df3a64e6656c25794) > Signed-off-by: Dan Streetman <ddstreet@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > > --- > changes since v1: removed return before added goto > > arch/s390/kernel/ptrace.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c > index c6aef2ecf289..ad74472ce967 100644 > --- a/arch/s390/kernel/ptrace.c > +++ b/arch/s390/kernel/ptrace.c > @@ -867,6 +867,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, > asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > { > unsigned long mask = -1UL; > + long ret = -1; > > /* > * The sysc_tracesys code in entry.S stored the system > @@ -878,27 +879,33 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > * Tracing decided this syscall should not happen. Skip > * the system call and the system call restart handling. > */ > - clear_pt_regs_flag(regs, PIF_SYSCALL); > - return -1; > + goto skip; > } > > /* Do the secure computing check after ptrace. */ > if (secure_computing(NULL)) { > /* seccomp failures shouldn't expose any additional code. */ > - return -1; > + goto skip; > } > > if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > - trace_sys_enter(regs, regs->gprs[2]); > + trace_sys_enter(regs, regs->int_code & 0xffff); > > if (is_compat_task()) > mask = 0xffffffff; > > - audit_syscall_entry(regs->gprs[2], regs->orig_gpr2 & mask, > + audit_syscall_entry(regs->int_code & 0xffff, regs->orig_gpr2 & mask, > regs->gprs[3] &mask, regs->gprs[4] &mask, > regs->gprs[5] &mask); > > + if ((signed long)regs->gprs[2] >= NR_syscalls) { > + regs->gprs[2] = -ENOSYS; > + ret = -ENOSYS; > + } > return regs->gprs[2]; > +skip: > + clear_pt_regs_flag(regs, PIF_SYSCALL); > + return ret; > } > > asmlinkage void do_syscall_trace_exit(struct pt_regs *regs) >
On 15.12.20 18:35, Dan Streetman wrote: > From: Sven Schnelle <svens@linux.ibm.com> > > BugLink: https://bugs.launchpad.net/bugs/1895132 > > The current code returns the syscall number which an invalid > syscall number is supplied and tracing is enabled. This makes > the strace testsuite fail. > > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> > (backported from commit cd29fa798001075a554b978df3a64e6656c25794) > Signed-off-by: Dan Streetman <ddstreet@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > > --- > changes since v1: removed return before added goto > > arch/s390/kernel/ptrace.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c > index c6aef2ecf289..ad74472ce967 100644 > --- a/arch/s390/kernel/ptrace.c > +++ b/arch/s390/kernel/ptrace.c > @@ -867,6 +867,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, > asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > { > unsigned long mask = -1UL; > + long ret = -1; > > /* > * The sysc_tracesys code in entry.S stored the system > @@ -878,27 +879,33 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > * Tracing decided this syscall should not happen. Skip > * the system call and the system call restart handling. > */ > - clear_pt_regs_flag(regs, PIF_SYSCALL); > - return -1; > + goto skip; > } > > /* Do the secure computing check after ptrace. */ > if (secure_computing(NULL)) { > /* seccomp failures shouldn't expose any additional code. */ > - return -1; > + goto skip; > } > > if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > - trace_sys_enter(regs, regs->gprs[2]); > + trace_sys_enter(regs, regs->int_code & 0xffff); > > if (is_compat_task()) > mask = 0xffffffff; > > - audit_syscall_entry(regs->gprs[2], regs->orig_gpr2 & mask, > + audit_syscall_entry(regs->int_code & 0xffff, regs->orig_gpr2 & mask, > regs->gprs[3] &mask, regs->gprs[4] &mask, > regs->gprs[5] &mask); > > + if ((signed long)regs->gprs[2] >= NR_syscalls) { > + regs->gprs[2] = -ENOSYS; > + ret = -ENOSYS; > + } > return regs->gprs[2]; > +skip: > + clear_pt_regs_flag(regs, PIF_SYSCALL); > + return ret; > } > > asmlinkage void do_syscall_trace_exit(struct pt_regs *regs) >
On 15.12.20 18:35, Dan Streetman wrote: > From: Sven Schnelle <svens@linux.ibm.com> > > BugLink: https://bugs.launchpad.net/bugs/1895132 > > The current code returns the syscall number which an invalid > syscall number is supplied and tracing is enabled. This makes > the strace testsuite fail. > > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> > (backported from commit cd29fa798001075a554b978df3a64e6656c25794) > Signed-off-by: Dan Streetman <ddstreet@canonical.com> > > --- Applied to focal:linux/master-next. Thanks. Btw, this was likely buried by the fact that the v2 was sent as a reply to the original v1 submission. I hope that shows why this is a bad idea. -Stefan > changes since v1: removed return before added goto > > arch/s390/kernel/ptrace.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c > index c6aef2ecf289..ad74472ce967 100644 > --- a/arch/s390/kernel/ptrace.c > +++ b/arch/s390/kernel/ptrace.c > @@ -867,6 +867,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, > asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > { > unsigned long mask = -1UL; > + long ret = -1; > > /* > * The sysc_tracesys code in entry.S stored the system > @@ -878,27 +879,33 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > * Tracing decided this syscall should not happen. Skip > * the system call and the system call restart handling. > */ > - clear_pt_regs_flag(regs, PIF_SYSCALL); > - return -1; > + goto skip; > } > > /* Do the secure computing check after ptrace. */ > if (secure_computing(NULL)) { > /* seccomp failures shouldn't expose any additional code. */ > - return -1; > + goto skip; > } > > if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > - trace_sys_enter(regs, regs->gprs[2]); > + trace_sys_enter(regs, regs->int_code & 0xffff); > > if (is_compat_task()) > mask = 0xffffffff; > > - audit_syscall_entry(regs->gprs[2], regs->orig_gpr2 & mask, > + audit_syscall_entry(regs->int_code & 0xffff, regs->orig_gpr2 & mask, > regs->gprs[3] &mask, regs->gprs[4] &mask, > regs->gprs[5] &mask); > > + if ((signed long)regs->gprs[2] >= NR_syscalls) { > + regs->gprs[2] = -ENOSYS; > + ret = -ENOSYS; > + } > return regs->gprs[2]; > +skip: > + clear_pt_regs_flag(regs, PIF_SYSCALL); > + return ret; > } > > asmlinkage void do_syscall_trace_exit(struct pt_regs *regs) >
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c index c6aef2ecf289..ad74472ce967 100644 --- a/arch/s390/kernel/ptrace.c +++ b/arch/s390/kernel/ptrace.c @@ -867,6 +867,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) { unsigned long mask = -1UL; + long ret = -1; /* * The sysc_tracesys code in entry.S stored the system @@ -878,27 +879,33 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) * Tracing decided this syscall should not happen. Skip * the system call and the system call restart handling. */ - clear_pt_regs_flag(regs, PIF_SYSCALL); - return -1; + goto skip; } /* Do the secure computing check after ptrace. */ if (secure_computing(NULL)) { /* seccomp failures shouldn't expose any additional code. */ - return -1; + goto skip; } if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) - trace_sys_enter(regs, regs->gprs[2]); + trace_sys_enter(regs, regs->int_code & 0xffff); if (is_compat_task()) mask = 0xffffffff; - audit_syscall_entry(regs->gprs[2], regs->orig_gpr2 & mask, + audit_syscall_entry(regs->int_code & 0xffff, regs->orig_gpr2 & mask, regs->gprs[3] &mask, regs->gprs[4] &mask, regs->gprs[5] &mask); + if ((signed long)regs->gprs[2] >= NR_syscalls) { + regs->gprs[2] = -ENOSYS; + ret = -ENOSYS; + } return regs->gprs[2]; +skip: + clear_pt_regs_flag(regs, PIF_SYSCALL); + return ret; } asmlinkage void do_syscall_trace_exit(struct pt_regs *regs)