Re: [PATCH] pidfs: preserve thread pidfds reopened by file handle

[Christian Brauner <brauner@kernel.org>]

On 2026-07-16 13:11 +0200, Jan Kara wrote:
> On Thu 16-07-26 13:27:22, Li Chen wrote:
> > PIDFD_THREAD shares O_EXCL. do_dentry_open() clears O_EXCL after
> > pidfs_export_open() validates the flags, so open_by_handle_at()
> > silently turns a thread pidfd into a process pidfd.
> > 
> > Restore PIDFD_THREAD on the opened file, matching pidfs_alloc_file(),
> > and cover the restored flag with F_GETFL.
> > 
> > Signed-off-by: Li Chen <me@linux.beauty>
> 
> Makes sense. Just looking at this code, what about PIDFD_AUTOKILL (aka
> O_TRUNC?)? Reading the commit introducing it 07c3ef58223e it appears it was
> never meant to be directly requested by userspace so probably
> pidfs_export_open() needs to reject it. But that's unrelated issue to your
> patch so feel free to add:

Good point. But pidfs_export_open() is guarded by
pidfs_export_permission() which does:

#define VALID_FILE_HANDLE_OPEN_FLAGS \
	(O_RDONLY | O_WRONLY | O_RDWR | O_NONBLOCK | O_CLOEXEC | O_EXCL)

	if (oflags & ~(VALID_FILE_HANDLE_OPEN_FLAGS | O_LARGEFILE))
		return -EINVAL;

and so is safe. But I've added the following patch on top which also
splats if that is passed!

From 99a859485394eca8cf6271de09ae72ac3b846bda Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 22 Jul 2026 13:56:28 +0200
Subject: [PATCH] pidfs: add pidfs_dentry_open() helper

Both pidfs_alloc_file() and pidfs_export_open() need to force O_RDWR
and reapply the pidfd flags that do_dentry_open() strips. Move the
common logic into a helper.

PIDFD_AUTOKILL is now part of the restore mask in the file handle path
as well, but pidfs_export_permission() rejects O_TRUNC, so this is a
no-op there. But warn nonetheless.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/pidfs.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/fs/pidfs.c b/fs/pidfs.c
index c20ffd747ff5..695215aa2a58 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -915,6 +915,20 @@ static struct dentry *pidfs_fh_to_dentry(struct super_block *sb,
 	return path.dentry;
 }
 
+static struct file *pidfs_dentry_open(const struct path *path,
+				      unsigned int flags,
+				      const struct cred *cred)
+{
+	struct file *file;
+
+	/* pidfds are always O_RDWR. */
+	file = dentry_open(path, flags | O_RDWR, cred);
+	/* do_dentry_open() strips O_EXCL and O_TRUNC. */
+	if (!IS_ERR(file))
+		file->f_flags |= flags & (PIDFD_THREAD | PIDFD_AUTOKILL);
+	return file;
+}
+
 /*
  * Make sure that we reject any nonsensical flags that users pass via
  * open_by_handle_at(). Note that PIDFD_THREAD is defined as O_EXCL, and
@@ -939,18 +953,14 @@ static int pidfs_export_permission(struct handle_to_path_ctx *ctx,
 
 static struct file *pidfs_export_open(const struct path *path, unsigned int oflags)
 {
-	struct file *file;
-
 	/*
-	 * Clear O_LARGEFILE as open_by_handle_at() forces it and raise
-	 * O_RDWR as pidfds always are.
+	 * Opening via file handle may never raise PIDFD_AUTOKILL. That can
+	 * only be done at task creation!
 	 */
-	oflags &= ~O_LARGEFILE;
-	file = dentry_open(path, oflags | O_RDWR, current_cred());
-	/* do_dentry_open() strips O_EXCL, which encodes PIDFD_THREAD. */
-	if (!IS_ERR(file))
-		file->f_flags |= oflags & PIDFD_THREAD;
-	return file;
+	if (WARN_ON_ONCE(oflags & PIDFD_AUTOKILL))
+		return ERR_PTR(-EINVAL);
+	/* Clear O_LARGEFILE as open_by_handle_at() forces it. */
+	return pidfs_dentry_open(path, oflags & ~O_LARGEFILE, current_cred());
 }
 
 static const struct export_operations pidfs_export_operations = {
@@ -1114,7 +1124,6 @@ static struct file_system_type pidfs_type = {
 
 struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags)
 {
-	struct file *pidfd_file;
 	struct path path __free(path_put) = {};
 	int ret;
 
@@ -1132,16 +1141,7 @@ struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags)
 	VFS_WARN_ON_ONCE(!pid->attr);
 
 	flags &= ~PIDFD_STALE;
-	flags |= O_RDWR;
-	pidfd_file = dentry_open(&path, flags, current_cred());
-	/*
-	 * Raise PIDFD_THREAD and PIDFD_AUTOKILL explicitly as
-	 * do_dentry_open() strips O_EXCL and O_TRUNC.
-	 */
-	if (!IS_ERR(pidfd_file))
-		pidfd_file->f_flags |= (flags & (PIDFD_THREAD | PIDFD_AUTOKILL));
-
-	return pidfd_file;
+	return pidfs_dentry_open(&path, flags, current_cred());
 }
 
 void __init pidfs_init(void)
-- 
2.53.0