On Thu, Nov 01, 2018 at 02:40:02PM +0100, Oleg Nesterov wrote:
> On 10/29, Tycho Andersen wrote:
> >
> > +static struct file *init_listener(struct seccomp_filter *filter)
> > +{
> > + struct file *ret = ERR_PTR(-EBUSY);
> > + struct seccomp_filter *cur, *last_locked = NULL;
> > + int filter_nesting = 0;
> > +
> > + for (cur = current->seccomp.filter; cur; cur = cur->prev) {
> > + mutex_lock_nested(&cur->notify_lock, filter_nesting);
> > + filter_nesting++;
> > + last_locked = cur;
> > + if (cur->notif)
> > + goto out;
> > + }
>
> Somehow I no longer understand why do you need to take all locks. Isn't
> the first filter's notify_lock enough? IOW,
>
> for (cur = current->seccomp.filter; cur; cur = cur->prev) {
> if (cur->notif)
> return ERR_PTR(-EBUSY);
> first = cur;
> }
>
> if (first)
> mutex_lock(&first->notify_lock);
>
> ... initialize filter->notif ...
>
> out:
> if (first)
> mutex_unlock(&first->notify_lock);
>
> return ret;
The idea here is to prevent people from "nesting" notify filters. So
if any filter in the chain has a listener attached, it refuses to
install another filter with a listener.
But it just occurred to me that we don't handle the TSYNC case
correctly by doing it this way, and it's not necessarily obvious to me
how we can :). So let me look into that.
Tycho
