linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Randy Dunlap <rdunlap@infradead.org>, Arnd Bergmann <arnd@arndb.de>
Cc: Aleksa Sarai <cyphar@cyphar.com>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	Jan Kara <jack@suse.cz>, Jonathan Corbet <corbet@lwn.net>,
	 Shuah Khan <shuah@kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	 linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-api@vger.kernel.org,  linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v4 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl
Date: Fri, 8 Aug 2025 16:12:19 +0200	[thread overview]
Message-ID: <20250808-huschen-jazzband-c7d1ba351773@brauner> (raw)
In-Reply-To: <1ea6f1d9-550d-4b81-bade-1a0ca14c27c6@infradead.org>

On Wed, Aug 06, 2025 at 11:57:42AM -0700, Randy Dunlap wrote:
> 
> 
> On 8/6/25 11:02 AM, Aleksa Sarai wrote:
> > On 2025-08-05, Randy Dunlap <rdunlap@infradead.org> wrote:
> >>
> >>
> >> On 8/4/25 10:45 PM, 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 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/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)
> >>
> >> Since the _IO() nr here is 32, Documentation/userspace-api/ioctl/ioctl-number.rst
> >> should be updated like:
> >>
> >> -'f'   00-0F  linux/fs.h                                                conflict!
> >> +'f'   00-1F  linux/fs.h                                                conflict!
> > 
> > Should this be 00-20 (or 00-2F) instead?
> 
> Oops, yes, it should be one of those. Thanks.
> 
> > Also, is there a better value to use for this new ioctl? I'm not quite
> > sure what is the best practice to handle these kinds of conflicts...
> 
> I wouldn't worry about it. We have *many* conflicts.
> (unless Al or Christian are concerned)

We try to minimize conflicts but we unfortunately give no strong
guarantees in any way. I always defer to Arnd in such matters as he's
got a pretty good mental model of what is best to do for ioctls.

> 
> >> (17 is already used for PROCFS_IOCTL_MAGIC somewhere else, so that probably should
> >> have update the Doc/rst file.)

  reply	other threads:[~2025-08-08 14:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-05  5:45 [PATCH v4 0/4] procfs: make reference pidns more user-visible Aleksa Sarai
2025-08-05  5:45 ` [PATCH v4 1/4] pidns: move is-ancestor logic to helper Aleksa Sarai
2025-08-05  5:45 ` [PATCH v4 2/4] procfs: add "pidns" mount option Aleksa Sarai
2025-08-05  7:29   ` Aleksa Sarai
2025-08-06 10:25     ` Askar Safin
2025-08-06 14:12       ` Aleksa Sarai
2025-08-07  7:17         ` Aleksa Sarai
2025-08-08 14:09           ` Christian Brauner
2025-08-08 15:51             ` Aleksa Sarai
2025-08-06  0:19   ` Randy Dunlap
2025-08-05  5:45 ` [PATCH v4 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl Aleksa Sarai
2025-08-06  0:25   ` Randy Dunlap
2025-08-06 18:02     ` Aleksa Sarai
2025-08-06 18:57       ` Randy Dunlap
2025-08-08 14:12         ` Christian Brauner [this message]
2025-08-05  5:45 ` [PATCH v4 4/4] selftests/proc: add tests for new pidns APIs Aleksa Sarai
2025-09-02  9:54 ` (subset) [PATCH v4 0/4] procfs: make reference pidns more user-visible Christian Brauner
2025-09-02 10:02 ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250808-huschen-jazzband-c7d1ba351773@brauner \
    --to=brauner@kernel.org \
    --cc=arnd@arndb.de \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=jack@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=rdunlap@infradead.org \
    --cc=shuah@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).