[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git-commits-head
Subject:    seccomp: Refactor notification handler to prepare for new semantics
From:       Linux Kernel Mailing List <linux-kernel () vger ! kernel ! org>
Date:       2021-05-30 4:30:47
Message-ID: git-mailbomb-linux-master-ddc473916955f7710d1eb17c1273d91c8622a9fe () kernel ! org
[Download RAW message or body]

Commit:     ddc473916955f7710d1eb17c1273d91c8622a9fe
Parent:     aac902925ea646e461c95edc98a8a57eb0def917
Refname:    refs/heads/master
Web:        https://git.kernel.org/torvalds/c/ddc473916955f7710d1eb17c1273d91c8622a9fe
Author:     Sargun Dhillon <sargun@sargun.me>
AuthorDate: Mon May 17 12:39:06 2021 -0700
Committer:  Kees Cook <keescook@chromium.org>
CommitDate: Sat May 29 11:13:27 2021 -0700

    seccomp: Refactor notification handler to prepare for new semantics
    
    This refactors the user notification code to have a do / while loop around
    the completion condition. This has a small change in semantic, in that
    previously we ignored addfd calls upon wakeup if the notification had been
    responded to, but instead with the new change we check for an outstanding
    addfd calls prior to returning to userspace.
    
    Rodrigo Campos also identified a bug that can result in addfd causing
    an early return, when the supervisor didn't actually handle the
    syscall [1].
    
    [1]: https://lore.kernel.org/lkml/20210413160151.3301-1-rodrigo@kinvolk.io/
    
    Fixes: 7cf97b125455 ("seccomp: Introduce addfd ioctl to seccomp user notifier")
    Signed-off-by: Sargun Dhillon <sargun@sargun.me>
    Acked-by: Tycho Andersen <tycho@tycho.pizza>
    Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
    Signed-off-by: Kees Cook <keescook@chromium.org>
    Tested-by: Rodrigo Campos <rodrigo@kinvolk.io>
    Cc: stable@vger.kernel.org
    Link: https://lore.kernel.org/r/20210517193908.3113-3-sargun@sargun.me
---
 kernel/seccomp.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 6ecd3f3a52b5b..9f58049ac16d9 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1105,28 +1105,30 @@ static int seccomp_do_user_notification(int this_syscall,
 
 	up(&match->notif->request);
 	wake_up_poll(&match->wqh, EPOLLIN | EPOLLRDNORM);
-	mutex_unlock(&match->notify_lock);
 
 	/*
 	 * This is where we wait for a reply from userspace.
 	 */
-wait:
-	err = wait_for_completion_interruptible(&n.ready);
-	mutex_lock(&match->notify_lock);
-	if (err == 0) {
-		/* Check if we were woken up by a addfd message */
+	do {
+		mutex_unlock(&match->notify_lock);
+		err = wait_for_completion_interruptible(&n.ready);
+		mutex_lock(&match->notify_lock);
+		if (err != 0)
+			goto interrupted;
+
 		addfd = list_first_entry_or_null(&n.addfd,
 						 struct seccomp_kaddfd, list);
-		if (addfd && n.state != SECCOMP_NOTIFY_REPLIED) {
+		/* Check if we were woken up by a addfd message */
+		if (addfd)
 			seccomp_handle_addfd(addfd);
-			mutex_unlock(&match->notify_lock);
-			goto wait;
-		}
-		ret = n.val;
-		err = n.error;
-		flags = n.flags;
-	}
 
+	}  while (n.state != SECCOMP_NOTIFY_REPLIED);
+
+	ret = n.val;
+	err = n.error;
+	flags = n.flags;
+
+interrupted:
 	/* If there were any pending addfd calls, clear them out */
 	list_for_each_entry_safe(addfd, tmp, &n.addfd, list) {
 		/* The process went away before we got a chance to handle it */
[prev in list] [next in list] [prev in thread] [next in thread]