2022-04-28 07:53:09

by Sargun Dhillon

[permalink] [raw]
Subject: [PATCH 1/2] seccomp: Use FIFO semantics to order notifications

Previously, the seccomp notifier used LIFO semantics, where each
notification would be added on top of the stack, and notifications
were popped off the top of the stack. This could result one process
that generates a large number of notifications preventing other
notifications from being handled. This patch moves from LIFO (stack)
semantics to FIFO (queue semantics).

Signed-off-by: Sargun Dhillon <[email protected]>
---
kernel/seccomp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index db10e73d06e0..2cb3bcd90eb3 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1101,7 +1101,7 @@ static int seccomp_do_user_notification(int this_syscall,
n.data = sd;
n.id = seccomp_next_notify_id(match);
init_completion(&n.ready);
- list_add(&n.list, &match->notif->notifications);
+ list_add_tail(&n.list, &match->notif->notifications);
INIT_LIST_HEAD(&n.addfd);

up(&match->notif->request);
--
2.25.1


2022-04-28 09:45:19

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 1/2] seccomp: Use FIFO semantics to order notifications

On Wed, Apr 27, 2022 at 06:54:46PM -0700, Sargun Dhillon wrote:
> Previously, the seccomp notifier used LIFO semantics, where each
> notification would be added on top of the stack, and notifications
> were popped off the top of the stack. This could result one process
> that generates a large number of notifications preventing other
> notifications from being handled. This patch moves from LIFO (stack)
> semantics to FIFO (queue semantics).
>
> Signed-off-by: Sargun Dhillon <[email protected]>
> ---

It has a minimal user-visible impact in a sense but I don't think it
should be an issue. Makes sense to me,
Reviewed-by: Christian Brauner (Microsoft) <[email protected]>

2022-05-03 00:42:57

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 1/2] seccomp: Use FIFO semantics to order notifications

On Wed, 27 Apr 2022 18:54:46 -0700, Sargun Dhillon wrote:
> Previously, the seccomp notifier used LIFO semantics, where each
> notification would be added on top of the stack, and notifications
> were popped off the top of the stack. This could result one process
> that generates a large number of notifications preventing other
> notifications from being handled. This patch moves from LIFO (stack)
> semantics to FIFO (queue semantics).
>
> [...]

Applied (which comment typo fix) to for-next/seccomp, thanks!

[1/2] seccomp: Use FIFO semantics to order notifications
https://git.kernel.org/kees/c/4cbf6f621150
[2/2] selftests/seccomp: Ensure that notifications come in FIFO order
https://git.kernel.org/kees/c/662340ef9218

--
Kees Cook