From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 980DBC47076 for ; Fri, 21 May 2021 15:46:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 751F3613EA for ; Fri, 21 May 2021 15:46:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237491AbhEUPrE (ORCPT ); Fri, 21 May 2021 11:47:04 -0400 Received: from mail.kernel.org ([198.145.29.99]:57210 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237439AbhEUPrB (ORCPT ); Fri, 21 May 2021 11:47:01 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 35C1D61401; Fri, 21 May 2021 15:45:34 +0000 (UTC) Date: Fri, 21 May 2021 17:45:32 +0200 From: Christian Brauner To: Sargun Dhillon Cc: Kees Cook , LKML , containers@lists.linux.dev, Tycho Andersen , Andy Lutomirski , Rodrigo Campos , Mauricio =?utf-8?Q?V=C3=A1squez?= Bernal , Giuseppe Scrivano , =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= Subject: Re: [PATCH v2 3/4] seccomp: Support atomic "addfd + send reply" Message-ID: <20210521154532.utttvofngdg6qeob@wittgenstein> References: <20210517193908.3113-1-sargun@sargun.me> <20210517193908.3113-4-sargun@sargun.me> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210517193908.3113-4-sargun@sargun.me> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 17, 2021 at 12:39:07PM -0700, Sargun Dhillon wrote: > From: Rodrigo Campos > > Alban Crequy reported a race condition userspace faces when we want to > add some fds and make the syscall return them[1] using seccomp notify. > > The problem is that currently two different ioctl() calls are needed by > the process handling the syscalls (agent) for another userspace process > (target): SECCOMP_IOCTL_NOTIF_ADDFD to allocate the fd and > SECCOMP_IOCTL_NOTIF_SEND to return that value. Therefore, it is possible > for the agent to do the first ioctl to add a file descriptor but the > target is interrupted (EINTR) before the agent does the second ioctl() > call. > > This patch adds a flag to the ADDFD ioctl() so it adds the fd and > returns that value atomically to the target program, as suggested by > Kees Cook[2]. This is done by simply allowing > seccomp_do_user_notification() to add the fd and return it in this case. > Therefore, in this case the target wakes up from the wait in > seccomp_do_user_notification() either to interrupt the syscall or to add > the fd and return it. > > This "allocate an fd and return" functionality is useful for syscalls > that return a file descriptor only, like connect(2). Other syscalls that > return a file descriptor but not as return value (or return more than > one fd), like socketpair(), pipe(), recvmsg with SCM_RIGHTs, will not > work with this flag. > > This effectively combines SECCOMP_IOCTL_NOTIF_ADDFD and > SECCOMP_IOCTL_NOTIF_SEND into an atomic opteration. The notification's > return value, nor error can be set by the user. Upon successful invocation > of the SECCOMP_IOCTL_NOTIF_ADDFD ioctl with the SECCOMP_ADDFD_FLAG_SEND > flag, the notifying process's errno will be 0, and the return value will > be the file descriptor number that was installed. > > [1]: https://lore.kernel.org/lkml/CADZs7q4sw71iNHmV8EOOXhUKJMORPzF7thraxZYddTZsxta-KQ@mail.gmail.com/ > [2]: https://lore.kernel.org/lkml/202012011322.26DCBC64F2@keescook/ > > Signed-off-by: Rodrigo Campos > Signed-off-by: Sargun Dhillon > Acked-by: Tycho Andersen > --- > .../userspace-api/seccomp_filter.rst | 12 +++++ > include/uapi/linux/seccomp.h | 1 + > kernel/seccomp.c | 49 +++++++++++++++++-- > 3 files changed, 58 insertions(+), 4 deletions(-) > > diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst > index 6efb41cc8072..d61219889e49 100644 > --- a/Documentation/userspace-api/seccomp_filter.rst > +++ b/Documentation/userspace-api/seccomp_filter.rst > @@ -259,6 +259,18 @@ and ``ioctl(SECCOMP_IOCTL_NOTIF_SEND)`` a response, indicating what should be > returned to userspace. The ``id`` member of ``struct seccomp_notif_resp`` should > be the same ``id`` as in ``struct seccomp_notif``. > > +Userspace can also add file descriptors to the notifying process via > +``ioctl(SECCOMP_IOCTL_NOTIF_ADDFD)``. The ``id`` member of > +``struct seccomp_notif_addfd`` should be the same ``id`` as in > +``struct seccomp_notif``. The ``newfd_flags`` flag may be used to set flags > +like O_EXEC on the file descriptor in the notifying process. If the supervisor nit: s/O_EXEC/O_CLOEXEC/