* Re: [PATCH RFC v2 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl
From: Christian Brauner @ 2025-07-31 10:31 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Alexander Viro, Jan Kara, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest
In-Reply-To: <2025-07-25.1753409614-vile-track-icky-epidemic-frail-antidote-d7NYuu@cyphar.com>
On Fri, Jul 25, 2025 at 12:24:28PM +1000, Aleksa Sarai wrote:
> On 2025-07-24, Christian Brauner <brauner@kernel.org> wrote:
> > On Wed, Jul 23, 2025 at 09:18:53AM +1000, Aleksa Sarai wrote:
> > > /proc has historically had very opaque semantics about PID namespaces,
> > > which is a little unfortunate for container runtimes and other programs
> > > that deal with switching namespaces very often. One common issue is that
> > > of converting between PIDs in the process's namespace and PIDs in the
> > > namespace of /proc.
> > >
> > > In principle, it is possible to do this today by opening a pidfd with
> > > pidfd_open(2) and then looking at /proc/self/fdinfo/$n (which will
> > > contain a PID value translated to the pid namespace associated with that
> > > procfs superblock). However, allocating a new file for each PID to be
> > > converted is less than ideal for programs that may need to scan procfs,
> > > and it is generally useful for userspace to be able to finally get this
> > > information from procfs.
> > >
> > > So, add a new API for this in the form of an ioctl(2) you can call on
> > > the root directory of procfs. The returned file descriptor will have
> > > O_CLOEXEC set. This acts as a sister feature to the new "pidns" mount
> > > option, finally allowing userspace full control of the pid namespaces
> > > associated with procfs instances.
> > >
> > > The permission model for this is a bit looser than that of the "pidns"
> > > mount option, but this is mainly because /proc/1/ns/pid provides the
> > > same information, so as long as you have access to that magic-link (or
> > > something equivalently reasonable such as privileges with CAP_SYS_ADMIN
> > > or being in an ancestor pid namespace) it makes sense to allow userspace
> > > to grab a handle. setns(2) will still have their own permission checks,
> > > so being able to open a pidns handle doesn't really provide too many
> > > other capabilities.
> > >
> > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > > ---
> > > Documentation/filesystems/proc.rst | 4 +++
> > > fs/proc/root.c | 54 ++++++++++++++++++++++++++++++++++++--
> > > include/uapi/linux/fs.h | 3 +++
> > > 3 files changed, 59 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > index c520b9f8a3fd..506383273c9d 100644
> > > --- a/Documentation/filesystems/proc.rst
> > > +++ b/Documentation/filesystems/proc.rst
> > > @@ -2398,6 +2398,10 @@ pidns= specifies a pid namespace (either as a string path to something like
> > > will be used by the procfs instance when translating pids. By default, procfs
> > > will use the calling process's active pid namespace.
> > >
> > > +Processes can check which pid namespace is used by a procfs instance by using
> > > +the `PROCFS_GET_PID_NAMESPACE` ioctl() on the root directory of the procfs
> > > +instance.
> > > +
> > > Chapter 5: Filesystem behavior
> > > ==============================
> > >
> > > diff --git a/fs/proc/root.c b/fs/proc/root.c
> > > index 057c8a125c6e..548a57ec2152 100644
> > > --- a/fs/proc/root.c
> > > +++ b/fs/proc/root.c
> > > @@ -23,8 +23,10 @@
> > > #include <linux/cred.h>
> > > #include <linux/magic.h>
> > > #include <linux/slab.h>
> > > +#include <linux/ptrace.h>
> > >
> > > #include "internal.h"
> > > +#include "../internal.h"
> > >
> > > struct proc_fs_context {
> > > struct pid_namespace *pid_ns;
> > > @@ -418,15 +420,63 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
> > > return proc_pid_readdir(file, ctx);
> > > }
> > >
> > > +static long int proc_root_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> > > +{
> > > + switch (cmd) {
> > > +#ifdef CONFIG_PID_NS
> > > + case PROCFS_GET_PID_NAMESPACE: {
> > > + struct pid_namespace *active = task_active_pid_ns(current);
> > > + struct pid_namespace *ns = proc_pid_ns(file_inode(filp)->i_sb);
> > > + bool can_access_pidns = false;
> > > +
> > > + /*
> > > + * If we are in an ancestors of the pidns, or have join
> > > + * privileges (CAP_SYS_ADMIN), then it makes sense that we
> > > + * would be able to grab a handle to the pidns.
> > > + *
> > > + * Otherwise, if there is a root process, then being able to
> > > + * access /proc/$pid/ns/pid is equivalent to this ioctl and so
> > > + * we should probably match the permission model. For empty
> > > + * namespaces it seems unlikely for there to be a downside to
> > > + * allowing unprivileged users to open a handle to it (setns
> > > + * will fail for unprivileged users anyway).
> > > + */
> > > + can_access_pidns = pidns_is_ancestor(ns, active) ||
> > > + ns_capable(ns->user_ns, CAP_SYS_ADMIN);
> >
> > This seems to imply that if @ns is a descendant of @active that the
> > caller holds privileges over it. Is that actually always true?
> >
> > IOW, why is the check different from the previous pidns= mount option
> > check. I would've expected:
> >
> > ns_capable(_no_audit)(ns->user_ns) && pidns_is_ancestor(ns, active)
> >
> > and then the ptrace check as a fallback.
>
> That would mirror pidns_install(), and I did think about it. The primary
> (mostly handwave-y) reasoning I had for making it less strict was that:
>
> * If you are in an ancestor pidns, then you can already see those
> processes in your own /proc. In theory that means that you will be
> able to access /proc/$pid/ns/pid for at least some subprocess there
> (even if some subprocesses have SUID_DUMP_DISABLE, that flag is
> cleared on ).
>
> Though hypothetically if they are all running as a different user,
> this does not apply (and you could create scenarios where a child
> pidns is owned by a userns that you do not have privileges over -- if
> you deal with setuid binaries). Maybe that risk means we should just
> combine them, I'm not sure.
>
> * If you have CAP_SYS_ADMIN permissions over the pidns, it seems
> strange to disallow access even if it is not in an ancestor
> namespace. This is distinct to pidns_install(), where you want to
> ensure you cannot escape to a parent pid namespace, this is about
> getting a handle to do other operations (i.e. NS_GET_{P,TG}ID_*_PIDNS).
>
> Maybe they should be combined to match pidns_install(), but then I would
> expect the ptrace_may_access() check to apply to all processes in the
> pidns to make it less restrictive, which is not something you can
> practically do (and there is a higher chance that pid1 will have
> SUID_DUMP_DISABLE than some random subprocess, which almost certainly
> will not be SUID_DUMP_DISABLE).
>
> Fundamentally, I guess I'm still trying to see what the risk is of
> allowing a process to get a handle to a pidns that they have some kind
> of privilege over (whether it's CAP_SYS_ADMIN, or by the virtue of being
There shouldn't be. For example, you kinda implicitly do that with a
pidfd, no? Because you can pass the pidfd to setns() instead of a
namespace fd itself. Maybe that's the argument you're lookin for?
^ permalink raw reply
* Re: [PATCH RFC v2 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl
From: Aleksa Sarai @ 2025-07-31 14:21 UTC (permalink / raw)
To: Christian Brauner
Cc: Alexander Viro, Jan Kara, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest
In-Reply-To: <20250731-angliederung-mahlt-9e5811969817@brauner>
[-- Attachment #1: Type: text/plain, Size: 7820 bytes --]
On 2025-07-31, Christian Brauner <brauner@kernel.org> wrote:
> On Fri, Jul 25, 2025 at 12:24:28PM +1000, Aleksa Sarai wrote:
> > On 2025-07-24, Christian Brauner <brauner@kernel.org> wrote:
> > > On Wed, Jul 23, 2025 at 09:18:53AM +1000, Aleksa Sarai wrote:
> > > > /proc has historically had very opaque semantics about PID namespaces,
> > > > which is a little unfortunate for container runtimes and other programs
> > > > that deal with switching namespaces very often. One common issue is that
> > > > of converting between PIDs in the process's namespace and PIDs in the
> > > > namespace of /proc.
> > > >
> > > > In principle, it is possible to do this today by opening a pidfd with
> > > > pidfd_open(2) and then looking at /proc/self/fdinfo/$n (which will
> > > > contain a PID value translated to the pid namespace associated with that
> > > > procfs superblock). However, allocating a new file for each PID to be
> > > > converted is less than ideal for programs that may need to scan procfs,
> > > > and it is generally useful for userspace to be able to finally get this
> > > > information from procfs.
> > > >
> > > > So, add a new API for this in the form of an ioctl(2) you can call on
> > > > the root directory of procfs. The returned file descriptor will have
> > > > O_CLOEXEC set. This acts as a sister feature to the new "pidns" mount
> > > > option, finally allowing userspace full control of the pid namespaces
> > > > associated with procfs instances.
> > > >
> > > > The permission model for this is a bit looser than that of the "pidns"
> > > > mount option, but this is mainly because /proc/1/ns/pid provides the
> > > > same information, so as long as you have access to that magic-link (or
> > > > something equivalently reasonable such as privileges with CAP_SYS_ADMIN
> > > > or being in an ancestor pid namespace) it makes sense to allow userspace
> > > > to grab a handle. setns(2) will still have their own permission checks,
> > > > so being able to open a pidns handle doesn't really provide too many
> > > > other capabilities.
> > > >
> > > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > > > ---
> > > > Documentation/filesystems/proc.rst | 4 +++
> > > > fs/proc/root.c | 54 ++++++++++++++++++++++++++++++++++++--
> > > > include/uapi/linux/fs.h | 3 +++
> > > > 3 files changed, 59 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > > index c520b9f8a3fd..506383273c9d 100644
> > > > --- a/Documentation/filesystems/proc.rst
> > > > +++ b/Documentation/filesystems/proc.rst
> > > > @@ -2398,6 +2398,10 @@ pidns= specifies a pid namespace (either as a string path to something like
> > > > will be used by the procfs instance when translating pids. By default, procfs
> > > > will use the calling process's active pid namespace.
> > > >
> > > > +Processes can check which pid namespace is used by a procfs instance by using
> > > > +the `PROCFS_GET_PID_NAMESPACE` ioctl() on the root directory of the procfs
> > > > +instance.
> > > > +
> > > > Chapter 5: Filesystem behavior
> > > > ==============================
> > > >
> > > > diff --git a/fs/proc/root.c b/fs/proc/root.c
> > > > index 057c8a125c6e..548a57ec2152 100644
> > > > --- a/fs/proc/root.c
> > > > +++ b/fs/proc/root.c
> > > > @@ -23,8 +23,10 @@
> > > > #include <linux/cred.h>
> > > > #include <linux/magic.h>
> > > > #include <linux/slab.h>
> > > > +#include <linux/ptrace.h>
> > > >
> > > > #include "internal.h"
> > > > +#include "../internal.h"
> > > >
> > > > struct proc_fs_context {
> > > > struct pid_namespace *pid_ns;
> > > > @@ -418,15 +420,63 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
> > > > return proc_pid_readdir(file, ctx);
> > > > }
> > > >
> > > > +static long int proc_root_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> > > > +{
> > > > + switch (cmd) {
> > > > +#ifdef CONFIG_PID_NS
> > > > + case PROCFS_GET_PID_NAMESPACE: {
> > > > + struct pid_namespace *active = task_active_pid_ns(current);
> > > > + struct pid_namespace *ns = proc_pid_ns(file_inode(filp)->i_sb);
> > > > + bool can_access_pidns = false;
> > > > +
> > > > + /*
> > > > + * If we are in an ancestors of the pidns, or have join
> > > > + * privileges (CAP_SYS_ADMIN), then it makes sense that we
> > > > + * would be able to grab a handle to the pidns.
> > > > + *
> > > > + * Otherwise, if there is a root process, then being able to
> > > > + * access /proc/$pid/ns/pid is equivalent to this ioctl and so
> > > > + * we should probably match the permission model. For empty
> > > > + * namespaces it seems unlikely for there to be a downside to
> > > > + * allowing unprivileged users to open a handle to it (setns
> > > > + * will fail for unprivileged users anyway).
> > > > + */
> > > > + can_access_pidns = pidns_is_ancestor(ns, active) ||
> > > > + ns_capable(ns->user_ns, CAP_SYS_ADMIN);
> > >
> > > This seems to imply that if @ns is a descendant of @active that the
> > > caller holds privileges over it. Is that actually always true?
> > >
> > > IOW, why is the check different from the previous pidns= mount option
> > > check. I would've expected:
> > >
> > > ns_capable(_no_audit)(ns->user_ns) && pidns_is_ancestor(ns, active)
> > >
> > > and then the ptrace check as a fallback.
> >
> > That would mirror pidns_install(), and I did think about it. The primary
> > (mostly handwave-y) reasoning I had for making it less strict was that:
> >
> > * If you are in an ancestor pidns, then you can already see those
> > processes in your own /proc. In theory that means that you will be
> > able to access /proc/$pid/ns/pid for at least some subprocess there
> > (even if some subprocesses have SUID_DUMP_DISABLE, that flag is
> > cleared on ).
> >
> > Though hypothetically if they are all running as a different user,
> > this does not apply (and you could create scenarios where a child
> > pidns is owned by a userns that you do not have privileges over -- if
> > you deal with setuid binaries). Maybe that risk means we should just
> > combine them, I'm not sure.
> >
> > * If you have CAP_SYS_ADMIN permissions over the pidns, it seems
> > strange to disallow access even if it is not in an ancestor
> > namespace. This is distinct to pidns_install(), where you want to
> > ensure you cannot escape to a parent pid namespace, this is about
> > getting a handle to do other operations (i.e. NS_GET_{P,TG}ID_*_PIDNS).
> >
> > Maybe they should be combined to match pidns_install(), but then I would
> > expect the ptrace_may_access() check to apply to all processes in the
> > pidns to make it less restrictive, which is not something you can
> > practically do (and there is a higher chance that pid1 will have
> > SUID_DUMP_DISABLE than some random subprocess, which almost certainly
> > will not be SUID_DUMP_DISABLE).
> >
> > Fundamentally, I guess I'm still trying to see what the risk is of
> > allowing a process to get a handle to a pidns that they have some kind
> > of privilege over (whether it's CAP_SYS_ADMIN, or by the virtue of being
>
> There shouldn't be. For example, you kinda implicitly do that with a
> pidfd, no? Because you can pass the pidfd to setns() instead of a
> namespace fd itself. Maybe that's the argument you're lookin for?
That argument works for me! I'll rewrite the commit message to make sure
it sounds like I came up with it. ;)
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] sched/deadline: sched_getattr(...flags=1) returns the runtime left and abs deadline for DEADLINE tasks
From: Juri Lelli @ 2025-08-01 12:53 UTC (permalink / raw)
To: Tommaso Cucinotta
Cc: Ingo Molnar, Peter Zijlstra, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
linux-kernel, linux-api, Tommaso Cucinotta
In-Reply-To: <20250715164148.1151620-1-tommaso.cucinotta@santannapisa.it>
Hi Tommaso,
On 15/07/25 18:39, Tommaso Cucinotta wrote:
> The SCHED_DEADLINE scheduler allows reading the statically configured
> run-time, deadline, and period parameters through the sched_getattr()
> system call. However, there is no immediate way to access, from user space,
> the current parameters used within the scheduler: the instantaneous runtime
> left in the current cycle, as well as the current absolute deadline.
>
> The `flags' sched_getattr() parameter, so far mandated to contain zero,
> now supports the SCHED_GETATTR_FLAG_DL_DYNAMIC=1 flag, to request
> retrieval of the leftover runtime and absolute deadline, converted to a
> CLOCK_MONOTONIC reference, instead of the statically configured parameters.
>
> This feature is useful for adaptive SCHED_DEADLINE tasks that need to
> modify their behavior depending on whether or not there is enough runtime
> left in the current period, and/or what is the current absolute deadline.
>
> Notes:
> - before returning the instantaneous parameters, the runtime is updated;
> - the abs deadline is returned shifted from rq_clock() to ktime_get_ns(),
> in CLOCK_MONOTONIC reference; this causes multiple invocations from the
> same period to return values that may differ for a few ns (showing some
> small drift), albeit the deadline doesn't move, in rq_clock() reference;
> - the abs deadline value returned to user-space, as unsigned 64-bit value,
> can represent nearly 585 years since boot time;
> - setting flags=0 provides the old behavior (retrieve static parameters).
As we discussed this offline before your submission you know that I was
already on board with the idea, so I would really like to hear what
Peter and others think about this.
Still a few comments below.
$SUBJECT can maybe simply be "sched/deadline: Add reporting of remaining
time/abs deadline to sched_getattr".
> See also the notes from discussion held at OSPM 2025 on the topic
> "Making user space aware of current deadline-scheduler parameters":
> https://lwn.net/Articles/1022054/
I would probably remove the link from the changelog as it might
disappear/change in the future.
> Signed-off-by: Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>
> ---
> include/uapi/linux/sched.h | 3 +++
> kernel/sched/deadline.c | 18 +++++++++++++++---
> kernel/sched/sched.h | 2 +-
> kernel/sched/syscalls.c | 16 +++++++++++-----
> 4 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
> index 359a14cc..52b69ce8 100644
> --- a/include/uapi/linux/sched.h
> +++ b/include/uapi/linux/sched.h
> @@ -146,4 +146,7 @@ struct clone_args {
> SCHED_FLAG_KEEP_ALL | \
> SCHED_FLAG_UTIL_CLAMP)
>
> +/* Only for sched_getattr() own flag param, if task is SCHED_DEADLINE */
> +#define SCHED_GETATTR_FLAG_DL_DYNAMIC 0x01
> +
> #endif /* _UAPI_LINUX_SCHED_H */
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 9c7d9528..1b5cd7fa 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -3290,13 +3290,25 @@ void __setparam_dl(struct task_struct *p, const struct sched_attr *attr)
> dl_se->dl_density = to_ratio(dl_se->dl_deadline, dl_se->dl_runtime);
> }
>
> -void __getparam_dl(struct task_struct *p, struct sched_attr *attr)
> +void __getparam_dl(struct task_struct *p, struct sched_attr *attr, unsigned int flags)
> {
> struct sched_dl_entity *dl_se = &p->dl;
> + struct rq *rq = task_rq(p);
> + u64 adj_deadline;
>
> attr->sched_priority = p->rt_priority;
> - attr->sched_runtime = dl_se->dl_runtime;
> - attr->sched_deadline = dl_se->dl_deadline;
> + if (flags & SCHED_GETATTR_FLAG_DL_DYNAMIC) {
> + guard(raw_spinlock_irq)(&rq->__lock);
> + update_rq_clock(rq);
> + update_curr_dl(rq);
I p is not current maybe we don't need to call update_curr_dl() as p
will still have sensible dynamic parameters updated last time it
blocked?
Also, even though this is superuser stuff and all, mildly fear this
could be used as a DOS attack vector? Do we want to be super defensive
and add some kind of rate limiting?
> +
> + attr->sched_runtime = dl_se->runtime;
> + adj_deadline = dl_se->deadline - rq_clock(rq) + ktime_get_ns();
> + attr->sched_deadline = adj_deadline;
> + } else {
> + attr->sched_runtime = dl_se->dl_runtime;
> + attr->sched_deadline = dl_se->dl_deadline;
> + }
> attr->sched_period = dl_se->dl_period;
> attr->sched_flags &= ~SCHED_DL_FLAGS;
> attr->sched_flags |= dl_se->flags;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 3058fb62..f69bf019 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -353,7 +353,7 @@ extern int sched_dl_global_validate(void);
> extern void sched_dl_do_global(void);
> extern int sched_dl_overflow(struct task_struct *p, int policy, const struct sched_attr *attr);
> extern void __setparam_dl(struct task_struct *p, const struct sched_attr *attr);
> -extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr);
> +extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr, unsigned int flags);
> extern bool __checkparam_dl(const struct sched_attr *attr);
> extern bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr);
> extern int dl_cpuset_cpumask_can_shrink(const struct cpumask *cur, const struct cpumask *trial);
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index 77ae87f3..c80b3568 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -928,10 +928,10 @@ static int sched_copy_attr(struct sched_attr __user *uattr, struct sched_attr *a
> return -E2BIG;
> }
>
> -static void get_params(struct task_struct *p, struct sched_attr *attr)
> +static void get_params(struct task_struct *p, struct sched_attr *attr, unsigned int flags)
> {
> if (task_has_dl_policy(p)) {
> - __getparam_dl(p, attr);
> + __getparam_dl(p, attr, flags);
> } else if (task_has_rt_policy(p)) {
> attr->sched_priority = p->rt_priority;
> } else {
> @@ -997,7 +997,7 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,
> return -ESRCH;
>
> if (attr.sched_flags & SCHED_FLAG_KEEP_PARAMS)
> - get_params(p, &attr);
> + get_params(p, &attr, 0);
>
> return sched_setattr(p, &attr);
> }
> @@ -1082,7 +1082,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> int retval;
>
> if (unlikely(!uattr || pid < 0 || usize > PAGE_SIZE ||
> - usize < SCHED_ATTR_SIZE_VER0 || flags))
> + usize < SCHED_ATTR_SIZE_VER0))
> return -EINVAL;
>
> scoped_guard (rcu) {
> @@ -1090,6 +1090,12 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> if (!p)
> return -ESRCH;
>
> + if (flags) {
> + if (!task_has_dl_policy(p)
> + || flags != SCHED_GETATTR_FLAG_DL_DYNAMIC)
Nit pick. Formatting usually has line break after '||'.
> + return -EINVAL;
> + }
> +
Thanks!
Juri
^ permalink raw reply
* Re: [RFC v3 1/4] kernel/api: introduce kernel API specification framework
From: Sasha Levin @ 2025-08-01 13:53 UTC (permalink / raw)
To: Askar Safin; +Cc: linux-api, linux-doc, linux-kernel, tools
In-Reply-To: <20250716072141.12-1-safinaskar@zohomail.com>
On Wed, Jul 16, 2025 at 10:21:41AM +0300, Askar Safin wrote:
>> + KAPI_PARAM_IN = (1 << 0),
>> + KAPI_PARAM_OUT = (1 << 1),
>> + KAPI_PARAM_INOUT = (1 << 2),
>
>There is no need for KAPI_PARAM_INOUT. It could be replaced by KAPI_PARAM_IN | KAPI_PARAM_OUT
It could, but it's easier to write _INOUT :)
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v3 1/2] man/man2/mremap.2: describe multiple mapping move
From: Alejandro Colomar @ 2025-08-02 16:14 UTC (permalink / raw)
To: Lorenzo Stoakes, Jan Kara
Cc: linux-man, Andrew Morton, Peter Xu, Alexander Viro,
Christian Brauner, Liam R . Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, Rik van Riel, linux-mm, linux-kernel, linux-api
In-Reply-To: <1fd0223a6f903ffdd8ba644d0b67820b1921671f.1753795807.git.lorenzo.stoakes@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 5053 bytes --]
Hi Lorenzo, Jann,
On Tue, Jul 29, 2025 at 02:47:35PM +0100, Lorenzo Stoakes wrote:
> Document the new behaviour introduced in Linux 6.17 whereby it is now
> possible to move multiple mappings in a single operation, as long as the
> operation is simply an MREMAP_FIXED move - that is old_size is equal to
> new_size and MREMAP_FIXED is specified.
>
> To make things clearer, also describe this kind of move operation, before
> expanding upon it to describe the newly introduced behaviour.
>
> This change also explains the limitations of of this method and the
> possibility of partial failure.
>
> Finally, we pluralise language where it makes sense to do so such that the
> documentation does not contradict either this new capability nor the
> pre-existing edge case.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
I see some very minor formatting or punctuation issues, but I'll fix
them while applying. Jann, do you plan to review this?
Have a lovely day!
Alex
> ---
> man/man2/mremap.2 | 84 ++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 73 insertions(+), 11 deletions(-)
>
> diff --git a/man/man2/mremap.2 b/man/man2/mremap.2
> index 2168ca728..6ba51310c 100644
> --- a/man/man2/mremap.2
> +++ b/man/man2/mremap.2
> @@ -25,18 +25,47 @@ moving it at the same time (controlled by the
> argument and
> the available virtual address space).
> .P
> +Mappings can also simply be moved
> +(without any resizing)
> +by specifying equal
> +.I old_size
> +and
> +.I new_size
> +and using the
> +.B MREMAP_FIXED
> +flag
> +(see below).
> +Since Linux 6.17,
> +while
> +.I old_address
> +must reside within a mapping,
> +.I old_size
> +may span multiple mappings
> +which do not have to be
> +adjacent to one another when
> +performing a move like this.
> +The
> +.B MREMAP_DONTUNMAP
> +flag may be specified.
> +.P
> +If the operation is not
> +simply moving mappings,
> +then
> +.I old_size
> +must span only a single mapping.
> +.P
> .I old_address
> -is the old address of the virtual memory block that you
> -want to expand (or shrink).
> +is the old address of the first virtual memory block that you
> +want to expand, shrink, and/or move.
> Note that
> .I old_address
> has to be page aligned.
> .I old_size
> -is the old size of the
> -virtual memory block.
> +is the size of the range containing
> +virtual memory blocks to be manipulated.
> .I new_size
> is the requested size of the
> -virtual memory block after the resize.
> +virtual memory blocks after the resize.
> An optional fifth argument,
> .IR new_address ,
> may be provided; see the description of
> @@ -105,13 +134,43 @@ If
> is specified, then
> .B MREMAP_MAYMOVE
> must also be specified.
> +.IP
> +Since Linux 6.17,
> +if
> +.I old_size
> +is equal to
> +.I new_size
> +and
> +.B MREMAP_FIXED
> +is specified, then
> +.I old_size
> +may span beyond the mapping in which
> +.I old_address
> +resides.
> +In this case,
> +gaps between mappings in the original range
> +are maintained in the new range.
> +The whole operation is performed atomically
> +unless an error arises,
> +in which case the operation may be partially
> +completed,
> +that is,
> +some mappings may be moved and others not.
> +.IP
> +
> +Moving multiple mappings is not permitted if
> +any of those mappings have either
> +been registered with
> +.BR userfaultfd (2) ,
> +or map drivers that
> +specify their own custom address mapping logic.
> .TP
> .BR MREMAP_DONTUNMAP " (since Linux 5.7)"
> .\" commit e346b3813067d4b17383f975f197a9aa28a3b077
> This flag, which must be used in conjunction with
> .BR MREMAP_MAYMOVE ,
> -remaps a mapping to a new address but does not unmap the mapping at
> -.IR old_address .
> +remaps mappings to a new address but does not unmap them
> +from their original address.
> .IP
> The
> .B MREMAP_DONTUNMAP
> @@ -149,13 +208,13 @@ mapped.
> See NOTES for some possible applications of
> .BR MREMAP_DONTUNMAP .
> .P
> -If the memory segment specified by
> +If the memory segments specified by
> .I old_address
> and
> .I old_size
> -is locked (using
> +are locked (using
> .BR mlock (2)
> -or similar), then this lock is maintained when the segment is
> +or similar), then this lock is maintained when the segments are
> resized and/or relocated.
> As a consequence, the amount of memory locked by the process may change.
> .SH RETURN VALUE
> @@ -188,7 +247,10 @@ virtual memory address for this process.
> You can also get
> .B EFAULT
> even if there exist mappings that cover the
> -whole address space requested, but those mappings are of different types.
> +whole address space requested, but those mappings are of different types,
> +and the
> +.BR mremap ()
> +operation being performed does not support this.
> .TP
> .B EINVAL
> An invalid argument was given.
> --
> 2.50.1
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 01/32] kho: init new_physxa->phys_bits to fix lockdep
From: Pasha Tatashin @ 2025-08-02 23:33 UTC (permalink / raw)
To: Mike Rapoport
Cc: pratyush, jasonmiu, graf, changyuanl, dmatlack, rientjes, corbet,
rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
david, joel.granados, rostedt, anna.schumaker, song, zhangguopeng,
linux, linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
jgg, parav, leonro, witu
In-Reply-To: <aIdNTN1qd0dTvsQm@kernel.org>
> Just int err should be fine here, otherwise
Done
> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Thanks.
Pasha
^ permalink raw reply
* Re: [PATCH v2 04/32] kho: allow to drive kho from within kernel
From: Pasha Tatashin @ 2025-08-02 23:40 UTC (permalink / raw)
To: Mike Rapoport
Cc: pratyush, jasonmiu, graf, changyuanl, dmatlack, rientjes, corbet,
rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
david, joel.granados, rostedt, anna.schumaker, song, zhangguopeng,
linux, linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
jgg, parav, leonro, witu
In-Reply-To: <aIdOcmTl-zrxXKAB@kernel.org>
Hi Mike,
Thank you for your review comments.
> > + mutex_unlock(&kho_out.lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(kho_abort);
>
> I don't think a module should be able to drive KHO. Please drop
> EXPORT_SYMBOL_GPL here and for kho_finalize().
Agreed, removed these exports.
Pasha
^ permalink raw reply
* Re: [PATCH v2 09/32] liveupdate: kho: move to kernel/liveupdate
From: Pasha Tatashin @ 2025-08-02 23:46 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: pratyush, jasonmiu, graf, changyuanl, rppt, dmatlack, rientjes,
corbet, rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl,
masahiroy, akpm, tj, yoann.congal, mmaurer, roman.gushchin,
chenridong, axboe, mark.rutland, jannh, vincent.guittot, hannes,
dan.j.williams, david, joel.granados, rostedt, anna.schumaker,
song, zhangguopeng, linux, linux-kernel, linux-doc, linux-mm,
gregkh, tglx, mingo, bp, dave.hansen, x86, hpa, rafael, dakr,
bartosz.golaszewski, cw00.choi, myungjoo.ham, yesanishhere,
Jonathan.Cameron, quic_zijuhu, aleksander.lobakin, ira.weiny,
andriy.shevchenko, leon, lukas, bhelgaas, wagi, djeffery,
stuart.w.hayes, ptyadav, lennart, brauner, linux-api,
linux-fsdevel, saeedm, ajayachandra, parav, leonro, witu
In-Reply-To: <20250729171454.GO36037@nvidia.com>
On Tue, Jul 29, 2025 at 1:14 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Jul 23, 2025 at 02:46:22PM +0000, Pasha Tatashin wrote:
> > Move KHO to kernel/liveupdate/ in preparation of placing all Live Update
> > core kernel related files to the same place.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
> > Documentation/core-api/kho/concepts.rst | 2 +-
> > MAINTAINERS | 2 +-
> > init/Kconfig | 2 ++
> > kernel/Kconfig.kexec | 25 ----------------
> > kernel/Makefile | 3 +-
> > kernel/liveupdate/Kconfig | 30 +++++++++++++++++++
> > kernel/liveupdate/Makefile | 7 +++++
> > kernel/{ => liveupdate}/kexec_handover.c | 6 ++--
> > .../{ => liveupdate}/kexec_handover_debug.c | 0
> > .../kexec_handover_internal.h | 0
> > 10 files changed, 45 insertions(+), 32 deletions(-)
> > create mode 100644 kernel/liveupdate/Kconfig
> > create mode 100644 kernel/liveupdate/Makefile
> > rename kernel/{ => liveupdate}/kexec_handover.c (99%)
> > rename kernel/{ => liveupdate}/kexec_handover_debug.c (100%)
> > rename kernel/{ => liveupdate}/kexec_handover_internal.h (100%)
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Thank you,
Pasha
^ permalink raw reply
* Re: [PATCH v3 1/2] man/man2/mremap.2: describe multiple mapping move
From: Alejandro Colomar @ 2025-08-03 6:47 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-man, Andrew Morton, Peter Xu, Alexander Viro,
Christian Brauner, Jan Kara, Liam R . Howlett, Vlastimil Babka,
Jann Horn, Pedro Falcato, Rik van Riel, linux-mm, linux-kernel,
linux-api
In-Reply-To: <1fd0223a6f903ffdd8ba644d0b67820b1921671f.1753795807.git.lorenzo.stoakes@oracle.com>
Hi Lorenzo,
On Tue, Jul 29, 2025 at 02:47:35PM +0100, Lorenzo Stoakes wrote:
> Document the new behaviour introduced in Linux 6.17 whereby it is now
> possible to move multiple mappings in a single operation, as long as the
> operation is simply an MREMAP_FIXED move - that is old_size is equal to
> new_size and MREMAP_FIXED is specified.
>
> To make things clearer, also describe this kind of move operation, before
> expanding upon it to describe the newly introduced behaviour.
>
> This change also explains the limitations of of this method and the
> possibility of partial failure.
>
> Finally, we pluralise language where it makes sense to do so such that the
> documentation does not contradict either this new capability nor the
> pre-existing edge case.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Would it be possible to write a small C program that uses this new
feature?
Cheers,
Alex
> ---
> man/man2/mremap.2 | 84 ++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 73 insertions(+), 11 deletions(-)
>
> diff --git a/man/man2/mremap.2 b/man/man2/mremap.2
> index 2168ca728..6ba51310c 100644
> --- a/man/man2/mremap.2
> +++ b/man/man2/mremap.2
> @@ -25,18 +25,47 @@ moving it at the same time (controlled by the
> argument and
> the available virtual address space).
> .P
> +Mappings can also simply be moved
> +(without any resizing)
> +by specifying equal
> +.I old_size
> +and
> +.I new_size
> +and using the
> +.B MREMAP_FIXED
> +flag
> +(see below).
> +Since Linux 6.17,
> +while
> +.I old_address
> +must reside within a mapping,
> +.I old_size
> +may span multiple mappings
> +which do not have to be
> +adjacent to one another when
> +performing a move like this.
> +The
> +.B MREMAP_DONTUNMAP
> +flag may be specified.
> +.P
> +If the operation is not
> +simply moving mappings,
> +then
> +.I old_size
> +must span only a single mapping.
> +.P
> .I old_address
> -is the old address of the virtual memory block that you
> -want to expand (or shrink).
> +is the old address of the first virtual memory block that you
> +want to expand, shrink, and/or move.
> Note that
> .I old_address
> has to be page aligned.
> .I old_size
> -is the old size of the
> -virtual memory block.
> +is the size of the range containing
> +virtual memory blocks to be manipulated.
> .I new_size
> is the requested size of the
> -virtual memory block after the resize.
> +virtual memory blocks after the resize.
> An optional fifth argument,
> .IR new_address ,
> may be provided; see the description of
> @@ -105,13 +134,43 @@ If
> is specified, then
> .B MREMAP_MAYMOVE
> must also be specified.
> +.IP
> +Since Linux 6.17,
> +if
> +.I old_size
> +is equal to
> +.I new_size
> +and
> +.B MREMAP_FIXED
> +is specified, then
> +.I old_size
> +may span beyond the mapping in which
> +.I old_address
> +resides.
> +In this case,
> +gaps between mappings in the original range
> +are maintained in the new range.
> +The whole operation is performed atomically
> +unless an error arises,
> +in which case the operation may be partially
> +completed,
> +that is,
> +some mappings may be moved and others not.
> +.IP
> +
> +Moving multiple mappings is not permitted if
> +any of those mappings have either
> +been registered with
> +.BR userfaultfd (2) ,
> +or map drivers that
> +specify their own custom address mapping logic.
> .TP
> .BR MREMAP_DONTUNMAP " (since Linux 5.7)"
> .\" commit e346b3813067d4b17383f975f197a9aa28a3b077
> This flag, which must be used in conjunction with
> .BR MREMAP_MAYMOVE ,
> -remaps a mapping to a new address but does not unmap the mapping at
> -.IR old_address .
> +remaps mappings to a new address but does not unmap them
> +from their original address.
> .IP
> The
> .B MREMAP_DONTUNMAP
> @@ -149,13 +208,13 @@ mapped.
> See NOTES for some possible applications of
> .BR MREMAP_DONTUNMAP .
> .P
> -If the memory segment specified by
> +If the memory segments specified by
> .I old_address
> and
> .I old_size
> -is locked (using
> +are locked (using
> .BR mlock (2)
> -or similar), then this lock is maintained when the segment is
> +or similar), then this lock is maintained when the segments are
> resized and/or relocated.
> As a consequence, the amount of memory locked by the process may change.
> .SH RETURN VALUE
> @@ -188,7 +247,10 @@ virtual memory address for this process.
> You can also get
> .B EFAULT
> even if there exist mappings that cover the
> -whole address space requested, but those mappings are of different types.
> +whole address space requested, but those mappings are of different types,
> +and the
> +.BR mremap ()
> +operation being performed does not support this.
> .TP
> .B EINVAL
> An invalid argument was given.
> --
> 2.50.1
--
<https://www.alejandro-colomar.es/>
^ permalink raw reply
* Re: [PATCH v3 1/2] man/man2/mremap.2: describe multiple mapping move
From: Lorenzo Stoakes @ 2025-08-03 11:15 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-man, Andrew Morton, Peter Xu, Alexander Viro,
Christian Brauner, Jan Kara, Liam R . Howlett, Vlastimil Babka,
Jann Horn, Pedro Falcato, Rik van Riel, linux-mm, linux-kernel,
linux-api
In-Reply-To: <ngytuoex4uvu5epsdqhvhypnhqidkr7cpwmmcxrml6kpftgusb@jo5ql6eko2ir>
On Sun, Aug 03, 2025 at 08:47:28AM +0200, Alejandro Colomar wrote:
> Would it be possible to write a small C program that uses this new
> feature?
I could do, but it's an unusual use of mremap() and we don't currently have
example C code for the _general_ usage so I wonder if it might be somewhat
misleading to have example code only for this?
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v3 1/2] man/man2/mremap.2: describe multiple mapping move
From: Alejandro Colomar @ 2025-08-03 14:17 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-man, Andrew Morton, Peter Xu, Alexander Viro,
Christian Brauner, Jan Kara, Liam R . Howlett, Vlastimil Babka,
Jann Horn, Pedro Falcato, Rik van Riel, linux-mm, linux-kernel,
linux-api
In-Reply-To: <0e3d0dd8-994f-4665-969c-6daf332c5b94@lucifer.local>
[-- Attachment #1: Type: text/plain, Size: 729 bytes --]
Hi Lorenzo,
On Sun, Aug 03, 2025 at 12:15:15PM +0100, Lorenzo Stoakes wrote:
> On Sun, Aug 03, 2025 at 08:47:28AM +0200, Alejandro Colomar wrote:
> > Would it be possible to write a small C program that uses this new
> > feature?
>
> I could do, but it's an unusual use of mremap() and we don't currently have
> example C code for the _general_ usage so I wonder if it might be somewhat
> misleading to have example code only for this?
I didn't mean for the manual page. It was more for helping review the
changes. If it's very small, it would be useful to include it in the
commit message. What do you think?
Have a lovely day!
Alex
>
> Cheers, Lorenzo
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 10/32] liveupdate: luo_core: Live Update Orchestrator
From: Pasha Tatashin @ 2025-08-04 1:11 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: pratyush, jasonmiu, graf, changyuanl, rppt, dmatlack, rientjes,
corbet, rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl,
masahiroy, akpm, tj, yoann.congal, mmaurer, roman.gushchin,
chenridong, axboe, mark.rutland, jannh, vincent.guittot, hannes,
dan.j.williams, david, joel.granados, rostedt, anna.schumaker,
song, zhangguopeng, linux, linux-kernel, linux-doc, linux-mm,
gregkh, tglx, mingo, bp, dave.hansen, x86, hpa, rafael, dakr,
bartosz.golaszewski, cw00.choi, myungjoo.ham, yesanishhere,
Jonathan.Cameron, quic_zijuhu, aleksander.lobakin, ira.weiny,
andriy.shevchenko, leon, lukas, bhelgaas, wagi, djeffery,
stuart.w.hayes, ptyadav, lennart, brauner, linux-api,
linux-fsdevel, saeedm, ajayachandra, parav, leonro, witu
In-Reply-To: <20250729172812.GP36037@nvidia.com>
> > +enum liveupdate_event {
> > + LIVEUPDATE_PREPARE,
> > + LIVEUPDATE_FREEZE,
> > + LIVEUPDATE_FINISH,
> > + LIVEUPDATE_CANCEL,
> > +};
>
> I saw a later patch moves these hunks, that is poor patch planning.
Yes, you're right. I have since moved this to uapi/linux/liveupdate.h
in the introductory patch to improve the structure of the patch
series.
> Ideally an ioctl subsystem should start out with the first patch
> introducing the basic cdev, file open, ioctl dispatch, ioctl uapi
> header and related simple infrastructure.
I have modified the patch series as follows: The rudimentary parts of
the cdev, including the uapi/liveupdate.h header, are now in this
introductory patch. The rest of the ioctl interface is added in the
old patch that introduced luo_ioctl.c.
> Then you'd go basically ioctl by ioctl adding the new ioctls and
> explaining what they do in the patch commit messages.
>
> > +/**
> > + * liveupdate_state_updated - Check if the system is in the live update
> > + * 'updated' state.
> > + *
> > + * This function checks if the live update orchestrator is in the
> > + * ``LIVEUPDATE_STATE_UPDATED`` state. This state indicates that the system has
> > + * successfully rebooted into a new kernel as part of a live update, and the
> > + * preserved devices are expected to be in the process of being reclaimed.
> > + *
> > + * This is typically used by subsystems during early boot of the new kernel
> > + * to determine if they need to attempt to restore state from a previous
> > + * live update.
> > + *
> > + * @return true if the system is in the ``LIVEUPDATE_STATE_UPDATED`` state,
> > + * false otherwise.
> > + */
> > +bool liveupdate_state_updated(void)
> > +{
> > + return is_current_luo_state(LIVEUPDATE_STATE_UPDATED);
> > +}
> > +EXPORT_SYMBOL_GPL(liveupdate_state_updated);
>
> Unless there are existing in tree users there should not be exports.
Thank you, I have removed the exports from this patch and all others
in the series.
> I'm also not really sure why there is global state, I would expect the
> fd and session objects to record what kind of things they are, not
> having weird globals.
Having a global state is necessary for performance optimizations. This
is similar to why we export the state to userspace via sysfs: it
allows other subsystems to behave differently during a
performance-optimized live update versus a normal boot.
For example, in our code base we have a driver that doesn't
participate in the live update itself (it has no state to preserve).
However, during boot, it checks this global state. If it's a live
update boot, the driver skips certain steps, like loading firmware, to
accelerate the overall boot time.
In other words, even before userspace starts, this global awareness
enables optimizations that aren't necessary during a cold boot or a
regular kexec.
> Like liveupdate_register_subsystem() stuff, it already has a lock,
> &luo_subsystem_list_mutex, if you want to block mutation of the list
> then, IMHO, it makes more sense to stick a specific variable
> 'luo_subsystems_list_immutable' under that lock and make it very
> obvious.
>
> Stuff like luo_files_startup() feels clunky to me:
>
> + ret = liveupdate_register_subsystem(&luo_file_subsys);
> + if (ret) {
> + pr_warn("Failed to register luo_file subsystem [%d]\n", ret);
> + return ret;
> + }
> +
> + if (liveupdate_state_updated()) {
>
> Thats going to be a standard pattern - I would expect that
> liveupdate_register_subsystem() would do the check for updated and
> then arrange to call back something like
> liveupdate_subsystem.ops.post_update()
>
> And then post_update() would get the info that is currently under
> liveupdate_get_subsystem_data() as arguments instead of having to make
> more functions calls.
>
> Maybe even the fdt_node_check_compatible() can be hoisted.
>
> That would remove a bunch more liveupdate_state_updated() calls.
That's a good suggestion for a potential refactor. For now, the
state-check call is inexpensive and is not in a performance-critical
path. We can certainly implement this optimization later if it becomes
necessary.
Thank you,
Pasha
^ permalink raw reply
* copy_file_range return value on FUSE
From: Florian Weimer @ 2025-08-04 9:41 UTC (permalink / raw)
To: fuse-devel, linux-api, linux-fsdevel, linux-kernel
The FUSE protocol uses struct fuse_write_out to convey the return value
of copy_file_range, which is restricted to uint32_t. But the
copy_file_range interface supports a 64-bit copy operation. Given that
copy_file_range is expected to clone huge files, large copies are not
unexpected, so this appears to be a real limitation.
There is another wrinkle: we'd need to check if the process runs in
32-bit compat mode, and reject size_t arguments larger than INT_MAX in
this case (with EOVERFLOW presumably). But perhaps this should be
handled on the kernel side? Currently, this doesn't seem to happen, and
we can get copy_file_range results in the in-band error range.
Applications have no way to disambiguate this.
Thanks,
Florian
^ permalink raw reply
* Re: [fuse-devel] copy_file_range return value on FUSE
From: Miklos Szeredi @ 2025-08-04 13:30 UTC (permalink / raw)
To: Florian Weimer; +Cc: fuse-devel, linux-api, linux-fsdevel, linux-kernel
In-Reply-To: <lhuh5ynl8z5.fsf@oldenburg.str.redhat.com>
On Mon, 4 Aug 2025 at 11:42, Florian Weimer via fuse-devel
<fuse-devel@lists.sourceforge.net> wrote:
>
> The FUSE protocol uses struct fuse_write_out to convey the return value
> of copy_file_range, which is restricted to uint32_t. But the
> copy_file_range interface supports a 64-bit copy operation. Given that
> copy_file_range is expected to clone huge files, large copies are not
> unexpected, so this appears to be a real limitation.
That's a nasty oversight. Fixing with a new FUSE_COPY_FILE_RANGE_64
op, fallback to the legacy FUSE_COPY_FILE_RANGE.
> There is another wrinkle: we'd need to check if the process runs in
> 32-bit compat mode, and reject size_t arguments larger than INT_MAX in
> this case (with EOVERFLOW presumably). But perhaps this should be
> handled on the kernel side? Currently, this doesn't seem to happen, and
> we can get copy_file_range results in the in-band error range.
> Applications have no way to disambiguate this.
That's not fuse specific, right?
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH v3 1/2] man/man2/mremap.2: describe multiple mapping move
From: Lorenzo Stoakes @ 2025-08-04 13:31 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-man, Andrew Morton, Peter Xu, Alexander Viro,
Christian Brauner, Jan Kara, Liam R . Howlett, Vlastimil Babka,
Jann Horn, Pedro Falcato, Rik van Riel, linux-mm, linux-kernel,
linux-api
In-Reply-To: <dizfk5jqwwzozotvkk6phb36blcpsve3yw53xrdjwqeut27djt@rftbd7lkgsjp>
On Sun, Aug 03, 2025 at 04:17:39PM +0200, Alejandro Colomar wrote:
> Hi Lorenzo,
>
> On Sun, Aug 03, 2025 at 12:15:15PM +0100, Lorenzo Stoakes wrote:
> > On Sun, Aug 03, 2025 at 08:47:28AM +0200, Alejandro Colomar wrote:
> > > Would it be possible to write a small C program that uses this new
> > > feature?
> >
> > I could do, but it's an unusual use of mremap() and we don't currently have
> > example C code for the _general_ usage so I wonder if it might be somewhat
> > misleading to have example code only for this?
>
> I didn't mean for the manual page. It was more for helping review the
> changes. If it's very small, it would be useful to include it in the
> commit message. What do you think?
Ack I can do.
In discussion with colleagues I also realise I have to add a little more
information to 2/2 as well, so worth a respin anyway.
>
>
> Have a lovely day!
> Alex
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [fuse-devel] copy_file_range return value on FUSE
From: Florian Weimer @ 2025-08-04 14:30 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: fuse-devel, linux-api, linux-fsdevel, linux-kernel
In-Reply-To: <CAJfpegur9fUQ8MaOqrE-XrGUDK40+PGQeMZ+AzzpX6hNV_BKsw@mail.gmail.com>
* Miklos Szeredi:
> On Mon, 4 Aug 2025 at 11:42, Florian Weimer via fuse-devel
> <fuse-devel@lists.sourceforge.net> wrote:
>>
>> The FUSE protocol uses struct fuse_write_out to convey the return value
>> of copy_file_range, which is restricted to uint32_t. But the
>> copy_file_range interface supports a 64-bit copy operation. Given that
>> copy_file_range is expected to clone huge files, large copies are not
>> unexpected, so this appears to be a real limitation.
>
> That's a nasty oversight. Fixing with a new FUSE_COPY_FILE_RANGE_64
> op, fallback to the legacy FUSE_COPY_FILE_RANGE.
Or adding a capability flag to switch from struct fuse_write_out to
something that uses an uint64_t value. One complication: The struct
fuse_write_out layout is too close to a potential 64-bit version of it
on little-endian systems, so that proper testing might be difficult with
the obvious approach.
>> There is another wrinkle: we'd need to check if the process runs in
>> 32-bit compat mode, and reject size_t arguments larger than INT_MAX in
>> this case (with EOVERFLOW presumably). But perhaps this should be
>> handled on the kernel side? Currently, this doesn't seem to happen, and
>> we can get copy_file_range results in the in-band error range.
>> Applications have no way to disambiguate this.
>
> That's not fuse specific, right?
In-kernel file systems can check if the request originated from a compat
process, using in_compat_syscall. I don't think that's possible over
FUSE.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH v2 14/32] liveupdate: luo_files: add infrastructure for FDs
From: Pasha Tatashin @ 2025-08-04 23:00 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: pratyush, jasonmiu, graf, changyuanl, rppt, dmatlack, rientjes,
corbet, rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl,
masahiroy, akpm, tj, yoann.congal, mmaurer, roman.gushchin,
chenridong, axboe, mark.rutland, jannh, vincent.guittot, hannes,
dan.j.williams, david, joel.granados, rostedt, anna.schumaker,
song, zhangguopeng, linux, linux-kernel, linux-doc, linux-mm,
gregkh, tglx, mingo, bp, dave.hansen, x86, hpa, rafael, dakr,
bartosz.golaszewski, cw00.choi, myungjoo.ham, yesanishhere,
Jonathan.Cameron, quic_zijuhu, aleksander.lobakin, ira.weiny,
andriy.shevchenko, leon, lukas, bhelgaas, wagi, djeffery,
stuart.w.hayes, ptyadav, lennart, brauner, linux-api,
linux-fsdevel, saeedm, ajayachandra, parav, leonro, witu
In-Reply-To: <20250729173318.GQ36037@nvidia.com>
> > +struct liveupdate_file_ops {
> > + int (*prepare)(struct file *file, void *arg, u64 *data);
> > + int (*freeze)(struct file *file, void *arg, u64 *data);
> > + void (*cancel)(struct file *file, void *arg, u64 data);
> > + void (*finish)(struct file *file, void *arg, u64 data, bool reclaimed);
> > + int (*retrieve)(void *arg, u64 data, struct file **file);
> > + bool (*can_preserve)(struct file *file, void *arg);
> > +};
>
> ops structures often have an owner = THIS_MODULE
Added here, and to subsystems.
>
> It wouldn't hurt to add it here too, and some appropriate module_get's
> though I didn't try to figure what happens if userspace races a module
> unload with other luo operations.
I added try_module_get()/module_put() to register/unregister functions.
> > +
> > +/**
> > + * struct liveupdate_file_handler - Represents a handler for a live-updatable
> > + * file type.
> > + * @ops: Callback functions
> > + * @compatible: The compatibility string (e.g., "memfd-v1", "vfiofd-v1")
> > + * that uniquely identifies the file type this handler supports.
> > + * This is matched against the compatible string associated with
> > + * individual &struct liveupdate_file instances.
> > + * @arg: An opaque pointer to implementation-specific context data
> > + * associated with this file handler registration.
>
> Why? This is not the normal way, if you want context data then
> allocate a struct driver_liveupdate_file_handler and embed a normal
> struct liveupdate_file_handler inside it, then use container_of.
Good point. I removed arg, and added handler as an argument to the
callback functions.
> > + fdt_for_each_subnode(file_node_offset, luo_file_fdt_in, 0) {
> > + bool handler_found = false;
> > + u64 token;
> > +
> > + node_name = fdt_get_name(luo_file_fdt_in, file_node_offset,
> > + NULL);
> > + if (!node_name) {
> > + panic("FDT subnode at offset %d: Cannot get name\n",
> > + file_node_offset);
>
> I think this approach will raise lots of questions..
>
> I'd introduce a new function "luo_deserialize_failure" that does panic
> internally.
>
> Only called by places that are parsing the FDT & related but run into
> trouble that cannot be savely recovered from.
Agreed. I added a new macro in luo_internal.h:
11 /*
12 * Handles a deserialization failure: devices and memory is in
unpredictable
13 * state.
14 *
15 * Continuing the boot process after a failure is dangerous
because it could
16 * lead to leaks of private data.
17 */
18 #define luo_restore_fail(__fmt, ...) panic(__fmt, ##__VA_ARGS__)
And use it in places where we panic during deserialization.
Pasha
^ permalink raw reply
* [PATCH v4 1/4] pidns: move is-ancestor logic to helper
From: Aleksa Sarai @ 2025-08-05 5:45 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: Andy Lutomirski, linux-kernel, linux-fsdevel, linux-api,
linux-doc, linux-kselftest, Aleksa Sarai
In-Reply-To: <20250805-procfs-pidns-api-v4-0-705f984940e7@cyphar.com>
This check will be needed in later patches, and there's no point
open-coding it each time.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
include/linux/pid_namespace.h | 9 +++++++++
kernel/pid_namespace.c | 22 ++++++++++++++--------
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 7c67a5811199..17fdc059f8da 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -84,6 +84,9 @@ extern void zap_pid_ns_processes(struct pid_namespace *pid_ns);
extern int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd);
extern void put_pid_ns(struct pid_namespace *ns);
+extern bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor);
+
#else /* !CONFIG_PID_NS */
#include <linux/err.h>
@@ -118,6 +121,12 @@ static inline int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd)
{
return 0;
}
+
+static inline bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor)
+{
+ return false;
+}
#endif /* CONFIG_PID_NS */
extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 7098ed44e717..b7b45c2597ec 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -390,11 +390,23 @@ static void pidns_put(struct ns_common *ns)
put_pid_ns(to_pid_ns(ns));
}
+bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor)
+{
+ struct pid_namespace *ns;
+
+ if (child->level < ancestor->level)
+ return false;
+ for (ns = child; ns->level > ancestor->level; ns = ns->parent)
+ ;
+ return ns == ancestor;
+}
+
static int pidns_install(struct nsset *nsset, struct ns_common *ns)
{
struct nsproxy *nsproxy = nsset->nsproxy;
struct pid_namespace *active = task_active_pid_ns(current);
- struct pid_namespace *ancestor, *new = to_pid_ns(ns);
+ struct pid_namespace *new = to_pid_ns(ns);
if (!ns_capable(new->user_ns, CAP_SYS_ADMIN) ||
!ns_capable(nsset->cred->user_ns, CAP_SYS_ADMIN))
@@ -408,13 +420,7 @@ static int pidns_install(struct nsset *nsset, struct ns_common *ns)
* this maintains the property that processes and their
* children can not escape their current pid namespace.
*/
- if (new->level < active->level)
- return -EINVAL;
-
- ancestor = new;
- while (ancestor->level > active->level)
- ancestor = ancestor->parent;
- if (ancestor != active)
+ if (!pidns_is_ancestor(new, active))
return -EINVAL;
put_pid_ns(nsproxy->pid_ns_for_children);
--
2.50.1
^ permalink raw reply related
* [PATCH v4 2/4] procfs: add "pidns" mount option
From: Aleksa Sarai @ 2025-08-05 5:45 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: Andy Lutomirski, linux-kernel, linux-fsdevel, linux-api,
linux-doc, linux-kselftest, Aleksa Sarai
In-Reply-To: <20250805-procfs-pidns-api-v4-0-705f984940e7@cyphar.com>
Since the introduction of pid namespaces, their interaction with procfs
has been entirely implicit in ways that require a lot of dancing around
by programs that need to construct sandboxes with different PID
namespaces.
Being able to explicitly specify the pid namespace to use when
constructing a procfs super block will allow programs to no longer need
to fork off a process which does then does unshare(2) / setns(2) and
forks again in order to construct a procfs in a pidns.
So, provide a "pidns" mount option which allows such users to just
explicitly state which pid namespace they want that procfs instance to
use. This interface can be used with fsconfig(2) either with a file
descriptor or a path:
fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
or with classic mount(2) / mount(8):
// mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
As this new API is effectively shorthand for setns(2) followed by
mount(2), the permission model for this mirrors pidns_install() to avoid
opening up new attack surfaces by loosening the existing permission
model.
In order to avoid having to RCU-protect all users of proc_pid_ns() (to
avoid UAFs), attempting to reconfigure an existing procfs instance's pid
namespace will error out with -EBUSY. Creating new procfs instances is
quite cheap, so this should not be an impediment to most users, and lets
us avoid a lot of churn in fs/proc/* for a feature that it seems
unlikely userspace would use.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Documentation/filesystems/proc.rst | 8 ++++
fs/proc/root.c | 98 +++++++++++++++++++++++++++++++++++---
2 files changed, 100 insertions(+), 6 deletions(-)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 5236cb52e357..5a157dadea0b 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2360,6 +2360,7 @@ The following mount options are supported:
hidepid= Set /proc/<pid>/ access mode.
gid= Set the group authorized to learn processes information.
subset= Show only the specified subset of procfs.
+ pidns= Specify a the namespace used by this procfs.
========= ========================================================
hidepid=off or hidepid=0 means classic mode - everybody may access all
@@ -2392,6 +2393,13 @@ information about processes information, just add identd to this group.
subset=pid hides all top level files and directories in the procfs that
are not related to tasks.
+pidns= specifies a pid namespace (either as a string path to something like
+`/proc/$pid/ns/pid`, or a file descriptor when using `FSCONFIG_SET_FD`) that
+will be used by the procfs instance when translating pids. By default, procfs
+will use the calling process's active pid namespace. Note that the pid
+namespace of an existing procfs instance cannot be modified (attempting to do
+so will give an `-EBUSY` error).
+
Chapter 5: Filesystem behavior
==============================
diff --git a/fs/proc/root.c b/fs/proc/root.c
index ed86ac710384..fd1f1c8a939a 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -38,12 +38,14 @@ enum proc_param {
Opt_gid,
Opt_hidepid,
Opt_subset,
+ Opt_pidns,
};
static const struct fs_parameter_spec proc_fs_parameters[] = {
- fsparam_u32("gid", Opt_gid),
+ fsparam_u32("gid", Opt_gid),
fsparam_string("hidepid", Opt_hidepid),
fsparam_string("subset", Opt_subset),
+ fsparam_file_or_string("pidns", Opt_pidns),
{}
};
@@ -109,11 +111,66 @@ static int proc_parse_subset_param(struct fs_context *fc, char *value)
return 0;
}
+#ifdef CONFIG_PID_NS
+static int proc_parse_pidns_param(struct fs_context *fc,
+ struct fs_parameter *param,
+ struct fs_parse_result *result)
+{
+ struct proc_fs_context *ctx = fc->fs_private;
+ struct pid_namespace *target, *active = task_active_pid_ns(current);
+ struct ns_common *ns;
+ struct file *ns_filp __free(fput) = NULL;
+
+ switch (param->type) {
+ case fs_value_is_file:
+ /* came through fsconfig, steal the file reference */
+ ns_filp = no_free_ptr(param->file);
+ break;
+ case fs_value_is_string:
+ ns_filp = filp_open(param->string, O_RDONLY, 0);
+ break;
+ default:
+ WARN_ON_ONCE(true);
+ break;
+ }
+ if (!ns_filp)
+ ns_filp = ERR_PTR(-EBADF);
+ if (IS_ERR(ns_filp)) {
+ errorfc(fc, "could not get file from pidns argument");
+ return PTR_ERR(ns_filp);
+ }
+
+ if (!proc_ns_file(ns_filp))
+ return invalfc(fc, "pidns argument is not an nsfs file");
+ ns = get_proc_ns(file_inode(ns_filp));
+ if (ns->ops->type != CLONE_NEWPID)
+ return invalfc(fc, "pidns argument is not a pidns file");
+ target = container_of(ns, struct pid_namespace, ns);
+
+ /*
+ * pidns= is shorthand for joining the pidns to get a fsopen fd, so the
+ * permission model should be the same as pidns_install().
+ */
+ if (!ns_capable(target->user_ns, CAP_SYS_ADMIN)) {
+ errorfc(fc, "insufficient permissions to set pidns");
+ return -EPERM;
+ }
+ if (!pidns_is_ancestor(target, active))
+ return invalfc(fc, "cannot set pidns to non-descendant pidns");
+
+ put_pid_ns(ctx->pid_ns);
+ ctx->pid_ns = get_pid_ns(target);
+ put_user_ns(fc->user_ns);
+ fc->user_ns = get_user_ns(ctx->pid_ns->user_ns);
+ return 0;
+}
+#endif /* CONFIG_PID_NS */
+
static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct proc_fs_context *ctx = fc->fs_private;
struct fs_parse_result result;
- int opt;
+ int opt, err;
opt = fs_parse(fc, proc_fs_parameters, param, &result);
if (opt < 0)
@@ -125,14 +182,38 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
break;
case Opt_hidepid:
- if (proc_parse_hidepid_param(fc, param))
- return -EINVAL;
+ err = proc_parse_hidepid_param(fc, param);
+ if (err)
+ return err;
break;
case Opt_subset:
- if (proc_parse_subset_param(fc, param->string) < 0)
- return -EINVAL;
+ err = proc_parse_subset_param(fc, param->string);
+ if (err)
+ return err;
+ break;
+
+ case Opt_pidns:
+#ifdef CONFIG_PID_NS
+ /*
+ * We would have to RCU-protect every proc_pid_ns() or
+ * proc_sb_info() access if we allowed this to be reconfigured
+ * for an existing procfs instance. Luckily, procfs instances
+ * are cheap to create, and mount-beneath would let you
+ * atomically replace an instance even with overmounts.
+ */
+ if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+ errorfc(fc, "cannot reconfigure pidns for existing procfs");
+ return -EBUSY;
+ }
+ err = proc_parse_pidns_param(fc, param, &result);
+ if (err)
+ return err;
break;
+#else
+ errorfc(fc, "pidns mount flag not supported on this system");
+ return -EOPNOTSUPP;
+#endif
default:
return -EINVAL;
@@ -154,6 +235,11 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
fs_info->hide_pid = ctx->hidepid;
if (ctx->mask & (1 << Opt_subset))
fs_info->pidonly = ctx->pidonly;
+ if (ctx->mask & (1 << Opt_pidns) &&
+ !WARN_ON_ONCE(fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)) {
+ put_pid_ns(fs_info->pid_ns);
+ fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+ }
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
--
2.50.1
^ permalink raw reply related
* [PATCH v4 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl
From: Aleksa Sarai @ 2025-08-05 5:45 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: Andy Lutomirski, linux-kernel, linux-fsdevel, linux-api,
linux-doc, linux-kselftest, Aleksa Sarai
In-Reply-To: <20250805-procfs-pidns-api-v4-0-705f984940e7@cyphar.com>
/proc has historically had very opaque semantics about PID namespaces,
which is a little unfortunate for container runtimes and other programs
that deal with switching namespaces very often. One common issue is that
of converting between PIDs in the process's namespace and PIDs in the
namespace of /proc.
In principle, it is possible to do this today by opening a pidfd with
pidfd_open(2) and then looking at /proc/self/fdinfo/$n (which will
contain a PID value translated to the pid namespace associated with that
procfs superblock). However, allocating a new file for each PID to be
converted is less than ideal for programs that may need to scan procfs,
and it is generally useful for userspace to be able to finally get this
information from procfs.
So, add a new API to get the pid namespace of a procfs instance, in the
form of an ioctl(2) you can call on the root directory of said procfs.
The returned file descriptor will have O_CLOEXEC set. This acts as a
sister feature to the new "pidns" mount option, finally allowing
userspace full control of the pid namespaces associated with procfs
instances.
The permission model for this is a bit looser than that of the "pidns"
mount option (and also setns(2)) because /proc/1/ns/pid provides the
same information, so as long as you have access to that magic-link (or
something equivalently reasonable such as being in an ancestor pid
namespace) it makes sense to allow userspace to grab a handle. Ideally
we would check for ptrace-read access against all processes in the pidns
(which is very likely to be true for at least one process, as
SUID_DUMP_DISABLE is cleared on exec(2) and is rarely set by most
programs), but this would obviously not scale.
setns(2) will still have their own permission checks, so being able to
open a pidns handle doesn't really provide too many other capabilities.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Documentation/filesystems/proc.rst | 4 +++
fs/proc/root.c | 68 ++++++++++++++++++++++++++++++++++++--
include/uapi/linux/fs.h | 4 +++
3 files changed, 74 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 5a157dadea0b..840f820fb467 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2400,6 +2400,10 @@ will use the calling process's active pid namespace. Note that the pid
namespace of an existing procfs instance cannot be modified (attempting to do
so will give an `-EBUSY` error).
+Processes can check which pid namespace is used by a procfs instance by using
+the `PROCFS_GET_PID_NAMESPACE` ioctl() on the root directory of the procfs
+instance.
+
Chapter 5: Filesystem behavior
==============================
diff --git a/fs/proc/root.c b/fs/proc/root.c
index fd1f1c8a939a..ac9b115fad7b 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -23,8 +23,10 @@
#include <linux/cred.h>
#include <linux/magic.h>
#include <linux/slab.h>
+#include <linux/ptrace.h>
#include "internal.h"
+#include "../internal.h"
struct proc_fs_context {
struct pid_namespace *pid_ns;
@@ -426,15 +428,77 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
return proc_pid_readdir(file, ctx);
}
+static long int proc_root_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ switch (cmd) {
+ case PROCFS_GET_PID_NAMESPACE: {
+#ifdef CONFIG_PID_NS
+ struct pid_namespace *active = task_active_pid_ns(current);
+ struct pid_namespace *ns = proc_pid_ns(file_inode(filp)->i_sb);
+ bool can_access_pidns = false;
+
+ /*
+ * Having a handle to a pidns is not sufficient to do anything
+ * particularly harmful, as setns(2) has its own separate
+ * privilege checks. So, we can loosen the privilege
+ * requirements here a little to make this more ergonomic.
+ *
+ * If we are in an ancestor pidns of the pidns, then we can
+ * already address any process in the pidns. From a setns(2)
+ * privileges perspective, we can create a pidfd which setns(2)
+ * would also accept (pending any privilege checks).
+ *
+ * If we are not in an ancestor pidns, because this operation
+ * is being done on the root of the /proc instance, the caller
+ * can try to access /proc/1/ns/pid which is equivalent to this
+ * ioctl and so we should copy the PTRACE_MODE_READ_FSCREDS
+ * permission model use by proc_ns_get_link(). Ideally we would
+ * check for ptrace-read access against all processes in the
+ * pidns (which is very likely to be true for at least one
+ * process, as SUID_DUMP_DISABLE is cleared on exec(2) and is
+ * rarely set by most programs), but this would obviously not
+ * scale.
+ *
+ * If there is no root process, then there is no real downside
+ * to unprivileged users to open a handle to it.
+ */
+ can_access_pidns = pidns_is_ancestor(ns, active);
+ if (!can_access_pidns) {
+ bool cannot_ptrace_pid1 = false;
+
+ read_lock(&tasklist_lock);
+ if (ns->child_reaper)
+ cannot_ptrace_pid1 = ptrace_may_access(ns->child_reaper,
+ PTRACE_MODE_READ_FSCREDS);
+ read_unlock(&tasklist_lock);
+ can_access_pidns = !cannot_ptrace_pid1;
+ }
+ if (!can_access_pidns)
+ return -EPERM;
+
+ /* open_namespace() unconditionally consumes the reference. */
+ get_pid_ns(ns);
+ return open_namespace(to_ns_common(ns));
+#else
+ return -EOPNOTSUPP;
+#endif
+ }
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
/*
* The root /proc directory is special, as it has the
* <pid> directories. Thus we don't use the generic
* directory handling functions for that..
*/
static const struct file_operations proc_root_operations = {
- .read = generic_read_dir,
- .iterate_shared = proc_root_readdir,
+ .read = generic_read_dir,
+ .iterate_shared = proc_root_readdir,
.llseek = generic_file_llseek,
+ .unlocked_ioctl = proc_root_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
};
/*
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 0bd678a4a10e..68e65e6d7d6b 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -435,8 +435,12 @@ typedef int __bitwise __kernel_rwf_t;
RWF_APPEND | RWF_NOAPPEND | RWF_ATOMIC |\
RWF_DONTCACHE)
+/* This matches XSDFEC_MAGIC, so we need to allocate subvalues carefully. */
#define PROCFS_IOCTL_MAGIC 'f'
+/* procfs root ioctls */
+#define PROCFS_GET_PID_NAMESPACE _IO(PROCFS_IOCTL_MAGIC, 32)
+
/* Pagemap ioctl */
#define PAGEMAP_SCAN _IOWR(PROCFS_IOCTL_MAGIC, 16, struct pm_scan_arg)
--
2.50.1
^ permalink raw reply related
* [PATCH v4 4/4] selftests/proc: add tests for new pidns APIs
From: Aleksa Sarai @ 2025-08-05 5:45 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: Andy Lutomirski, linux-kernel, linux-fsdevel, linux-api,
linux-doc, linux-kselftest, Aleksa Sarai
In-Reply-To: <20250805-procfs-pidns-api-v4-0-705f984940e7@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
tools/testing/selftests/proc/proc-pidns.c | 315 ++++++++++++++++++++++++++++++
3 files changed, 317 insertions(+)
diff --git a/tools/testing/selftests/proc/.gitignore b/tools/testing/selftests/proc/.gitignore
index 973968f45bba..2dced03e9e0e 100644
--- a/tools/testing/selftests/proc/.gitignore
+++ b/tools/testing/selftests/proc/.gitignore
@@ -17,6 +17,7 @@
/proc-tid0
/proc-uptime-001
/proc-uptime-002
+/proc-pidns
/read
/self
/setns-dcache
diff --git a/tools/testing/selftests/proc/Makefile b/tools/testing/selftests/proc/Makefile
index b12921b9794b..c6f7046b9860 100644
--- a/tools/testing/selftests/proc/Makefile
+++ b/tools/testing/selftests/proc/Makefile
@@ -27,5 +27,6 @@ TEST_GEN_PROGS += setns-sysvipc
TEST_GEN_PROGS += thread-self
TEST_GEN_PROGS += proc-multiple-procfs
TEST_GEN_PROGS += proc-fsconfig-hidepid
+TEST_GEN_PROGS += proc-pidns
include ../lib.mk
diff --git a/tools/testing/selftests/proc/proc-pidns.c b/tools/testing/selftests/proc/proc-pidns.c
new file mode 100644
index 000000000000..f7dd80a2c150
--- /dev/null
+++ b/tools/testing/selftests/proc/proc-pidns.c
@@ -0,0 +1,315 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Author: Aleksa Sarai <cyphar@cyphar.com>
+ * Copyright (C) 2025 SUSE LLC.
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/prctl.h>
+
+#include "../kselftest_harness.h"
+
+#define ASSERT_ERRNO(expected, _t, seen) \
+ __EXPECT(expected, #expected, \
+ ({__typeof__(seen) _tmp_seen = (seen); \
+ _tmp_seen >= 0 ? _tmp_seen : -errno; }), #seen, _t, 1)
+
+#define ASSERT_ERRNO_EQ(expected, seen) \
+ ASSERT_ERRNO(expected, ==, seen)
+
+#define ASSERT_SUCCESS(seen) \
+ ASSERT_ERRNO(0, <=, seen)
+
+static int touch(char *path)
+{
+ int fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC, 0644);
+ if (fd < 0)
+ return -1;
+ return close(fd);
+}
+
+FIXTURE(ns)
+{
+ int host_mntns, host_pidns;
+ int dummy_pidns;
+};
+
+FIXTURE_SETUP(ns)
+{
+ /* Stash the old mntns. */
+ self->host_mntns = open("/proc/self/ns/mnt", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(self->host_mntns);
+
+ /* Create a new mount namespace and make it private. */
+ ASSERT_SUCCESS(unshare(CLONE_NEWNS));
+ ASSERT_SUCCESS(mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL));
+
+ /*
+ * Create a proper tmpfs that we can use and will disappear once we
+ * leave this mntns.
+ */
+ ASSERT_SUCCESS(mount("tmpfs", "/tmp", "tmpfs", 0, NULL));
+
+ /*
+ * Create a pidns we can use for later tests. We need to fork off a
+ * child so that we get a usable nsfd that we can bind-mount and open.
+ */
+ ASSERT_SUCCESS(mkdir("/tmp/dummy", 0755));
+ ASSERT_SUCCESS(touch("/tmp/dummy/pidns"));
+ ASSERT_SUCCESS(mkdir("/tmp/dummy/proc", 0755));
+
+ self->host_pidns = open("/proc/self/ns/pid", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(self->host_pidns);
+ ASSERT_SUCCESS(unshare(CLONE_NEWPID));
+
+ pid_t pid = fork();
+ ASSERT_SUCCESS(pid);
+ if (!pid) {
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+ ASSERT_SUCCESS(mount("/proc/self/ns/pid", "/tmp/dummy/pidns", NULL, MS_BIND, NULL));
+ ASSERT_SUCCESS(mount("proc", "/tmp/dummy/proc", "proc", 0, NULL));
+ exit(0);
+ }
+
+ int wstatus;
+ ASSERT_EQ(waitpid(pid, &wstatus, 0), pid);
+ ASSERT_TRUE(WIFEXITED(wstatus));
+ ASSERT_EQ(WEXITSTATUS(wstatus), 0);
+
+ ASSERT_SUCCESS(setns(self->host_pidns, CLONE_NEWPID));
+
+ self->dummy_pidns = open("/tmp/dummy/pidns", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(self->dummy_pidns);
+}
+
+FIXTURE_TEARDOWN(ns)
+{
+ ASSERT_SUCCESS(setns(self->host_mntns, CLONE_NEWNS));
+ ASSERT_SUCCESS(close(self->host_mntns));
+
+ ASSERT_SUCCESS(close(self->host_pidns));
+ ASSERT_SUCCESS(close(self->dummy_pidns));
+}
+
+TEST_F(ns, pidns_mount_string_path)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc-host", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-host", "proc", 0, "pidns=/proc/self/ns/pid"));
+ ASSERT_SUCCESS(access("/tmp/proc-host/self/", X_OK));
+
+ ASSERT_SUCCESS(mkdir("/tmp/proc-dummy", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-dummy", "proc", 0, "pidns=/tmp/dummy/pidns"));
+ ASSERT_ERRNO_EQ(-ENOENT, access("/tmp/proc-dummy/1/", X_OK));
+ ASSERT_ERRNO_EQ(-ENOENT, access("/tmp/proc-dummy/self/", X_OK));
+}
+
+TEST_F(ns, pidns_fsconfig_string_path)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_SUCCESS(fsfd);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy/pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_SUCCESS(mountfd);
+
+ ASSERT_ERRNO_EQ(-ENOENT, faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_ERRNO_EQ(-ENOENT, faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_fsconfig_fd)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_SUCCESS(fsfd);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_SUCCESS(mountfd);
+
+ ASSERT_ERRNO_EQ(-ENOENT, faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_ERRNO_EQ(-ENOENT, faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_reconfigure_remount)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc", "proc", 0, ""));
+
+ ASSERT_SUCCESS(access("/tmp/proc/1/", X_OK));
+ ASSERT_SUCCESS(access("/tmp/proc/self/", X_OK));
+
+ ASSERT_ERRNO_EQ(-EBUSY, mount(NULL, "/tmp/proc", NULL, MS_REMOUNT, "pidns=/tmp/dummy/pidns"));
+
+ ASSERT_SUCCESS(access("/tmp/proc/1/", X_OK));
+ ASSERT_SUCCESS(access("/tmp/proc/self/", X_OK));
+}
+
+TEST_F(ns, pidns_reconfigure_fsconfig_string_path)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_SUCCESS(fsfd);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_SUCCESS(mountfd);
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_ERRNO_EQ(-EBUSY, fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy/pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0)); /* noop */
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_reconfigure_fsconfig_fd)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_SUCCESS(fsfd);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_SUCCESS(mountfd);
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_ERRNO_EQ(-EBUSY, fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0)); /* noop */
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+int is_same_inode(int fd1, int fd2)
+{
+ struct stat stat1, stat2;
+
+ assert(fstat(fd1, &stat1) == 0);
+ assert(fstat(fd2, &stat2) == 0);
+
+ return stat1.st_ino == stat2.st_ino && stat1.st_dev == stat2.st_dev;
+}
+
+#define PROCFS_IOCTL_MAGIC 'f'
+#define PROCFS_GET_PID_NAMESPACE _IO(PROCFS_IOCTL_MAGIC, 32)
+
+TEST_F(ns, host_get_pidns_ioctl)
+{
+ int procfs = open("/proc", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(procfs);
+
+ int procfs_pidns = ioctl(procfs, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_SUCCESS(procfs_pidns);
+
+ ASSERT_TRUE(is_same_inode(self->host_pidns, procfs_pidns));
+ ASSERT_FALSE(is_same_inode(self->dummy_pidns, procfs_pidns));
+
+ ASSERT_SUCCESS(close(procfs));
+ ASSERT_SUCCESS(close(procfs_pidns));
+}
+
+TEST_F(ns, mount_implicit_get_pidns_ioctl)
+{
+ int procfs = open("/tmp/dummy/proc", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(procfs);
+
+ int procfs_pidns = ioctl(procfs, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_SUCCESS(procfs_pidns);
+
+ ASSERT_FALSE(is_same_inode(self->host_pidns, procfs_pidns));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, procfs_pidns));
+
+ ASSERT_SUCCESS(close(procfs));
+ ASSERT_SUCCESS(close(procfs_pidns));
+}
+
+TEST_F(ns, mount_pidns_get_pidns_ioctl)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc-host", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-host", "proc", 0, "pidns=/proc/self/ns/pid"));
+
+ int host_procfs = open("/tmp/proc-host", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(host_procfs);
+ int host_procfs_pidns = ioctl(host_procfs, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_SUCCESS(host_procfs_pidns);
+
+ ASSERT_TRUE(is_same_inode(self->host_pidns, host_procfs_pidns));
+ ASSERT_FALSE(is_same_inode(self->dummy_pidns, host_procfs_pidns));
+
+ ASSERT_SUCCESS(mkdir("/tmp/proc-dummy", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-dummy", "proc", 0, "pidns=/tmp/dummy/pidns"));
+
+ int dummy_procfs = open("/tmp/proc-dummy", O_RDONLY|O_CLOEXEC);
+ ASSERT_SUCCESS(dummy_procfs);
+ int dummy_procfs_pidns = ioctl(dummy_procfs, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_SUCCESS(dummy_procfs_pidns);
+
+ ASSERT_FALSE(is_same_inode(self->host_pidns, dummy_procfs_pidns));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, dummy_procfs_pidns));
+
+ ASSERT_SUCCESS(close(host_procfs));
+ ASSERT_SUCCESS(close(host_procfs_pidns));
+ ASSERT_SUCCESS(close(dummy_procfs));
+ ASSERT_SUCCESS(close(dummy_procfs_pidns));
+}
+
+TEST_F(ns, fsconfig_pidns_get_pidns_ioctl)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_SUCCESS(fsfd);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_SUCCESS(mountfd);
+
+ /* fsmount returns an O_PATH, which ioctl(2) doesn't accept. */
+ int new_mountfd = openat(mountfd, ".", O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+ ASSERT_SUCCESS(new_mountfd);
+
+ ASSERT_SUCCESS(close(mountfd));
+ mountfd = -EBADF;
+
+ int procfs_pidns = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_SUCCESS(procfs_pidns);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns);
+ ASSERT_FALSE(is_same_inode(self->host_pidns, procfs_pidns));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, procfs_pidns));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(new_mountfd));
+ ASSERT_SUCCESS(close(procfs_pidns));
+}
+
+TEST_HARNESS_MAIN
--
2.50.1
^ permalink raw reply related
* [PATCH v4 0/4] procfs: make reference pidns more user-visible
From: Aleksa Sarai @ 2025-08-05 5:45 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: Andy Lutomirski, linux-kernel, linux-fsdevel, linux-api,
linux-doc, linux-kselftest, Aleksa Sarai
Ever since the introduction of pid namespaces, procfs has had very
implicit behaviour surrounding them (the pidns used by a procfs mount is
auto-selected based on the mounting process's active pidns, and the
pidns itself is basically hidden once the mount has been constructed).
/* pidns mount option for procfs */
This implicit behaviour has historically meant that userspace was
required to do some special dances in order to configure the pidns of a
procfs mount as desired. Examples include:
* In order to bypass the mnt_too_revealing() check, Kubernetes creates
a procfs mount from an empty pidns so that user namespaced containers
can be nested (without this, the nested containers would fail to
mount procfs). But this requires forking off a helper process because
you cannot just one-shot this using mount(2).
* Container runtimes in general need to fork into a container before
configuring its mounts, which can lead to security issues in the case
of shared-pidns containers (a privileged process in the pidns can
interact with your container runtime process). While
SUID_DUMP_DISABLE and user namespaces make this less of an issue, the
strict need for this due to a minor uAPI wart is kind of unfortunate.
Things would be much easier if there was a way for userspace to just
specify the pidns they want. Patch 1 implements a new "pidns" argument
which can be set using fsconfig(2):
fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
or classic mount(2) / mount(8):
// mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
The initial security model I have in this RFC is to be as conservative
as possible and just mirror the security model for setns(2) -- which
means that you can only set pidns=... to pid namespaces that your
current pid namespace is a direct ancestor of and you have CAP_SYS_ADMIN
privileges over the pid namespace. This fulfils the requirements of
container runtimes, but I suspect that this may be too strict for some
usecases.
The pidns argument is not displayed in mountinfo -- it's not clear to me
what value it would make sense to show (maybe we could just use ns_dname
to provide an identifier for the namespace, but this number would be
fairly useless to userspace). I'm open to suggestions. Note that
PROCFS_GET_PID_NAMESPACE (see below) does at least let userspace get
information about this outside of mountinfo.
Note that you cannot change the pidns of an already-created procfs
instance. The primary reason is that allowing this to be changed would
require RCU-protecting proc_pid_ns(sb) and thus auditing all of
fs/proc/* and some of the users in fs/* to make sure they wouldn't UAF
the pid namespace. Since creating procfs instances is very cheap, it
seems unnecessary to overcomplicate this upfront. Trying to reconfigure
procfs this way errors out with -EBUSY.
/* ioctl(PROCFS_GET_PID_NAMESPACE) */
In addition, being able to figure out what pid namespace is being used
by a procfs mount is quite useful when you have an administrative
process (such as a container runtime) which wants to figure out the
correct way of mapping PIDs between its own namespace and the namespace
for procfs (using NS_GET_{PID,TGID}_{IN,FROM}_PIDNS). There are
alternative ways to do this, but they all rely on ancillary information
that third-party libraries and tools do not necessarily have access to.
To make this easier, add a new ioctl (PROCFS_GET_PID_NAMESPACE) which
can be used to get a reference to the pidns that a procfs is using.
Rather than copying the (fairly strict) security model for setns(2),
apply a slightly looser model to better match what userspace can already
do:
* Make the ioctl only valid on the root (meaning that a process without
access to the procfs root -- such as only having an fd to a procfs
file or some open_tree(2)-like subset -- cannot use this API). This
means that the process already has some level of access to the
/proc/$pid directories.
* If the calling process is in an ancestor pidns, then they can already
create pidfd for processes inside the pidns, which is morally
equivalent to a pidns file descriptor according to setns(2). So it
seems reasonable to just allow it in this case. (The justification
for this model was suggested by Christian.)
* If the process has access to /proc/1/ns/pid already (i.e. has
ptrace-read access to the pidns pid1), then this ioctl is equivalent
to just opening a handle to it that way.
Ideally we would check for ptrace-read access against all processes
in the pidns (which is very likely to be true for at least one
process, as SUID_DUMP_DISABLE is cleared on exec(2) and is rarely set
by most programs), but this would obviously not scale.
I'm open to suggestions for whether we need to make this stricter (or
possibly allow more cases).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Changes in v4:
- Remove unneeded EXPORT_SYMBOL_GPL. [Christian Brauner]
- Return -EOPNOTSUPP for new APIs for CONFIG_PID_NS=n rather than
pretending they don't exist entirely. [Christian Brauner]
- PROCFS_IOCTL_MAGIC conflicts with XSDFEC_MAGIC, so we need to allocate
subvalues more carefully (switch to _IO(PROCFS_IOCTL_MAGIC, 32)).
- Add some more selftests for PROCFS_GET_PID_NAMESPACE.
- Reword argument for PROCFS_GET_PID_NAMESPACE security model based on
Christian's suggestion, and remove CAP_SYS_ADMIN edge-case (in most
cases, such a process would also have ptrace-read credentials over the
pidns pid1).
- v3: <https://lore.kernel.org/r/20250724-procfs-pidns-api-v3-0-4c685c910923@cyphar.com>
Changes in v3:
- Disallow changing pidns for existing procfs instances, as we'd
probably have to RCU-protect everything that touches the pinned pidns
reference.
- Improve tests with slightly nicer ASSERT_ERRNO* macros.
- v2: <https://lore.kernel.org/r/20250723-procfs-pidns-api-v2-0-621e7edd8e40@cyphar.com>
Changes in v2:
- #ifdef CONFIG_PID_NS
- Improve cover letter wording to make it clear we're talking about two
separate features with different permission models. [Andy Lutomirski]
- Fix build warnings in pidns_is_ancestor() patch. [kernel test robot]
- v1: <https://lore.kernel.org/r/20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
---
Aleksa Sarai (4):
pidns: move is-ancestor logic to helper
procfs: add "pidns" mount option
procfs: add PROCFS_GET_PID_NAMESPACE ioctl
selftests/proc: add tests for new pidns APIs
Documentation/filesystems/proc.rst | 12 ++
fs/proc/root.c | 166 +++++++++++++++-
include/linux/pid_namespace.h | 9 +
include/uapi/linux/fs.h | 4 +
kernel/pid_namespace.c | 22 ++-
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
tools/testing/selftests/proc/proc-pidns.c | 315 ++++++++++++++++++++++++++++++
8 files changed, 514 insertions(+), 16 deletions(-)
---
base-commit: 66639db858112bf6b0f76677f7517643d586e575
change-id: 20250717-procfs-pidns-api-8ed1583431f0
Best regards,
--
Aleksa Sarai <cyphar@cyphar.com>
^ permalink raw reply
* Re: [PATCH v4 2/4] procfs: add "pidns" mount option
From: Aleksa Sarai @ 2025-08-05 7:29 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: Andy Lutomirski, linux-kernel, linux-fsdevel, linux-api,
linux-doc, linux-kselftest, Amir Goldstein
In-Reply-To: <20250805-procfs-pidns-api-v4-2-705f984940e7@cyphar.com>
[-- Attachment #1: Type: text/plain, Size: 8905 bytes --]
On 2025-08-05, Aleksa Sarai <cyphar@cyphar.com> wrote:
> Since the introduction of pid namespaces, their interaction with procfs
> has been entirely implicit in ways that require a lot of dancing around
> by programs that need to construct sandboxes with different PID
> namespaces.
>
> Being able to explicitly specify the pid namespace to use when
> constructing a procfs super block will allow programs to no longer need
> to fork off a process which does then does unshare(2) / setns(2) and
> forks again in order to construct a procfs in a pidns.
>
> So, provide a "pidns" mount option which allows such users to just
> explicitly state which pid namespace they want that procfs instance to
> use. This interface can be used with fsconfig(2) either with a file
> descriptor or a path:
>
> fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
> fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
>
> or with classic mount(2) / mount(8):
>
> // mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
> mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
>
> As this new API is effectively shorthand for setns(2) followed by
> mount(2), the permission model for this mirrors pidns_install() to avoid
> opening up new attack surfaces by loosening the existing permission
> model.
>
> In order to avoid having to RCU-protect all users of proc_pid_ns() (to
> avoid UAFs), attempting to reconfigure an existing procfs instance's pid
> namespace will error out with -EBUSY. Creating new procfs instances is
> quite cheap, so this should not be an impediment to most users, and lets
> us avoid a lot of churn in fs/proc/* for a feature that it seems
> unlikely userspace would use.
>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> Documentation/filesystems/proc.rst | 8 ++++
> fs/proc/root.c | 98 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 100 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 5236cb52e357..5a157dadea0b 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -2360,6 +2360,7 @@ The following mount options are supported:
> hidepid= Set /proc/<pid>/ access mode.
> gid= Set the group authorized to learn processes information.
> subset= Show only the specified subset of procfs.
> + pidns= Specify a the namespace used by this procfs.
> ========= ========================================================
>
> hidepid=off or hidepid=0 means classic mode - everybody may access all
> @@ -2392,6 +2393,13 @@ information about processes information, just add identd to this group.
> subset=pid hides all top level files and directories in the procfs that
> are not related to tasks.
>
> +pidns= specifies a pid namespace (either as a string path to something like
> +`/proc/$pid/ns/pid`, or a file descriptor when using `FSCONFIG_SET_FD`) that
> +will be used by the procfs instance when translating pids. By default, procfs
> +will use the calling process's active pid namespace. Note that the pid
> +namespace of an existing procfs instance cannot be modified (attempting to do
> +so will give an `-EBUSY` error).
> +
> Chapter 5: Filesystem behavior
> ==============================
>
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index ed86ac710384..fd1f1c8a939a 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -38,12 +38,14 @@ enum proc_param {
> Opt_gid,
> Opt_hidepid,
> Opt_subset,
> + Opt_pidns,
> };
>
> static const struct fs_parameter_spec proc_fs_parameters[] = {
> - fsparam_u32("gid", Opt_gid),
> + fsparam_u32("gid", Opt_gid),
> fsparam_string("hidepid", Opt_hidepid),
> fsparam_string("subset", Opt_subset),
> + fsparam_file_or_string("pidns", Opt_pidns),
> {}
> };
>
> @@ -109,11 +111,66 @@ static int proc_parse_subset_param(struct fs_context *fc, char *value)
> return 0;
> }
>
> +#ifdef CONFIG_PID_NS
> +static int proc_parse_pidns_param(struct fs_context *fc,
> + struct fs_parameter *param,
> + struct fs_parse_result *result)
> +{
> + struct proc_fs_context *ctx = fc->fs_private;
> + struct pid_namespace *target, *active = task_active_pid_ns(current);
> + struct ns_common *ns;
> + struct file *ns_filp __free(fput) = NULL;
> +
> + switch (param->type) {
> + case fs_value_is_file:
> + /* came through fsconfig, steal the file reference */
> + ns_filp = no_free_ptr(param->file);
> + break;
> + case fs_value_is_string:
> + ns_filp = filp_open(param->string, O_RDONLY, 0);
> + break;
I just realised that we probably also want to support FSCONFIG_SET_PATH
here, but fsparam_file_or_string() doesn't handle that at the moment. I
think we probably want to have fsparam_file_or_path() which would act
like:
1. A path with FSCONFIG_SET_STRING and FSCONFIG_SET_PATH.
2. A file with FSCONFIG_SET_FD.
These are the semantics I would already expect from these kinds of
flags, but at the moment FSCONFIG_SET_PATH is entirely disallowed.
@Amir:
I wonder if overlayfs (the only other user of fsparam_file_or_string())
would also prefer having these semantics? We could just migrate
fsparam_file_or_string() to fsparam_file_or_path() everwhere, since I'm
pretty sure these are the semantics userspace expects anyway.
> + default:
> + WARN_ON_ONCE(true);
> + break;
> + }
> + if (!ns_filp)
> + ns_filp = ERR_PTR(-EBADF);
> + if (IS_ERR(ns_filp)) {
> + errorfc(fc, "could not get file from pidns argument");
> + return PTR_ERR(ns_filp);
> + }
> +
> + if (!proc_ns_file(ns_filp))
> + return invalfc(fc, "pidns argument is not an nsfs file");
> + ns = get_proc_ns(file_inode(ns_filp));
> + if (ns->ops->type != CLONE_NEWPID)
> + return invalfc(fc, "pidns argument is not a pidns file");
> + target = container_of(ns, struct pid_namespace, ns);
> +
> + /*
> + * pidns= is shorthand for joining the pidns to get a fsopen fd, so the
> + * permission model should be the same as pidns_install().
> + */
> + if (!ns_capable(target->user_ns, CAP_SYS_ADMIN)) {
> + errorfc(fc, "insufficient permissions to set pidns");
> + return -EPERM;
> + }
> + if (!pidns_is_ancestor(target, active))
> + return invalfc(fc, "cannot set pidns to non-descendant pidns");
> +
> + put_pid_ns(ctx->pid_ns);
> + ctx->pid_ns = get_pid_ns(target);
> + put_user_ns(fc->user_ns);
> + fc->user_ns = get_user_ns(ctx->pid_ns->user_ns);
> + return 0;
> +}
> +#endif /* CONFIG_PID_NS */
> +
> static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
> {
> struct proc_fs_context *ctx = fc->fs_private;
> struct fs_parse_result result;
> - int opt;
> + int opt, err;
>
> opt = fs_parse(fc, proc_fs_parameters, param, &result);
> if (opt < 0)
> @@ -125,14 +182,38 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
> break;
>
> case Opt_hidepid:
> - if (proc_parse_hidepid_param(fc, param))
> - return -EINVAL;
> + err = proc_parse_hidepid_param(fc, param);
> + if (err)
> + return err;
> break;
>
> case Opt_subset:
> - if (proc_parse_subset_param(fc, param->string) < 0)
> - return -EINVAL;
> + err = proc_parse_subset_param(fc, param->string);
> + if (err)
> + return err;
> + break;
> +
> + case Opt_pidns:
> +#ifdef CONFIG_PID_NS
> + /*
> + * We would have to RCU-protect every proc_pid_ns() or
> + * proc_sb_info() access if we allowed this to be reconfigured
> + * for an existing procfs instance. Luckily, procfs instances
> + * are cheap to create, and mount-beneath would let you
> + * atomically replace an instance even with overmounts.
> + */
> + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> + errorfc(fc, "cannot reconfigure pidns for existing procfs");
> + return -EBUSY;
> + }
> + err = proc_parse_pidns_param(fc, param, &result);
> + if (err)
> + return err;
> break;
> +#else
> + errorfc(fc, "pidns mount flag not supported on this system");
> + return -EOPNOTSUPP;
> +#endif
>
> default:
> return -EINVAL;
> @@ -154,6 +235,11 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
> fs_info->hide_pid = ctx->hidepid;
> if (ctx->mask & (1 << Opt_subset))
> fs_info->pidonly = ctx->pidonly;
> + if (ctx->mask & (1 << Opt_pidns) &&
> + !WARN_ON_ONCE(fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)) {
> + put_pid_ns(fs_info->pid_ns);
> + fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
> + }
> }
>
> static int proc_fill_super(struct super_block *s, struct fs_context *fc)
>
> --
> 2.50.1
>
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 10/32] liveupdate: luo_core: Live Update Orchestrator
From: Jason Gunthorpe @ 2025-08-05 12:31 UTC (permalink / raw)
To: Pasha Tatashin
Cc: pratyush, jasonmiu, graf, changyuanl, rppt, dmatlack, rientjes,
corbet, rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl,
masahiroy, akpm, tj, yoann.congal, mmaurer, roman.gushchin,
chenridong, axboe, mark.rutland, jannh, vincent.guittot, hannes,
dan.j.williams, david, joel.granados, rostedt, anna.schumaker,
song, zhangguopeng, linux, linux-kernel, linux-doc, linux-mm,
gregkh, tglx, mingo, bp, dave.hansen, x86, hpa, rafael, dakr,
bartosz.golaszewski, cw00.choi, myungjoo.ham, yesanishhere,
Jonathan.Cameron, quic_zijuhu, aleksander.lobakin, ira.weiny,
andriy.shevchenko, leon, lukas, bhelgaas, wagi, djeffery,
stuart.w.hayes, ptyadav, lennart, brauner, linux-api,
linux-fsdevel, saeedm, ajayachandra, parav, leonro, witu
In-Reply-To: <CA+CK2bCrfVef_sFWCQpdwe9N_go8F_pU4O-w+XBJZ6yEuXRj9g@mail.gmail.com>
On Sun, Aug 03, 2025 at 09:11:20PM -0400, Pasha Tatashin wrote:
> Having a global state is necessary for performance optimizations. This
> is similar to why we export the state to userspace via sysfs: it
> allows other subsystems to behave differently during a
> performance-optimized live update versus a normal boot.
> For example, in our code base we have a driver that doesn't
> participate in the live update itself (it has no state to preserve).
> However, during boot, it checks this global state. If it's a live
> update boot, the driver skips certain steps, like loading firmware, to
> accelerate the overall boot time.
TBH, I'm against this. Give the driver a 0 byte state if it wants to
behave differently during live update. We should not be making
implicit things like this.
Plus the usual complaining about building core kernel infrastructure
around weird out of tree drivers.
If userspace wants a device to participate in live update, even just
"optimizations", then it has to opt in.
Frankly, this driver has no idea what the prior kernel did, and by
"optimizing" I think you are actually assuming that the prior kernel
had it bound to a normal kernel driver that left it in some
predictable configuration.
Vs say bound to VFIO and completely messed up.
So this should be represented by a LUO serialization that says "the
prior kernel left this device in well defined state X" even if it
takes 0 bytes to describe that state.
So no globals, there should be a way for a driver to tell if it is
participating in LUO, but not some global 'is luo' boot fla.g
> > + ret = liveupdate_register_subsystem(&luo_file_subsys);
> > + if (ret) {
> > + pr_warn("Failed to register luo_file subsystem [%d]\n", ret);
> > + return ret;
> > + }
> > +
> > + if (liveupdate_state_updated()) {
> >
> > Thats going to be a standard pattern - I would expect that
> > liveupdate_register_subsystem() would do the check for updated and
> > then arrange to call back something like
> > liveupdate_subsystem.ops.post_update()
> >
> > And then post_update() would get the info that is currently under
> > liveupdate_get_subsystem_data() as arguments instead of having to make
> > more functions calls.
> >
> > Maybe even the fdt_node_check_compatible() can be hoisted.
> >
> > That would remove a bunch more liveupdate_state_updated() calls.
>
> That's a good suggestion for a potential refactor. For now, the
> state-check call is inexpensive and is not in a performance-critical
> path. We can certainly implement this optimization later if it becomes
> necessary.
It is not an optimization, it is having a proper logical code
structure that doesn't rely on globals. I'm strongly against
sprinkling globals everywhere in the code when there are simple
logical APIs that entirely avoid it.
When I looked at where there were globals I didn't find any good
justifications, just thinks like this that are poor API design.
Jason
^ permalink raw reply
* Re: [PATCH 1/5] Add manpage for open_tree(2)
From: Aleksa Sarai @ 2025-08-05 16:45 UTC (permalink / raw)
To: Askar Safin
Cc: Aleksa Sarai, Christian Brauner, David Howells, Alexander Viro,
linux-api, linux-fsdevel, linux-kernel, linux-man
In-Reply-To: <20250201024322.2625842-1-safinaskar@zohomail.com>
On 2025-02-01, Askar Safin said:
> David Howells, ping! You still didn't add all these manpages.
In case you're interested, Christian Brauner maintained these man pages
in a markdown format from 2024[1]. I sat down this last weekend and
rewrote them mostly from scratch as man pages, and resubmitted them to
man-pages again today[2].
PTAL.
[1]: https://github.com/brauner/man-pages-md
[2]: https://lore.kernel.org/all/20250806-new-mount-api-v1-0-8678f56c6ee0@cyphar.com/
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox