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

Li Chen posted 1 patch 1 week, 2 days ago
fs/pidfs.c                                             | 8 +++++++-
tools/testing/selftests/pidfd/pidfd_file_handle_test.c | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
[PATCH] pidfs: preserve thread pidfds reopened by file handle
Posted by Li Chen 1 week, 2 days ago
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>
---
 fs/pidfs.c                                             | 8 +++++++-
 tools/testing/selftests/pidfd/pidfd_file_handle_test.c | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/pidfs.c b/fs/pidfs.c
index aaa609ddab044..c20ffd747ff51 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -939,12 +939,18 @@ 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.
 	 */
 	oflags &= ~O_LARGEFILE;
-	return dentry_open(path, oflags | O_RDWR, current_cred());
+	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;
 }
 
 static const struct export_operations pidfs_export_operations = {
diff --git a/tools/testing/selftests/pidfd/pidfd_file_handle_test.c b/tools/testing/selftests/pidfd/pidfd_file_handle_test.c
index 68918734dcf32..1e03ae9575fe6 100644
--- a/tools/testing/selftests/pidfd/pidfd_file_handle_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_file_handle_test.c
@@ -373,6 +373,7 @@ TEST_F(file_handle, open_by_handle_at_valid_flags)
 				  O_CLOEXEC |
 				  O_EXCL);
 	ASSERT_GE(pidfd, 0);
+	ASSERT_NE(fcntl(pidfd, F_GETFL) & PIDFD_THREAD, 0);
 
 	ASSERT_EQ(fstat(pidfd, &st2), 0);
 	ASSERT_TRUE(st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
-- 
2.52.0
Re: [PATCH] pidfs: preserve thread pidfds reopened by file handle
Posted by Christian Brauner 3 days, 4 hours ago
On Thu, 16 Jul 2026 13:27:22 +0800, Li Chen wrote:
> pidfs: preserve thread pidfds reopened by file handle

The patch is correct but I'm likely massing it so a tiny shared helper
is called in both the regular creation and the file handle creation.

---

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] pidfs: preserve thread pidfds reopened by file handle
      https://git.kernel.org/vfs/vfs/c/c8d2d281934c
Re: [PATCH] pidfs: preserve thread pidfds reopened by file handle
Posted by Jan Kara 1 week, 2 days ago
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:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/pidfs.c                                             | 8 +++++++-
>  tools/testing/selftests/pidfd/pidfd_file_handle_test.c | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index aaa609ddab044..c20ffd747ff51 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -939,12 +939,18 @@ 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.
>  	 */
>  	oflags &= ~O_LARGEFILE;
> -	return dentry_open(path, oflags | O_RDWR, current_cred());
> +	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;
>  }
>  
>  static const struct export_operations pidfs_export_operations = {
> diff --git a/tools/testing/selftests/pidfd/pidfd_file_handle_test.c b/tools/testing/selftests/pidfd/pidfd_file_handle_test.c
> index 68918734dcf32..1e03ae9575fe6 100644
> --- a/tools/testing/selftests/pidfd/pidfd_file_handle_test.c
> +++ b/tools/testing/selftests/pidfd/pidfd_file_handle_test.c
> @@ -373,6 +373,7 @@ TEST_F(file_handle, open_by_handle_at_valid_flags)
>  				  O_CLOEXEC |
>  				  O_EXCL);
>  	ASSERT_GE(pidfd, 0);
> +	ASSERT_NE(fcntl(pidfd, F_GETFL) & PIDFD_THREAD, 0);
>  
>  	ASSERT_EQ(fstat(pidfd, &st2), 0);
>  	ASSERT_TRUE(st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] pidfs: preserve thread pidfds reopened by file handle
Posted by Christian Brauner 3 days, 3 hours ago
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