On Thu, Nov 01, 2018 at 02:40:02PM +0100, Oleg Nesterov wrote:

On 10/29, Tycho Andersen wrote:

quoted

+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