All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrei Vagin <avagin@google.com>
To: Kees Cook <keescook@chromium.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Christian Brauner <brauner@kernel.org>
Cc: linux-kernel@vger.kernel.org, Andrei Vagin <avagin@gmail.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Peter Oskolkov <posk@google.com>,
	Tycho Andersen <tycho@tycho.pizza>,
	Will Drewry <wad@chromium.org>,
	Vincent Guittot <vincent.guittot@linaro.org>
Subject: [PATCH 1/5] seccomp: don't use semaphore and wait_queue together
Date: Thu, 10 Nov 2022 23:31:50 -0800	[thread overview]
Message-ID: <20221111073154.784261-2-avagin@google.com> (raw)
In-Reply-To: <20221111073154.784261-1-avagin@google.com>

From: Andrei Vagin <avagin@gmail.com>

The main reason is to use new wake_up helpers that will be added in the
following patches. But here are a few other reasons:

* if we use two different ways, we always need to call them both. This
  patch fixes seccomp_notify_recv where we forgot to call wake_up_poll
  in the error path.

* If we use one primitive, we can control how many waiters are woken up
  for each request. Our goal is to wake up just one that will handle a
  request. Right now, wake_up_poll can wake up one waiter and
  up(&match->notif->request) can wake up one more.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 kernel/seccomp.c | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e9852d1b4a5e..876022e9c88c 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -145,7 +145,7 @@ struct seccomp_kaddfd {
  * @notifications: A list of struct seccomp_knotif elements.
  */
 struct notification {
-	struct semaphore request;
+	atomic_t requests;
 	u64 next_id;
 	struct list_head notifications;
 };
@@ -1116,7 +1116,7 @@ static int seccomp_do_user_notification(int this_syscall,
 	list_add_tail(&n.list, &match->notif->notifications);
 	INIT_LIST_HEAD(&n.addfd);
 
-	up(&match->notif->request);
+	atomic_add(1, &match->notif->requests);
 	wake_up_poll(&match->wqh, EPOLLIN | EPOLLRDNORM);
 
 	/*
@@ -1450,6 +1450,37 @@ find_notification(struct seccomp_filter *filter, u64 id)
 	return NULL;
 }
 
+static int recv_wake_function(wait_queue_entry_t *wait, unsigned int mode, int sync,
+				  void *key)
+{
+	/* Avoid a wakeup if event not interesting for us. */
+	if (key && !(key_to_poll(key) & (EPOLLIN | EPOLLERR)))
+		return 0;
+	return autoremove_wake_function(wait, mode, sync, key);
+}
+
+static int recv_wait_event(struct seccomp_filter *filter)
+{
+	DEFINE_WAIT_FUNC(wait, recv_wake_function);
+	int ret;
+
+	if (atomic_add_unless(&filter->notif->requests, -1, 0) != 0)
+		return 0;
+
+	for (;;) {
+		ret = prepare_to_wait_event(&filter->wqh, &wait, TASK_INTERRUPTIBLE);
+
+		if (atomic_add_unless(&filter->notif->requests, -1, 0) != 0)
+			break;
+
+		if (ret)
+			return ret;
+
+		schedule();
+	}
+	finish_wait(&filter->wqh, &wait);
+	return 0;
+}
 
 static long seccomp_notify_recv(struct seccomp_filter *filter,
 				void __user *buf)
@@ -1467,7 +1498,7 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
 
 	memset(&unotif, 0, sizeof(unotif));
 
-	ret = down_interruptible(&filter->notif->request);
+	ret = recv_wait_event(filter);
 	if (ret < 0)
 		return ret;
 
@@ -1515,7 +1546,8 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
 			if (should_sleep_killable(filter, knotif))
 				complete(&knotif->ready);
 			knotif->state = SECCOMP_NOTIFY_INIT;
-			up(&filter->notif->request);
+			atomic_add(1, &filter->notif->requests);
+			wake_up_poll(&filter->wqh, EPOLLIN | EPOLLRDNORM);
 		}
 		mutex_unlock(&filter->notify_lock);
 	}
@@ -1777,7 +1809,6 @@ static struct file *init_listener(struct seccomp_filter *filter)
 	if (!filter->notif)
 		goto out;
 
-	sema_init(&filter->notif->request, 0);
 	filter->notif->next_id = get_random_u64();
 	INIT_LIST_HEAD(&filter->notif->notifications);
 
-- 
2.38.1.493.g58b659f92b-goog


  reply	other threads:[~2022-11-11  7:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-11  7:31 [PATCH 0/5 v3] seccomp: add the synchronous mode for seccomp_unotify Andrei Vagin
2022-11-11  7:31 ` Andrei Vagin [this message]
2022-11-11  7:31 ` [PATCH 2/5] sched: add WF_CURRENT_CPU and externise ttwu Andrei Vagin
2022-11-11  7:31 ` [PATCH 3/5] sched: add a few helpers to wake up tasks on the current cpu Andrei Vagin
2022-11-11  7:31 ` [PATCH 4/5] seccomp: add the synchronous mode for seccomp_unotify Andrei Vagin
2022-11-11  7:31 ` [PATCH 5/5] selftest/seccomp: add a new test for the sync mode of seccomp_user_notify Andrei Vagin
2022-11-18 22:38 ` [PATCH 0/5 v3] seccomp: add the synchronous mode for seccomp_unotify Kees Cook
2022-11-22  7:52   ` Andrei Vagin
2022-12-06  6:52     ` Andrei Vagin
  -- strict thread matches above, loose matches on Subject: below --
2023-01-10 21:30 [PATCH 0/5 v3 RESEND] " Andrei Vagin
2023-01-10 21:30 ` [PATCH 1/5] seccomp: don't use semaphore and wait_queue together Andrei Vagin
2023-01-12 14:58   ` Tycho Andersen
2023-01-13 21:51     ` Andrei Vagin
2023-01-16  9:48   ` Peter Zijlstra
2022-10-20  1:10 [PATCH 0/5 v2] seccomp: add the synchronous mode for seccomp_unotify Andrei Vagin
2022-10-20  1:10 ` [PATCH 1/5] seccomp: don't use semaphore and wait_queue together Andrei Vagin
2022-10-20  5:10   ` Kees Cook
2022-10-21  4:30     ` Andrei Vagin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221111073154.784261-2-avagin@google.com \
    --to=avagin@google.com \
    --cc=avagin@gmail.com \
    --cc=brauner@kernel.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=posk@google.com \
    --cc=tycho@tycho.pizza \
    --cc=vincent.guittot@linaro.org \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.