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

[Li Chen]

Hi Jan & Christian,

 ---- On Mon, 27 Jul 2026 23:57:14 +0800  Jan Kara <jack@xxxxxxx> wrote --- 
 > On Wed 22-07-26 14:07:37, Christian Brauner wrote:
 > > 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@xxxxxxxxxx>
 > > 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@xxxxxxxxxx>
 > 
 > Nice! Feel free to add:
 > 
 > Reviewed-by: Jan Kara <jack@xxxxxxx>
 > 

  > 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.

Thanks both.

Jan, thanks for pointing out the PIDFD_AUTOKILL angle.

Christian, thanks for factoring this into pidfs_dentry_open() and
applying the fix.

Regards,
Li​