Messages in this thread | | | Date | Tue, 20 Oct 2015 20:00:24 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v7] seccomp, ptrace: add support for dumping seccomp filters |
| |
Sorry for delay...
On 10/13, Tycho Andersen wrote: > > --- a/include/uapi/linux/ptrace.h > +++ b/include/uapi/linux/ptrace.h > @@ -23,6 +23,8 @@ > > #define PTRACE_SYSCALL 24 > > +#define PTRACE_SECCOMP_GET_FILTER 40
Probably it would be better to add this at the end of other 0x42.. constants? After PTRACE_SETSIGMASK.
> --- a/kernel/seccomp.c > +++ b/kernel/seccomp.c > @@ -347,6 +347,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog) > { > struct seccomp_filter *sfilter; > int ret; > + const bool save_orig = config_enabled(CONFIG_CHECKPOINT_RESTORE); > > if (fprog->len == 0 || fprog->len > BPF_MAXINSNS) > return ERR_PTR(-EINVAL); > @@ -370,7 +371,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog) > return ERR_PTR(-ENOMEM); > > ret = bpf_prog_create_from_user(&sfilter->prog, fprog, > - seccomp_check_filter, false); > + seccomp_check_filter, save_orig);
Can't comment, this depends on other changes I missed... but I don't this you need my review here ;)
> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE) > +long seccomp_get_filter(struct task_struct *task, unsigned long filter_off, > + void __user *data) > +{ > + struct seccomp_filter *filter; > + struct sock_fprog_kern *fprog; > + long ret; > + unsigned long count = 0; > + > + spin_lock_irq(¤t->sighand->siglock); > + if (!capable(CAP_SYS_ADMIN) || > + current->seccomp.mode != SECCOMP_MODE_DISABLED) { > + ret = -EACCES; > + goto out_self; > + } > + > + spin_lock_irq(&task->sighand->siglock);
Oh, no, you can't do this.
This is deadlockable. Suppose that this task's sub-thread traces the caller (the current task) and does PTRACE_SECCOMP_GET_FILTER too.
In this case it can take the same 2 locks in reverse order, deadlock.
But why do you need to hold both ->siglock's at the same time?
Oleg.
| |