pidfs: convert to path_from_stashed() helper

Moving pidfds from the anonymous inode infrastructure to a separate tiny
in-kernel filesystem similar to sockfs, pipefs, and anon_inodefs causes
Selinux denials and thus various userspace components that make heavy
use of pidfds to fail if we would be using dentry_open() directly.
pidfds used anon_inode_getfile() which never was subject to any LSM
hooks. But dentry_open() is and that would cause regressions:

Feb 23 12:09:58 fed1 audit[353]: AVC avc:  denied  { read write open } for  pid=353 comm="systemd-userdbd" path="pidfd:[709]" dev="pidfs" ino=709 scontext=system_u:system_r:systemd_userdbd_t:>

The failures that are seen are selinux denials. But the core failure is
dbus-broker. That cascades into other services failing that depend on
dbus-broker. For example, when dbus-broker fails to start polkit and all
the others won't be able to work because they depend on dbus-broker.

The reason for dbus-broker failing is because it doesn't handle failures
for SO_PEERPIDFD correctly. Last kernel release (either v6.7 or v6.6,
I'm not completely sure right now) we introduced SO_PEERPIDFD (and
SCM_PIDFD). SO_PEERPIDFD allows dbus-broker and polkit and others to
receive a pidfd for the peer of an AF_UNIX socket. This is the first
time in the history of Linux that we can safely authenticate clients in
a race-free manner.

dbus-broker immediately made use of this but messed up the error
checking. It only allowed EINVAL as a valid failure for SO_PEERPIDFD.
That's obviously problematic not just because of LSM denials but because
of seccomp denials that would prevent SO_PEERPIDFD from working; or any
other new error code from there.

So this is catching a flawed implementation in dbus-broker as well. It
has to fallback to the old pid-based authentication when SO_PEERPIDFD
doesn't work no matter the reasons otherwise it'll always risk such
failures. So overall that LSM denial should not have caused dbus-broker
to fail. It can never assume that a feature released one kernel ago like
SO_PEERPIDFD can be assumed to be available.

So, the next fix separate from the selinux policy update is to fix
dbus-broker at [3]. That should make it into Fedora as well. In addition
the selinux reference policy should also be updated. See [4] for that.
If Selinux is in enforcing mode in userspace and it encounters anything
that it doesn't know about it will deny it by default. And the policy is
entirely in userspace including declaring new types for stuff like nsfs
or pidfs to allow it. There's just nothing to do in the kernel so we
can't use dentry_open(). So instead we use alloc_file(). Once selinux is
ready we can switch to dentry_open() or we introduce separate LSM hook
for pidfds.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=2265630 [1]
Link: fedora-selinux/selinux-policy#2050 [2]
Link: bus1/dbus-broker#343 [3]
Link: SELinuxProject/refpolicy#762 [4]
Reported-by: Nathan Chancellor <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/20240218-neufahrzeuge-brauhaus-fb0eb6459771@brauner
Signed-off-by: Christian Brauner <[email protected]>

fs/file_table.c

@@ -282,8 +282,8 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
  * @flags: O_... flags with which the new file will be opened
  * @fop: the 'struct file_operations' for the new file
  */
-static struct file *alloc_file(const struct path *path, int flags,
-		const struct file_operations *fop)
+struct file *alloc_file(const struct path *path, int flags,
+			const struct file_operations *fop)
 {
 	struct file *file;
 

fs/internal.h

@@ -312,4 +312,7 @@ struct mnt_idmap *mnt_idmap_get(struct mnt_idmap *idmap);
 void mnt_idmap_put(struct mnt_idmap *idmap);
 int path_from_stashed(struct dentry **stashed, unsigned long ino,
 		      struct vfsmount *mnt, const struct file_operations *fops,
-		      void *data, struct path *path);
+		      const struct inode_operations *iops, void *data,
+		      struct path *path);
+struct file *alloc_file(const struct path *path, int flags,
+			const struct file_operations *fop);

fs/libfs.c

@@ -1990,6 +1990,7 @@ static inline struct dentry *get_stashed_dentry(struct dentry *stashed)
 static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
 				   struct super_block *sb,
 				   const struct file_operations *fops,
+				   const struct inode_operations *iops,
 				   void *data)
 {
 	struct dentry *dentry;
@@ -2008,7 +2009,10 @@ static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
 	inode->i_ino = ino;
 	inode->i_flags |= S_IMMUTABLE;
 	inode->i_mode = S_IFREG | S_IRUGO;
-	inode->i_fop = fops;
+	if (iops)
+		inode->i_op = iops;
+	if (fops)
+		inode->i_fop = fops;
 	inode->i_private = data;
 	simple_inode_init_ts(inode);
 
@@ -2030,6 +2034,7 @@ static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
  * @stashed:    where to retrieve or stash dentry
  * @ino:        inode number to use
  * @mnt:        mnt of the filesystems to use
+ * @iops:       inode operations to use
  * @fops:       file operations to use
  * @data:       data to store in inode->i_private
  * @path:       path to create
@@ -2048,7 +2053,8 @@ static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
  */
 int path_from_stashed(struct dentry **stashed, unsigned long ino,
 		      struct vfsmount *mnt, const struct file_operations *fops,
-		      void *data, struct path *path)
+		      const struct inode_operations *iops, void *data,
+		      struct path *path)
 {
 	struct dentry *dentry;
 	int ret = 0;
@@ -2057,7 +2063,7 @@ int path_from_stashed(struct dentry **stashed, unsigned long ino,
 	if (dentry)
 		goto out_path;
 
-	dentry = stash_dentry(stashed, ino, mnt->mnt_sb, fops, data);
+	dentry = stash_dentry(stashed, ino, mnt->mnt_sb, fops, iops, data);
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
 	ret = 1;

fs/nsfs.c

@@ -67,7 +67,7 @@ int ns_get_path_cb(struct path *path, ns_get_path_helper_t *ns_get_cb,
 		if (!ns)
 			return -ENOENT;
 		ret = path_from_stashed(&ns->stashed, ns->inum, nsfs_mnt,
-					&ns_file_operations, ns, path);
+					&ns_file_operations, NULL, ns, path);
 		if (ret <= 0 && ret != -EAGAIN)
 			ns->ops->put(ns);
 	} while (ret == -EAGAIN);
@@ -122,8 +122,9 @@ int open_related_ns(struct ns_common *ns,
 			return PTR_ERR(relative);
 		}
 
-		err = path_from_stashed(&relative->stashed, relative->inum, nsfs_mnt,
-					&ns_file_operations, relative, &path);
+		err = path_from_stashed(&relative->stashed, relative->inum,
+					nsfs_mnt, &ns_file_operations, NULL,
+					relative, &path);
 		if (err <= 0 && err != -EAGAIN)
 			relative->ops->put(relative);
 	} while (err == -EAGAIN);

fs/pidfs.c

@@ -14,6 +14,8 @@
 #include <linux/seq_file.h>
 #include <uapi/linux/pidfd.h>
 
+#include "internal.h"
+
 static int pidfd_release(struct inode *inode, struct file *file)
 {
 #ifndef CONFIG_FS_PID
@@ -186,9 +188,21 @@ static char *pidfs_dname(struct dentry *dentry, char *buffer, int buflen)
 			     d_inode(dentry)->i_ino);
 }
 
+static void pidfdfs_prune_dentry(struct dentry *dentry)
+{
+	struct inode *inode;
+
+	inode = d_inode(dentry);
+	if (inode) {
+		struct pid *pid = inode->i_private;
+		WRITE_ONCE(pid->stashed, NULL);
+	}
+}
+
 static const struct dentry_operations pidfs_dentry_operations = {
 	.d_delete	= always_delete_dentry,
 	.d_dname	= pidfs_dname,
+	.d_prune	= pidfdfs_prune_dentry,
 };
 
 static int pidfs_init_fs_context(struct fs_context *fc)
@@ -213,29 +227,24 @@ static struct file_system_type pidfs_type = {
 struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags)
 {
 
-	struct inode *inode;
 	struct file *pidfd_file;
-
-	inode = iget_locked(pidfs_sb, pid->ino);
-	if (!inode)
-		return ERR_PTR(-ENOMEM);
-
-	if (inode->i_state & I_NEW) {
-		inode->i_ino = pid->ino;
-		inode->i_mode = S_IFREG | S_IRUGO;
-		inode->i_op = &pidfs_inode_operations;
-		inode->i_fop = &pidfs_file_operations;
-		inode->i_flags |= S_IMMUTABLE;
-		inode->i_private = get_pid(pid);
-		simple_inode_init_ts(inode);
-		unlock_new_inode(inode);
-	}
-
-	pidfd_file = alloc_file_pseudo(inode, pidfs_mnt, "", flags,
-				       &pidfs_file_operations);
+	struct path path;
+	int ret;
+
+	do {
+		ret = path_from_stashed(&pid->stashed, pid->ino, pidfs_mnt,
+					&pidfs_file_operations,
+					&pidfs_inode_operations, get_pid(pid),
+					&path);
+		if (ret <= 0 && ret != -EAGAIN)
+			put_pid(pid);
+	} while (ret == -EAGAIN);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	pidfd_file = alloc_file(&path, flags, &pidfs_file_operations);
 	if (IS_ERR(pidfd_file))
-		iput(inode);
-
+		path_put(&path);
 	return pidfd_file;
 }
 

include/linux/pid.h

@@ -56,6 +56,7 @@ struct pid
 	unsigned int level;
 	spinlock_t lock;
 #ifdef CONFIG_FS_PID
+	struct dentry *stashed;
 	unsigned long ino;
 #endif
 	/* lists of tasks that use this pid */

kernel/pid.c

@@ -277,6 +277,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
 	if (!(ns->pid_allocated & PIDNS_ADDING))
 		goto out_unlock;
 #ifdef CONFIG_FS_PID
+	pid->stashed = NULL;
 	pid->ino = ++pidfs_ino;
 #endif
 	for ( ; upid >= pid->numbers; --upid) {