From nobody Wed Nov 27 19:41:02 2024 Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 566FD1DEFD7; Tue, 8 Oct 2024 13:20:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728393660; cv=none; b=KIpdiTJvv6SrVy6VWTHytBSHhw2FJw8WEyXXX5DSWhADzr13FbX4EVrpm9h0VaXdRDVEjXamwtVADAHW0RBGE9jehpx430LYQ/SQKAaxPvY9tU2/iFX9M4vBC2z1ZTd3KeaBlV5pmf0nfnik9x1lTwcPGA588iJeQDL1z087nlo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728393660; c=relaxed/simple; bh=QZEXC1BaE9H07q0O3c4vPr9WYVJ1wliJTTYPhKg9nIU=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=F4XgxBFz2WN8LKGFdS2mSP2Oh9WyXEPm8+goIZs9j7sfwi4cpF6tJtYxkiGgU0h4tEmHUpyBa8rNu7mbALN5tEW0lKVDBrXfmb2DGfXs1VYB7VEgYckYWTaHWzcyLxKmJC2iril076lPJEJWdbLEycjSMKboaj+CpVeEcRT2Na0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=PH1h1/Mw; arc=none smtp.client-ip=209.85.221.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="PH1h1/Mw" Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-37d2ddb61c8so606473f8f.2; Tue, 08 Oct 2024 06:20:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1728393656; x=1728998456; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=wnv8F+1ufi0FZk0Wia60FxxOpLB4CVsWaIwlw7XIvuU=; b=PH1h1/MwvWtH1jizXWQexo2wzPJZI80FxtT+L8vmftdaNm+GCOkfjeYmOBF3EsuMg9 nuca8mmd5PL8p+MLRZwXh6ZvVz+XS2T0lR7fdGgxIkn8xHs39Emk8FksGeGM2NzD+ExU 4n0CBdCu2ymuewnOuTNXeWd29rZe8FvYXx/SbomTcOxW6bpdj4QsZ89vnvfI0wdEQJMW +uijcMQnbsDtB3JmT3hEgrXl//gBPF8UVnFzW25FPdVyC/j0u7FGdNGlpvDfGUc2RxD2 U5yGkzHm9nHUjpfAriIELKG6ByKlrTrePlUm/ZbmD8V2v4rYscPU+iRkC+XCyjhnbpcr wfFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728393656; x=1728998456; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=wnv8F+1ufi0FZk0Wia60FxxOpLB4CVsWaIwlw7XIvuU=; b=SiDacfKN7m2MqZB96XJpEbegjN5aDFCsVO8XtJXb38isPEtzaB3EudeesxKpWbCOaM O8gJdkF7JzvuLm+TZWakByZLyLRA0kU381uWKPTj2FfZyeCDGtZj8hH7R17NAN89vSAm oOSuY2S74W13VBJPTQ9AWjleG6BzkLhrLYt8OZWRtrH+JrA22YgZDv5nWJRoxz6VyVIe ihl99nwrrieCzfmGXnpiteRVwn/+8puSQ6nyI4nzk5LDIVkfrGYylLK5RKRigl3zdeB9 20Yl7ImtRI/QQCU/2de1HaGAdlVtPhf25fJX3z8byzN/YFIdW1s4MHgIMDc/7ICnUahv DBSw== X-Forwarded-Encrypted: i=1; AJvYcCVyvZhJKWrfriKIs57kea1cYSp2/KiHbDMu7+l+xM1y/wghaq5IaGUWNYsqMmoips9UcmTt1uCVpANmdiU=@vger.kernel.org X-Gm-Message-State: AOJu0YxXkW7eCu4mjW/nxYA6gueTGOCs25hGeiRjaWFhx/LYIchdKp0L ERppxiPPqdGYp0gd6S7p8gOguwG16YRfpbxiOB0ORzZn2TprVnPYfIj1RJha X-Google-Smtp-Source: AGHT+IEFBdzWQG6n6KACrk5XJjQMSBpMS3BHTDw+e5aWcH7zy71POBE7nZegWS51fhyNx74EvzKUng== X-Received: by 2002:a5d:5f46:0:b0:374:c6af:1658 with SMTP id ffacd0b85a97d-37d0e6eb8afmr16912841f8f.1.1728393655952; Tue, 08 Oct 2024 06:20:55 -0700 (PDT) Received: from localhost ([2a01:4b00:d036:ae00:a93f:a9ae:12b5:349a]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-37d169730fdsm8065052f8f.109.2024.10.08.06.20.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Oct 2024 06:20:55 -0700 (PDT) From: luca.boccassi@gmail.com To: linux-fsdevel@vger.kernel.org Cc: christian@brauner.io, linux-kernel@vger.kernel.org, oleg@redhat.com Subject: [PATCH v10] pidfd: add ioctl to retrieve pid info Date: Tue, 8 Oct 2024 14:19:58 +0100 Message-ID: <20241008132051.1011467-1-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.45.2 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Luca Boccassi 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 returning pid/tgid/ppid and creds unconditionally, and cgroupid optionally. Signed-off-by: Luca Boccassi --- v10: do not fail if we cannot get cgroupid, simply skip it (and document it) v9: drop result_mask and reuse request_mask instead v8: use RAII guard for rcu, call put_cred() v7: fix RCU issue and style issue introduced by v6 found by reviewer v6: use rcu_read_lock() when fetching cgroupid, use task_ppid_nr_ns() to get the ppid, return ESCHR if any of pid/tgid/ppid are 0 at the end of the call to avoid providing incomplete data, document what the callers should expect v5: check again that the task hasn't exited immediately before copying the result out to userspace, to ensure we are not returning stale data add an ifdef around the cgroup structs usage to fix build errors when the feature is disabled v4: fix arg check in pidfd_ioctl() by moving it after the new call v3: switch from pid_vnr() to task_pid_vnr() v2: Apply comments from Christian, apart from the one about pid namespaces as I need additional hints on how to implement it. Drop the security_context string as it is not the appropriate metadata to give userspace these days. fs/pidfs.c | 91 ++++++++++++++++++- include/uapi/linux/pidfd.h | 31 +++++++ .../testing/selftests/pidfd/pidfd_open_test.c | 78 +++++++++++++++- 3 files changed, 196 insertions(+), 4 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index 80675b6bf884..3b24124a4f41 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -114,6 +115,86 @@ static __poll_t pidfd_poll(struct file *file, struct p= oll_table_struct *pts) return poll_flags; } =20 +static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigne= d long arg) +{ + struct pidfd_info __user *uinfo =3D (struct pidfd_info __user *)arg; + size_t usize =3D _IOC_SIZE(cmd); + struct pidfd_info kinfo =3D {}; + struct user_namespace *user_ns; + const struct cred *c; + __u64 request_mask; + + if (!uinfo) + return -EINVAL; + if (usize < sizeof(struct pidfd_info)) + return -EINVAL; /* First version, no smaller struct possible */ + + if (copy_from_user(&request_mask, &uinfo->request_mask, sizeof(request_ma= sk))) + return -EFAULT; + + c =3D get_task_cred(task); + if (!c) + return -ESRCH; + + /* Unconditionally return identifiers and credentials, the rest only on r= equest */ + + user_ns =3D current_user_ns(); + kinfo.ruid =3D from_kuid_munged(user_ns, c->uid); + kinfo.rgid =3D from_kgid_munged(user_ns, c->gid); + kinfo.euid =3D from_kuid_munged(user_ns, c->euid); + kinfo.egid =3D from_kgid_munged(user_ns, c->egid); + kinfo.suid =3D from_kuid_munged(user_ns, c->suid); + kinfo.sgid =3D from_kgid_munged(user_ns, c->sgid); + kinfo.fsuid =3D from_kuid_munged(user_ns, c->fsuid); + kinfo.fsgid =3D from_kgid_munged(user_ns, c->fsgid); + put_cred(c); + +#ifdef CONFIG_CGROUPS + if (request_mask & PIDFD_INFO_CGROUPID) { + /* + * The cgroup id cannot be retrieved anymore after the task has exited + * (even if it has not been reaped yet), contrary to other fields. Set + * the flag only if we can still access it. */ + struct cgroup *cgrp; + + guard(rcu)(); + cgrp =3D task_cgroup(task, pids_cgrp_id); + if (cgrp) { + kinfo.cgroupid =3D cgroup_id(cgrp); + kinfo.request_mask |=3D PIDFD_INFO_CGROUPID; + } + } +#endif + + /* + * Copy pid/tgid last, to reduce the chances the information might be + * stale. Note that it is not possible to ensure it will be valid as the + * task might return as soon as the copy_to_user finishes, but that's ok + * and userspace expects that might happen and can act accordingly, so + * this is just best-effort. What we can do however is checking that all + * the fields are set correctly, or return ESRCH to avoid providing + * incomplete information. */ + + kinfo.ppid =3D task_ppid_nr_ns(task, NULL); + kinfo.tgid =3D task_tgid_vnr(task); + kinfo.pid =3D task_pid_vnr(task); + + if (kinfo.pid =3D=3D 0 || kinfo.tgid =3D=3D 0 || (kinfo.ppid =3D=3D 0 && = kinfo.pid !=3D 1)) + return -ESRCH; + + /* + * If userspace and the kernel have the same struct size it can just + * be copied. If userspace provides an older struct, only the bits that + * userspace knows about will be copied. If userspace provides a new + * struct, only the bits that the kernel knows about will be copied and + * the size value will be set to the size the kernel knows about. + */ + if (copy_to_user(uinfo, &kinfo, min(usize, sizeof(kinfo)))) + return -EFAULT; + + return 0; +} + static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long= arg) { struct task_struct *task __free(put_task) =3D NULL; @@ -122,13 +203,17 @@ static long pidfd_ioctl(struct file *file, unsigned i= nt cmd, unsigned long arg) struct ns_common *ns_common =3D NULL; struct pid_namespace *pid_ns; =20 - if (arg) - return -EINVAL; - task =3D get_pid_task(pid, PIDTYPE_PID); if (!task) return -ESRCH; =20 + /* Extensible IOCTL that does not open namespace FDs, take a shortcut */ + if (_IOC_NR(cmd) =3D=3D _IOC_NR(PIDFD_GET_INFO)) + return pidfd_info(task, cmd, arg); + + if (arg) + return -EINVAL; + scoped_guard(task_lock, task) { nsp =3D task->nsproxy; if (nsp) diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h index 565fc0629fff..5501cd808517 100644 --- a/include/uapi/linux/pidfd.h +++ b/include/uapi/linux/pidfd.h @@ -16,6 +16,36 @@ #define PIDFD_SIGNAL_THREAD_GROUP (1UL << 1) #define PIDFD_SIGNAL_PROCESS_GROUP (1UL << 2) =20 +/* Flags for pidfd_info. */ +#define PIDFD_INFO_CGROUPID (1UL << 0) + +struct pidfd_info { + /* Let userspace request expensive stuff explictly, and let the kernel + * indicate whether it knows about it, and/or whether it can still retrie= ve + * it (e.g.: the cgroup id is not available after the task has exited). */ + __u64 request_mask; + /* + * The information contained in the following fields might be stale at the + * time it is received, as the target process might have exited as soon as + * the IOCTL was processed, and there is no way to avoid that. However, it + * is guaranteed that if the call was successful, then the information was + * correct and referred to the intended process at the time the work was + * performed. */ + __u64 cgroupid; + __u32 pid; + __u32 tgid; + __u32 ppid; + __u32 ruid; + __u32 rgid; + __u32 euid; + __u32 egid; + __u32 suid; + __u32 sgid; + __u32 fsuid; + __u32 fsgid; + __u32 spare0[1]; +}; + #define PIDFS_IOCTL_MAGIC 0xFF =20 #define PIDFD_GET_CGROUP_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 1) @@ -28,5 +58,6 @@ #define PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 8) #define PIDFD_GET_USER_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 9) #define PIDFD_GET_UTS_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 10) +#define PIDFD_GET_INFO _IOWR(PIDFS_IOCTL_MAGIC, 11,= struct pidfd_info) =20 #endif /* _UAPI_LINUX_PIDFD_H */ diff --git a/tools/testing/selftests/pidfd/pidfd_open_test.c b/tools/testin= g/selftests/pidfd/pidfd_open_test.c index c62564c264b1..036e517246a3 100644 --- a/tools/testing/selftests/pidfd/pidfd_open_test.c +++ b/tools/testing/selftests/pidfd/pidfd_open_test.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -21,6 +22,32 @@ #include "pidfd.h" #include "../kselftest.h" =20 +#ifndef PIDFS_IOCTL_MAGIC +#define PIDFS_IOCTL_MAGIC 0xFF +#endif + +#ifndef PIDFD_GET_INFO +#define PIDFD_GET_INFO _IOWR(PIDFS_IOCTL_MAGIC, 11, struct pidfd_info) +#define PIDFD_INFO_CGROUPID (1UL << 0) + +struct pidfd_info { + __u64 request_mask; + __u64 cgroupid; + __u32 pid; + __u32 tgid; + __u32 ppid; + __u32 ruid; + __u32 rgid; + __u32 euid; + __u32 egid; + __u32 suid; + __u32 sgid; + __u32 fsuid; + __u32 fsgid; + __u32 spare0[1]; +}; +#endif + static int safe_int(const char *numstr, int *converted) { char *err =3D NULL; @@ -120,10 +147,13 @@ static pid_t get_pid_from_fdinfo_file(int pidfd, cons= t char *key, size_t keylen) =20 int main(int argc, char **argv) { + struct pidfd_info info =3D { + .request_mask =3D PIDFD_INFO_CGROUPID, + }; int pidfd =3D -1, ret =3D 1; pid_t pid; =20 - ksft_set_plan(3); + ksft_set_plan(4); =20 pidfd =3D sys_pidfd_open(-1, 0); if (pidfd >=3D 0) { @@ -153,6 +183,52 @@ int main(int argc, char **argv) pid =3D get_pid_from_fdinfo_file(pidfd, "Pid:", sizeof("Pid:") - 1); ksft_print_msg("pidfd %d refers to process with pid %d\n", pidfd, pid); =20 + if (ioctl(pidfd, PIDFD_GET_INFO, &info) < 0) { + ksft_print_msg("%s - failed to get info from pidfd\n", strerror(errno)); + goto on_error; + } + if (info.pid !=3D pid) { + ksft_print_msg("pid from fdinfo file %d does not match pid from ioctl %d= \n", + pid, info.pid); + goto on_error; + } + if (info.ppid !=3D getppid()) { + ksft_print_msg("ppid %d does not match ppid from ioctl %d\n", + pid, info.pid); + goto on_error; + } + if (info.ruid !=3D getuid()) { + ksft_print_msg("uid %d does not match uid from ioctl %d\n", + getuid(), info.ruid); + goto on_error; + } + if (info.rgid !=3D getgid()) { + ksft_print_msg("gid %d does not match gid from ioctl %d\n", + getgid(), info.rgid); + goto on_error; + } + if (info.euid !=3D geteuid()) { + ksft_print_msg("euid %d does not match euid from ioctl %d\n", + geteuid(), info.euid); + goto on_error; + } + if (info.egid !=3D getegid()) { + ksft_print_msg("egid %d does not match egid from ioctl %d\n", + getegid(), info.egid); + goto on_error; + } + if (info.suid !=3D geteuid()) { + ksft_print_msg("suid %d does not match suid from ioctl %d\n", + geteuid(), info.suid); + goto on_error; + } + if (info.sgid !=3D getegid()) { + ksft_print_msg("sgid %d does not match sgid from ioctl %d\n", + getegid(), info.sgid); + goto on_error; + } + ksft_test_result_pass("get info from pidfd test: passed\n"); + ret =3D 0; =20 on_error: base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b --=20 2.45.2