All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tycho Andersen <tycho@kernel.org>
To: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Cc: kees@kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, "Andy Lutomirski" <luto@amacapital.net>,
	"Will Drewry" <wad@chromium.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <shuah@kernel.org>,
	"Andrei Vagin" <avagin@gmail.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Stéphane Graber" <stgraber@stgraber.org>
Subject: Re: [PATCH v1 4/6] seccomp: handle multiple listeners case
Date: Mon, 1 Dec 2025 07:38:53 -0700	[thread overview]
Message-ID: <aS2offcUPOkfkye1@tycho.pizza> (raw)
In-Reply-To: <20251201122406.105045-5-aleksandr.mikhalitsyn@canonical.com>

On Mon, Dec 01, 2025 at 01:24:01PM +0100, Alexander Mikhalitsyn wrote:
> If we have more than one listener in the tree and lower listener
> wants us to continue syscall (SECCOMP_USER_NOTIF_FLAG_CONTINUE)
> we must consult with upper listeners first, otherwise it is a
> clear seccomp restrictions bypass scenario.
> 
> Cc: linux-kernel@vger.kernel.org
> Cc: bpf@vger.kernel.org
> Cc: Kees Cook <kees@kernel.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Tycho Andersen <tycho@tycho.pizza>
> Cc: Andrei Vagin <avagin@gmail.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Stéphane Graber <stgraber@stgraber.org>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---
>  kernel/seccomp.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index ded3f6a6430b..ad733f849e0f 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -450,6 +450,9 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
>  			ret = cur_ret;
>  			matches->n = 1;
>  			matches->filters[0] = f;
> +		} else if ((ACTION_ONLY(cur_ret) == ACTION_ONLY(ret)) &&
> +			    ACTION_ONLY(cur_ret) == SECCOMP_RET_USER_NOTIF) {
> +			matches->filters[matches->n++] = f;
>  		}
>  	}
>  	return ret;
> @@ -1362,8 +1365,17 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
>  		return 0;
>  
>  	case SECCOMP_RET_USER_NOTIF:
> -		if (seccomp_do_user_notification(match, &sd))
> -			goto skip;
> +		for (unsigned char i = 0; i < matches.n; i++) {
> +			match = matches.filters[i];
> +			/*
> +			 * If userspace wants us to skip this syscall, do so.
> +			 * But if userspace wants to continue syscall, we
> +			 * must consult with the upper-level filters listeners
> +			 * and act accordingly.

This looks reasonable to me, pending whatever the outcome is of your
discussion of plumber's (I won't be there), feel free to add:

Reviewed-by: Tycho Andersen (AMD) <tycho@kernel.org>

I did have to think a bit about why matches.filters would be
guaranteed to have a user notification for this filter, but it's
because of your == check above in seccomp_run_filters(). Maybe worth
noting that here.

Tycho

  reply	other threads:[~2025-12-01 14:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01 12:23 [PATCH v1 0/6] seccomp: support nested listeners Alexander Mikhalitsyn
2025-12-01 12:23 ` [PATCH v1 1/6] seccomp: remove unused argument from seccomp_do_user_notification Alexander Mikhalitsyn
2025-12-01 14:19   ` Tycho Andersen
2025-12-02 11:56     ` Aleksandr Mikhalitsyn
2025-12-01 12:23 ` [PATCH v1 2/6] seccomp: prepare seccomp_run_filters() to support more than one listener Alexander Mikhalitsyn
2025-12-01 14:24   ` Tycho Andersen
2025-12-02 11:58     ` Aleksandr Mikhalitsyn
2025-12-02 14:06       ` Tycho Andersen
2025-12-02 20:26   ` Kees Cook
2025-12-03 15:25     ` Aleksandr Mikhalitsyn
2025-12-01 12:24 ` [PATCH v1 3/6] seccomp: limit number of listeners in seccomp tree Alexander Mikhalitsyn
2025-12-01 12:24 ` [PATCH v1 4/6] seccomp: handle multiple listeners case Alexander Mikhalitsyn
2025-12-01 14:38   ` Tycho Andersen [this message]
2025-12-02 12:01     ` Aleksandr Mikhalitsyn
2025-12-01 12:24 ` [PATCH v1 5/6] seccomp: relax has_duplicate_listeners check Alexander Mikhalitsyn
2025-12-01 12:24 ` [PATCH v1 6/6] tools/testing/selftests/seccomp: test nested listeners Alexander Mikhalitsyn

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=aS2offcUPOkfkye1@tycho.pizza \
    --to=tycho@kernel.org \
    --cc=aleksandr.mikhalitsyn@canonical.com \
    --cc=avagin@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=corbet@lwn.net \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=shuah@kernel.org \
    --cc=stgraber@stgraber.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.