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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Dan Streetman-2
From: Sven Schnelle <[hidden email]>

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 <[hidden email]>
Signed-off-by: Vasily Gorbik <[hidden email]>
(backported from commit cd29fa798001075a554b978df3a64e6656c25794)
Signed-off-by: Dan Streetman <[hidden email]>
---
 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)
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bader-2
On 04.12.20 21:25, Dan Streetman wrote:

> From: Sven Schnelle <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Vasily Gorbik <[hidden email]>
> (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> Signed-off-by: Dan Streetman <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

Since the bug report mentions kernels <5.8, what about Bionic and Xenial?

-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)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Dan Streetman-2
On Mon, Dec 7, 2020 at 5:03 AM Stefan Bader <[hidden email]> wrote:

>
> On 04.12.20 21:25, Dan Streetman wrote:
> > From: Sven Schnelle <[hidden email]>
> >
> > 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 <[hidden email]>
> > Signed-off-by: Vasily Gorbik <[hidden email]>
> > (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> > Signed-off-by: Dan Streetman <[hidden email]>
> Acked-by: Stefan Bader <[hidden email]>
> > ---
>
> 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)
> >
>
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Kleber Souza
In reply to this post by Dan Streetman-2
On 04.12.20 21:25, Dan Streetman wrote:

> From: Sven Schnelle <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Vasily Gorbik <[hidden email]>
> (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> Signed-off-by: Dan Streetman <[hidden email]>
> ---
>   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;

The return should be removed here as well ^.

Otherwise it looks good. Could you please send a v2 with this fix?

Thanks,
Kleber

>   }
>  
>   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)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Dan Streetman-2
From: Sven Schnelle <[hidden email]>

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 <[hidden email]>
Signed-off-by: Vasily Gorbik <[hidden email]>
(backported from commit cd29fa798001075a554b978df3a64e6656c25794)
Signed-off-by: Dan Streetman <[hidden email]>

---
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)
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Dan Streetman-2
In reply to this post by Kleber Souza
On Tue, Dec 15, 2020 at 5:15 AM Kleber Souza <[hidden email]> wrote:

>
> On 04.12.20 21:25, Dan Streetman wrote:
> > From: Sven Schnelle <[hidden email]>
> >
> > 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 <[hidden email]>
> > Signed-off-by: Vasily Gorbik <[hidden email]>
> > (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> > Signed-off-by: Dan Streetman <[hidden email]>
> > ---
> >   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;
>
> The return should be removed here as well ^.

oops! sorry.

>
> Otherwise it looks good. Could you please send a v2 with this fix?

yep, resent. thanks!

>
> Thanks,
> Kleber
>
> >       }
> >
> >       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)
> >
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Kleber Souza
In reply to this post by Dan Streetman-2
On 15.12.20 18:35, Dan Streetman wrote:

> From: Sven Schnelle <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Vasily Gorbik <[hidden email]>
> (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> Signed-off-by: Dan Streetman <[hidden email]>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

>
> ---
> 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)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bader-2
In reply to this post by Dan Streetman-2
On 15.12.20 18:35, Dan Streetman wrote:

> From: Sven Schnelle <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Vasily Gorbik <[hidden email]>
> (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> Signed-off-by: Dan Streetman <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

>
> ---
> 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)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bader-2
In reply to this post by Dan Streetman-2
On 15.12.20 18:35, Dan Streetman wrote:

> From: Sven Schnelle <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Vasily Gorbik <[hidden email]>
> (backported from commit cd29fa798001075a554b978df3a64e6656c25794)
> Signed-off-by: Dan Streetman <[hidden email]>
>
> ---
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)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment