ACK/Cmnt: [SRU][Focal][PATCH] s390/ptrace: return -ENOSYS when invalid syscall is supplied

[Dan Streetman]

On Mon, Dec 7, 2020 at 5:03 AM Stefan Bader <stefan.bader at canonical.com> wrote:
>
> On 04.12.20 21:25, Dan Streetman wrote:
> > From: Sven Schnelle <svens at 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 at linux.ibm.com>
> > Signed-off-by: Vasily Gorbik <gor at linux.ibm.com>
> > (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> > Signed-off-by: Dan Streetman <ddstreet at canonical.com>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
> > ---
>
> Since the bug report mentions kernels <5.8, what about Bionic and Xenial?

The bug was introduced by upstream commit
00332c16b1604242a56289ff2b26e283dbad0812, which was added to stable
v5.4:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/s390/kernel/ptrace.c?h=v5.4.81#n875

but (as of today) hasn't been added to stable v4.14 or v4.4:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/s390/kernel/ptrace.c?h=v4.14.210#n873
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/s390/kernel/ptrace.c?h=v4.4.247#n839

The latest xenial 4.4 and the latest bionic 4.15 don't include that
commit either.

So this patch isn't needed for X 4.4 or B 4.15 currently, and would
only be needed if someone backports the upstream commit introducing
the bug.

>
> -Stefan
>
> >  arch/s390/kernel/ptrace.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
> > index c6aef2ecf289..2ec0538fdf8a 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,34 @@ 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)
> >
>
>