ACK: [SRU Bionic/Impish 1/1] ptrace: Check PTRACE_O_SUSPEND_SECCOMP permission on PTRACE_SEIZE

[Cengiz Can]

On Mon, 2022-05-09 at 21:42 -0300, Thadeu Lima de Souza Cascardo wrote:
> From: Jann Horn <jannh at google.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1972740
> 
> Setting PTRACE_O_SUSPEND_SECCOMP is supposed to be a highly
> privileged
> operation because it allows the tracee to completely bypass all
> seccomp
> filters on kernels with CONFIG_CHECKPOINT_RESTORE=y. It is only
> supposed to
> be settable by a process with global CAP_SYS_ADMIN, and only if that
> process is not subject to any seccomp filters at all.
> 
> However, while these permission checks were done on the
> PTRACE_SETOPTIONS
> path, they were missing on the PTRACE_SEIZE path, which also sets
> user-specified ptrace flags.
> 
> Move the permissions checks out into a helper function and let both
> ptrace_attach() and ptrace_setoptions() call it.
> 
> Cc: stable at kernel.org
> Fixes: 13c4a90119d2 ("seccomp: add ptrace options for
> suspend/resume")
> Signed-off-by: Jann Horn <jannh at google.com>
> Link:
> https://lkml.kernel.org/r/20220319010838.1386861-1-jannh@google.com
> Signed-off-by: Eric W. Biederman <ebiederm at xmission.com>
> (cherry picked from commit ee1fee900537b5d9560e9f937402de5ddc8412f3)
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
Acked-by: Cengiz Can <cengiz.can at canonical.com>
> ---
>  kernel/ptrace.c | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 2997ca600d18..0a0fd0e74a6b 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -371,6 +371,26 @@ bool ptrace_may_access(struct task_struct *task,
> unsigned int mode)
>         return !err;
>  }
>  
> +static int check_ptrace_options(unsigned long data)
> +{
> +       if (data & ~(unsigned long)PTRACE_O_MASK)
> +               return -EINVAL;
> +
> +       if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
> +               if (!IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) ||
> +                   !IS_ENABLED(CONFIG_SECCOMP))
> +                       return -EINVAL;
> +
> +               if (!capable(CAP_SYS_ADMIN))
> +                       return -EPERM;
> +
> +               if (seccomp_mode(&current->seccomp) !=
> SECCOMP_MODE_DISABLED ||
> +                   current->ptrace & PT_SUSPEND_SECCOMP)
> +                       return -EPERM;
> +       }
> +       return 0;
> +}
> +
>  static int ptrace_attach(struct task_struct *task, long request,
>                          unsigned long addr,
>                          unsigned long flags)
> @@ -382,8 +402,16 @@ static int ptrace_attach(struct task_struct
> *task, long request,
>         if (seize) {
>                 if (addr != 0)
>                         goto out;
> +               /*
> +                * This duplicates the check in
> check_ptrace_options() because
> +                * ptrace_attach() and ptrace_setoptions() have
> historically
> +                * used different error codes for unknown ptrace
> options.
> +                */
>                 if (flags & ~(unsigned long)PTRACE_O_MASK)
>                         goto out;
> +               retval = check_ptrace_options(flags);
> +               if (retval)
> +                       return retval;
>                 flags = PT_PTRACED | PT_SEIZED | (flags <<
> PT_OPT_FLAG_SHIFT);
>         } else {
>                 flags = PT_PTRACED;
> @@ -656,22 +684,11 @@ int ptrace_writedata(struct task_struct *tsk,
> char __user *src, unsigned long ds
>  static int ptrace_setoptions(struct task_struct *child, unsigned
> long data)
>  {
>         unsigned flags;
> +       int ret;
>  
> -       if (data & ~(unsigned long)PTRACE_O_MASK)
> -               return -EINVAL;
> -
> -       if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
> -               if (!IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) ||
> -                   !IS_ENABLED(CONFIG_SECCOMP))
> -                       return -EINVAL;
> -
> -               if (!capable(CAP_SYS_ADMIN))
> -                       return -EPERM;
> -
> -               if (seccomp_mode(&current->seccomp) !=
> SECCOMP_MODE_DISABLED ||
> -                   current->ptrace & PT_SUSPEND_SECCOMP)
> -                       return -EPERM;
> -       }
> +       ret = check_ptrace_options(data);
> +       if (ret)
> +               return ret;
>  
>         /* Avoid intermediate state when all opts are cleared */
>         flags = child->ptrace;
> -- 
> 2.32.0
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 870 bytes
Desc: This is a digitally signed message part
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220510/d2f4c72a/attachment.sig>