Received-SPF: pass (google.com: domain of linux-kernel+bounces-188982-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240523014540.372255-1-avagin@google.com> <20240523014540.372255-2-avagin@google.com>
 <20240523085859.GB15163@redhat.com>
In-Reply-To: <20240523085859.GB15163@redhat.com>
From: Andrei Vagin <avagin@google.com>
Date: Fri, 24 May 2024 10:47:47 -0700
Message-ID: <CAEWA0a43nYT+YVXWLhv8THL+gu3QLVKznPppjQ_mWina2c-HJw@mail.gmail.com>
Subject: Re: [PATCH 1/3] seccomp: interrupt SECCOMP_IOCTL_NOTIF_RECV when all
 users have exited
To: Oleg Nesterov <oleg@redhat.com>
Cc: Kees Cook <keescook@chromium.org>, Andy Lutomirski <luto@amacapital.net>, 
	Will Drewry <wad@chromium.org>, Christian Brauner <brauner@kernel.org>, linux-kernel@vger.kernel.org, 
	Tycho Andersen <tandersen@netflix.com>, Jens Axboe <axboe@kernel.dk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 23, 2024 at 2:00=E2=80=AFAM Oleg Nesterov <oleg@redhat.com> wro=
te:
>
> Hi Andrei,
>
> the patch looks good to me even if I don't really understand what
> SECCOMP_IOCTL_NOTIF_RECV does. But let me ask a stupid question,
>
> On 05/23, Andrei Vagin wrote:
> >
> > The change is based on the 'commit 99cdb8b9a573 ("seccomp: notify about
> > unused filter")' which implemented (E)POLLHUP notifications.
>
> To me this patch fixes the commit above, because without this change

It depends on how we look at it. I think the intention was to address
the epoll/poll/select syscalls to return (E)POLLHUP notifications when
filters have been orphaned. Plus, this code looked a bit different that
time and recv_wake_function used another notification mechanism.

>
> > @@ -1466,7 +1466,7 @@ static int recv_wake_function(wait_queue_entry_t =
*wait, unsigned int mode, int s
> >                                 void *key)
> >  {
> >       /* Avoid a wakeup if event not interesting for us. */
> > -     if (key && !(key_to_poll(key) & (EPOLLIN | EPOLLERR)))
> > +     if (key && !(key_to_poll(key) & (EPOLLIN | EPOLLERR | EPOLLHUP)))
>
> __seccomp_filter_orphan() -> wake_up_poll(&orig->wqh, EPOLLHUP) won't
> wakeup the task sleeping in recv_wait_event(), right ?
>
> In any case, FWIW
>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>

Thanks,
Andrei