Skip to content

Commit

Permalink
seccomp: Refactor notification handler to prepare for new semantics
Browse files Browse the repository at this point in the history
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: 7cf97b1 ("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
  • Loading branch information
sargun authored and kees committed May 29, 2021
1 parent aac9029 commit ddc4739
Showing 1 changed file with 16 additions and 14 deletions.
30 changes: 16 additions & 14 deletions kernel/seccomp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit ddc4739

Please sign in to comment.