Re: [PATCH] pidfd: add ioctl to retrieve pid info

[Luca Boccassi]

On Wed, 2 Oct 2024 at 15:48, Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 2, 2024 at 10:25 AM <luca.boccassi@xxxxxxxxx> wrote:
> >
> > From: Luca Boccassi <bluca@xxxxxxxxxx>
> >
> > A common pattern when using pid fds is having to get information
> > about the process, which currently requires /proc being mounted,
> > resolving the fd to a pid, and then do manual string parsing of
> > /proc/N/status and friends. This needs to be reimplemented over
> > and over in all userspace projects (e.g.: I have reimplemented
> > resolving in systemd, dbus, dbus-daemon, polkit so far), and
> > requires additional care in checking that the fd is still valid
> > after having parsed the data, to avoid races.
> >
> > Having a programmatic API that can be used directly removes all
> > these requirements, including having /proc mounted.
> >
> > As discussed at LPC24, add an ioctl with an extensible struct
> > so that more parameters can be added later if needed. Start with
> > exposing: pid, uid, gid, groupid, security label (the latter was
> > requested by the LSM maintainer).
> >
> > Signed-off-by: Luca Boccassi <bluca@xxxxxxxxxx>
> > ---
> >  fs/pidfs.c                                    | 61 ++++++++++++++++++-
> >  include/uapi/linux/pidfd.h                    | 17 ++++++
> >  .../testing/selftests/pidfd/pidfd_open_test.c | 50 ++++++++++++++-
> >  3 files changed, 126 insertions(+), 2 deletions(-)
>
> ...
>
> > diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h
> > index 565fc0629fff..bfd0965e01f3 100644
> > --- a/include/uapi/linux/pidfd.h
> > +++ b/include/uapi/linux/pidfd.h
> > @@ -16,6 +16,22 @@
> >  #define PIDFD_SIGNAL_THREAD_GROUP      (1UL << 1)
> >  #define PIDFD_SIGNAL_PROCESS_GROUP     (1UL << 2)
> >
> > +/* Flags for pidfd_info. */
> > +#define PIDFD_INFO_PID                 (1UL << 0)
> > +#define PIDFD_INFO_CREDS                   (1UL << 1)
> > +#define PIDFD_INFO_CGROUPID                (1UL << 2)
> > +#define PIDFD_INFO_SECURITY_CONTEXT        (1UL << 3)
> > +
> > +struct pidfd_info {
> > +        __u64 request_mask;
> > +        __u32 size;
> > +        uint pid;
> > +        uint uid;
> > +        uint gid;
> > +        __u64 cgroupid;
> > +        char security_context[NAME_MAX];
>
> [NOTE: please CC the LSM list on changes like this]
>
> Thanks Luca :)
>
> With the addition of the LSM syscalls we've created a lsm_ctx struct
> (see include/uapi/linux/lsm.h) that properly supports multiple LSMs.
> The original char ptr "secctx" approach worked back when only a single
> LSM was supported at any given time, but now that multiple LSMs are
> supported we need something richer, and it would be good to use this
> new struct in any new userspace API.
>
> See the lsm_get_self_attr(2) syscall for an example (defined in
> security/lsm_syscalls.c but effectively implemented via
> security_getselfattr() in security/security.c).

Thanks for the review, makes sense to me - I had a look at those
examples but unfortunately it is getting a bit beyond my (very low)
kernel skills, so I've dropped the string-based security_context from
v2 but without adding something else, is there someone more familiar
with the LSM world that could help implementing that side?