All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tycho Andersen <tycho@tycho.pizza>
To: Rodrigo Campos <rodrigo@kinvolk.io>
Cc: Giuseppe Scrivano <gscrivan@redhat.com>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	Linux Containers <containers@lists.linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Alban Crequy <alban@kinvolk.io>,
	Andy Lutomirski <luto@kernel.org>,
	Christian Brauner <christian.brauner@ubuntu.com>
Subject: Re: [PATCH RESEND 2/5] seccomp: Add wait_killable semantic to seccomp user notifier
Date: Wed, 28 Apr 2021 08:08:17 -0600	[thread overview]
Message-ID: <20210428140817.GA1965193@cisco> (raw)
In-Reply-To: <CACaBj2aSnzQnbZG0sMM2Vae_yWGzxKWrGUz9xJVL_7akF8DvNA@mail.gmail.com>

On Wed, Apr 28, 2021 at 03:20:02PM +0200, Rodrigo Campos wrote:
> On Wed, Apr 28, 2021 at 1:10 PM Rodrigo Campos <rodrigo@kinvolk.io> wrote:
> >
> > On Wed, Apr 28, 2021 at 2:22 AM Tycho Andersen <tycho@tycho.pizza> wrote:
> > >
> > > On Tue, Apr 27, 2021 at 04:19:54PM -0700, Andy Lutomirski wrote:
> > > > User notifiers should allow correct emulation.  Right now, it doesn't,
> > > > but there is no reason it can't.
> > >
> > > Thanks for the explanation.
> > >
> > > Consider fsmount, which has a,
> > >
> > >         ret = mutex_lock_interruptible(&fc->uapi_mutex);
> > >         if (ret < 0)
> > >                 goto err_fsfd;
> > >
> > > If a regular task is interrupted during that wait, it return -EINTR
> > > or whatever back to userspace.
> > >
> > > Suppose that we intercept fsmount. The supervisor decides the mount is
> > > OK, does the fsmount, injects the mount fd into the container, and
> > > then the tracee receives a signal. At this point, the mount fd is
> > > visible inside the container. The supervisor gets a notification about
> > > the signal and revokes the mount fd, but there was some time where it
> > > was exposed in the container, whereas with the interrupt in the native
> > > syscall there was never any exposure.
> >
> > IIUC, this is solved by my patch, patch 4 of the series. The
> > supervisor should do the addfd with the flag added in that patch
> > (SECCOMP_ADDFD_FLAG_SEND) for an atomic "addfd + send".
> 
> Well, under Andy's proposal handling that is even simpler. If the
> signal is delivered after we added the fd (note that the container
> syscall does not return when the signal arrives, as it happens today,
> it just signals the notifier and continues to wait), we can just
> ignore the signal and return that (if that is the appropriate thing
> for that syscall, but I guess after adding an fd there isn't any other
> reasonable thing to do).

Yes, agreed. After thinking about this more, my example is bogus: the
kernel doesn't sleep after it installs the fd, so it would ignore any
signals too.

Even if the kernel *did* sleep after installing the fd, it would still
be correct emulation to install it and then do whatever the kernel did
during that sleep. So I withdraw my objection :)

Thanks,

Tycho
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

WARNING: multiple messages have this Message-ID (diff)
From: Tycho Andersen <tycho@tycho.pizza>
To: Rodrigo Campos <rodrigo@kinvolk.io>
Cc: "Andy Lutomirski" <luto@kernel.org>,
	"Sargun Dhillon" <sargun@sargun.me>,
	"Kees Cook" <keescook@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Linux Containers" <containers@lists.linux-foundation.org>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	"Mauricio Vásquez Bernal" <mauricio@kinvolk.io>,
	"Giuseppe Scrivano" <gscrivan@redhat.com>,
	"Will Drewry" <wad@chromium.org>,
	"Alban Crequy" <alban@kinvolk.io>
Subject: Re: [PATCH RESEND 2/5] seccomp: Add wait_killable semantic to seccomp user notifier
Date: Wed, 28 Apr 2021 08:08:17 -0600	[thread overview]
Message-ID: <20210428140817.GA1965193@cisco> (raw)
In-Reply-To: <CACaBj2aSnzQnbZG0sMM2Vae_yWGzxKWrGUz9xJVL_7akF8DvNA@mail.gmail.com>

On Wed, Apr 28, 2021 at 03:20:02PM +0200, Rodrigo Campos wrote:
> On Wed, Apr 28, 2021 at 1:10 PM Rodrigo Campos <rodrigo@kinvolk.io> wrote:
> >
> > On Wed, Apr 28, 2021 at 2:22 AM Tycho Andersen <tycho@tycho.pizza> wrote:
> > >
> > > On Tue, Apr 27, 2021 at 04:19:54PM -0700, Andy Lutomirski wrote:
> > > > User notifiers should allow correct emulation.  Right now, it doesn't,
> > > > but there is no reason it can't.
> > >
> > > Thanks for the explanation.
> > >
> > > Consider fsmount, which has a,
> > >
> > >         ret = mutex_lock_interruptible(&fc->uapi_mutex);
> > >         if (ret < 0)
> > >                 goto err_fsfd;
> > >
> > > If a regular task is interrupted during that wait, it return -EINTR
> > > or whatever back to userspace.
> > >
> > > Suppose that we intercept fsmount. The supervisor decides the mount is
> > > OK, does the fsmount, injects the mount fd into the container, and
> > > then the tracee receives a signal. At this point, the mount fd is
> > > visible inside the container. The supervisor gets a notification about
> > > the signal and revokes the mount fd, but there was some time where it
> > > was exposed in the container, whereas with the interrupt in the native
> > > syscall there was never any exposure.
> >
> > IIUC, this is solved by my patch, patch 4 of the series. The
> > supervisor should do the addfd with the flag added in that patch
> > (SECCOMP_ADDFD_FLAG_SEND) for an atomic "addfd + send".
> 
> Well, under Andy's proposal handling that is even simpler. If the
> signal is delivered after we added the fd (note that the container
> syscall does not return when the signal arrives, as it happens today,
> it just signals the notifier and continues to wait), we can just
> ignore the signal and return that (if that is the appropriate thing
> for that syscall, but I guess after adding an fd there isn't any other
> reasonable thing to do).

Yes, agreed. After thinking about this more, my example is bogus: the
kernel doesn't sleep after it installs the fd, so it would ignore any
signals too.

Even if the kernel *did* sleep after installing the fd, it would still
be correct emulation to install it and then do whatever the kernel did
during that sleep. So I withdraw my objection :)

Thanks,

Tycho

  reply	other threads:[~2021-04-28 14:08 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 18:06 [PATCH RESEND 0/5] Handle seccomp notification preemption Sargun Dhillon
2021-04-26 18:06 ` Sargun Dhillon
2021-04-26 18:06 ` [PATCH RESEND 1/5] seccomp: Refactor notification handler to prepare for new semantics Sargun Dhillon
2021-04-26 18:06   ` Sargun Dhillon
2021-04-26 18:06 ` [PATCH RESEND 2/5] seccomp: Add wait_killable semantic to seccomp user notifier Sargun Dhillon
2021-04-26 18:06   ` Sargun Dhillon
2021-04-26 19:02   ` Tycho Andersen
2021-04-26 19:02     ` Tycho Andersen
2021-04-26 22:15     ` Sargun Dhillon
2021-04-26 22:15       ` Sargun Dhillon
2021-04-27 13:48       ` Tycho Andersen
2021-04-27 13:48         ` Tycho Andersen
2021-04-27 16:23         ` Andy Lutomirski
2021-04-27 16:23           ` Andy Lutomirski
2021-04-27 17:07           ` Tycho Andersen
2021-04-27 17:07             ` Tycho Andersen
2021-04-27 22:10             ` Sargun Dhillon
2021-04-27 22:10               ` Sargun Dhillon
2021-04-27 23:19               ` Andy Lutomirski
2021-04-27 23:19                 ` Andy Lutomirski
2021-04-28  0:22                 ` Tycho Andersen
2021-04-28  0:22                   ` Tycho Andersen
2021-04-28 11:10                   ` Rodrigo Campos
2021-04-28 11:10                     ` Rodrigo Campos
2021-04-28 13:20                     ` Rodrigo Campos
2021-04-28 13:20                       ` Rodrigo Campos
2021-04-28 14:08                       ` Tycho Andersen [this message]
2021-04-28 14:08                         ` Tycho Andersen
2021-04-28 17:13                         ` Sargun Dhillon
2021-04-28 17:13                           ` Sargun Dhillon
2021-04-28  3:20                 ` Sargun Dhillon
2021-04-28  3:20                   ` Sargun Dhillon
2021-04-27 16:34         ` Sargun Dhillon
2021-04-27 16:34           ` Sargun Dhillon
2021-04-26 18:06 ` [PATCH RESEND 3/5] selftests/seccomp: Add test for wait killable notifier Sargun Dhillon
2021-04-26 18:06   ` Sargun Dhillon
2021-04-26 18:51   ` Tycho Andersen
2021-04-26 18:51     ` Tycho Andersen
2021-04-26 18:06 ` [PATCH RESEND 4/5] seccomp: Support atomic "addfd + send reply" Sargun Dhillon
2021-04-26 18:06   ` Sargun Dhillon
2021-04-26 18:06 ` [PATCH RESEND 5/5] selftests/seccomp: Add test for atomic addfd+send Sargun Dhillon
2021-04-26 18:06   ` Sargun Dhillon

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=20210428140817.GA1965193@cisco \
    --to=tycho@tycho.pizza \
    --cc=alban@kinvolk.io \
    --cc=christian.brauner@ubuntu.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=gscrivan@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=rodrigo@kinvolk.io \
    --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.