This patch set addresses two problems with the SECCOMP_IOCTL_NOTIF_RECV
ioctl:
* it doesn't return when the seccomp filter becomes unused (all tasks
  have exited).
* EPOLLHUP is triggered not when a task exits, but rather when its zombie
  is collected.

v2: - Remove unnecessary checks of PF_EXITING.
    - Take siglock with disabling irqs.
    Thanks to Oleg for the review and the help with the first version.

Andrei Vagin (3):
  seccomp: interrupt SECCOMP_IOCTL_NOTIF_RECV when all users have exited
  seccomp: release task filters when the task exits
  selftests/seccomp: add test for NOTIF_RECV and unused filters

 kernel/exit.c                                 |  3 +-
 kernel/seccomp.c                              | 38 ++++++++++---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 54 +++++++++++++++++++
 3 files changed, 88 insertions(+), 7 deletions(-)

Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Tycho Andersen <tandersen@netflix.com>


-- 
2.45.0.rc1.225.g2a3ae87e7f-goog

Kees,

Are you waiting for anything from me? I think this series is ready to be merged.

Thanks,
Andrei

On Wed, May 22, 2024 at 6:45 PM Andrei Vagin <avagin@google.com> wrote:
>
> This patch set addresses two problems with the SECCOMP_IOCTL_NOTIF_RECV
> ioctl:
> * it doesn't return when the seccomp filter becomes unused (all tasks
>   have exited).
> * EPOLLHUP is triggered not when a task exits, but rather when its zombie
>   is collected.
>
> v2: - Remove unnecessary checks of PF_EXITING.
>     - Take siglock with disabling irqs.
>     Thanks to Oleg for the review and the help with the first version.
>
> Andrei Vagin (3):
>   seccomp: interrupt SECCOMP_IOCTL_NOTIF_RECV when all users have exited
>   seccomp: release task filters when the task exits
>   selftests/seccomp: add test for NOTIF_RECV and unused filters
>
>  kernel/exit.c                                 |  3 +-
>  kernel/seccomp.c                              | 38 ++++++++++---
>  tools/testing/selftests/seccomp/seccomp_bpf.c | 54 +++++++++++++++++++
>  3 files changed, 88 insertions(+), 7 deletions(-)
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Tycho Andersen <tandersen@netflix.com>
>
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>

On Mon, Jun 24, 2024 at 05:17:09PM -0700, Andrei Vagin wrote:
> Are you waiting for anything from me? I think this series is ready to be merged.

Oops, sorry for the silence! I had been waiting for Oleg's review, and
then it happened and I missed it, so it fell off my TODO list. :)

I just sent a reply with 2 bits of feedback, but with a v3, I think I
can land this ASAP.

Thanks for chasing me down, and I appreciate the selftest updates!

-Kees

-- 
Kees Cook

On 05/23, Andrei Vagin wrote:
>
> This patch set addresses two problems with the SECCOMP_IOCTL_NOTIF_RECV
> ioctl:
> * it doesn't return when the seccomp filter becomes unused (all tasks
>   have exited).
> * EPOLLHUP is triggered not when a task exits, but rather when its zombie
>   is collected.

It seems that 2/3 also fixes another minor problem.

Suppose that a group leader installs the new filter without
SECCOMP_FILTER_FLAG_TSYNC, exits, and becomes a zombie. It can't be
released until all its sub-threads exit.

After that, without 2/3, SECCOMP_FILTER_FLAG_TSYNC from any other thread
can never succeed, seccomp_can_sync_threads() will check a zombie leader
and is_ancestor() will fail.

Right?

Oleg.

On Thu, May 23, 2024 at 2:35 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 05/23, Andrei Vagin wrote:
> >
> > This patch set addresses two problems with the SECCOMP_IOCTL_NOTIF_RECV
> > ioctl:
> > * it doesn't return when the seccomp filter becomes unused (all tasks
> >   have exited).
> > * EPOLLHUP is triggered not when a task exits, but rather when its zombie
> >   is collected.
>
> It seems that 2/3 also fixes another minor problem.
>
> Suppose that a group leader installs the new filter without
> SECCOMP_FILTER_FLAG_TSYNC, exits, and becomes a zombie. It can't be
> released until all its sub-threads exit.
>
> After that, without 2/3, SECCOMP_FILTER_FLAG_TSYNC from any other thread
> can never succeed, seccomp_can_sync_threads() will check a zombie leader
> and is_ancestor() will fail.
>
> Right?

It is right. I can introduce a self test for this case too, but let's
do that in a separate patch set.

>
> Oleg.
>