Messages in this thread | | | Date | Thu, 4 Jun 2015 20:31:49 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v2] seccomp: add ptrace options for suspend/resume |
| |
On 06/03, Tycho Andersen wrote: > > @@ -556,6 +557,11 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data) > if (data & ~(unsigned long)PTRACE_O_MASK) > return -EINVAL; > > +#ifdef CONFIG_CHECKPOINT_RESTORE > + if (data & PTRACE_O_SUSPEND_SECCOMP && !may_suspend_seccomp()) > + return -EPERM; > +#endif > +
Well. This -EPERM doesn't look consistent...
if config_enabled(CONFIG_CHECKPOINT_RESTORE) == F, we return success but PTRACE_O_SUSPEND_SECCOMP has no effect because of another ifdef in seccomp.
OTOH, if CONFIG_SECCOMP=n, this option has no effect too but we return -EPERM even.
Also. Suppose that the tracer sets SUSPEND_SECCOMP and then drops CAP_SYS_ADMIN. After that it can't set or clear other ptrace options.
So if we really want the security checks (I still think we do not ;) then we should probably check "flags & SUSPEND_SECCOMP" as well.
> +#ifdef CONFIG_CHECKPOINT_RESTORE > +bool may_suspend_seccomp(void) > +{ > + if (!capable(CAP_SYS_ADMIN)) > + return false; > + > + if (current->seccomp.mode != SECCOMP_MODE_DISABLED) > + return false;
Heh. OK, I won't argue with the new check too ;)
Oleg.
| |