Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230522132439.634031-1-aleksandr.mikhalitsyn@canonical.com>
 <20230522132439.634031-2-aleksandr.mikhalitsyn@canonical.com>
 <20230522133409.5c6e839a@kernel.org> <20230523-flechten-ortsschild-e5724ecc4ed0@brauner>
In-Reply-To: <20230523-flechten-ortsschild-e5724ecc4ed0@brauner>
From:   Luca Boccassi <bluca@debian.org>
Date:   Tue, 23 May 2023 11:44:01 +0100
Message-ID: <CAMw=ZnS8GBTDV0rw+Dh6hPv3uLXJVwapRFQHLMYEYGZHNoLNOw@mail.gmail.com>
Subject: Re: [PATCH net-next v6 1/3] scm: add SO_PASSPIDFD and SCM_PIDFD
To:     Christian Brauner <brauner@kernel.org>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>,
        davem@davemloft.net, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Leon Romanovsky <leon@kernel.org>,
        David Ahern <dsahern@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Kees Cook <keescook@chromium.org>,
        Kuniyuki Iwashima <kuniyu@amazon.com>,
        Lennart Poettering <mzxreary@0pointer.de>,
        linux-arch@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 23 May 2023 at 10:49, Christian Brauner <brauner@kernel.org> wrote:
>
> On Mon, May 22, 2023 at 01:34:09PM -0700, Jakub Kicinski wrote:
> > On Mon, 22 May 2023 15:24:37 +0200 Alexander Mikhalitsyn wrote:
> > > v6:
> > >     - disable feature when CONFIG_UNIX=n/m (pidfd_prepare API is not exported to modules)
> >
> > IMHO hiding the code under #if IS_BUILTIN(CONFIG_UNRELATED) is
> > surprising to the user and.. ugly?
> >
> > Can we move scm_pidfd_recv() into a C source and export that?
> > That should be less controversial than exporting pidfd_prepare()
> > directly?
>
> I really would like to avoid that because it will just mean that someone
> else will abuse that function and then make an argument why we should
> export the other function.
>
> I think it would be ok if we required that unix support is built in
> because it's not unprecedented either and we're not breaking anything.
> Bpf has the same requirement:
>
>   #if IS_BUILTIN(CONFIG_UNIX) && defined(CONFIG_BPF_SYSCALL)
>   struct bpf_unix_iter_state {
>           struct seq_net_private p;
>           unsigned int cur_sk;
>           unsigned int end_sk;
>           unsigned int max_sk;
>           struct sock **batch;
>           bool st_bucket_done;
>   };
>
> and
>
>   #if IS_BUILTIN(CONFIG_UNIX) && defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_PROC_FS)
>   DEFINE_BPF_ITER_FUNC(unix, struct bpf_iter_meta *meta,
>                        struct unix_sock *unix_sk, uid_t uid)

Some data points: Debian, Ubuntu, Fedora, RHEL, CentOS, Archlinux all
ship with CONFIG_UNIX=y, so a missing SCM_PIDFD in unlikely to have a
widespread impact, and if it does, it might encourage someone to
review their kconfig.

As mentioned on the v5 thread, we are waiting for this API to get the
userspace side sorted (systemd/dbus/dbus-broker/polkit), so I'd be
really grateful if we could start with the simplest and most
conservative approach (which seems to be the current one in v6 to me),
and then eventually later decide whether to export more functions, or
to deprecate CONFIG_UNIX=m, or something else entirely, as that
doesn't really affect the shape of the UAPI, just the details of its
availability. Thank you.

Kind regards,
Luca Boccassi