Skip to content

Commit b28ddcc

Browse files
committed
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 as pidfds used anon_inode_getfile() which aren't subject to any LSM hooks. But dentry_open() is and that would cause regressions. 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 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 try and 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. For now we continue to raise S_PRIVATE on the inode if it's a pidfs inode which means things behave exactly like before. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2265630 Link: fedora-selinux/selinux-policy#2050 Link: bus1/dbus-broker#343 [3] Link: SELinuxProject/refpolicy#762 [4] Reported-by: Nathan Chancellor <nathan@kernel.org> Link: https://lore.kernel.org/r/20240222190334.GA412503@dev-arch.thelio-3990X Link: https://lore.kernel.org/r/20240218-neufahrzeuge-brauhaus-fb0eb6459771@brauner Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent 1fa08ae commit b28ddcc

File tree

7 files changed

+59
-28
lines changed

7 files changed

+59
-28
lines changed

fs/internal.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,4 +312,5 @@ struct mnt_idmap *mnt_idmap_get(struct mnt_idmap *idmap);
312312
void mnt_idmap_put(struct mnt_idmap *idmap);
313313
int path_from_stashed(struct dentry **stashed, unsigned long ino,
314314
struct vfsmount *mnt, const struct file_operations *fops,
315-
void *data, struct path *path);
315+
const struct inode_operations *iops, void *data,
316+
struct path *path);

fs/libfs.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <linux/fsnotify.h>
2424
#include <linux/unicode.h>
2525
#include <linux/fscrypt.h>
26+
#include <linux/pidfs.h>
2627

2728
#include <linux/uaccess.h>
2829

@@ -1990,6 +1991,7 @@ static inline struct dentry *get_stashed_dentry(struct dentry *stashed)
19901991
static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
19911992
struct super_block *sb,
19921993
const struct file_operations *fops,
1994+
const struct inode_operations *iops,
19931995
void *data)
19941996
{
19951997
struct dentry *dentry;
@@ -2007,8 +2009,13 @@ static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
20072009

20082010
inode->i_ino = ino;
20092011
inode->i_flags |= S_IMMUTABLE;
2012+
if (is_pidfs_sb(sb))
2013+
inode->i_flags |= S_PRIVATE;
20102014
inode->i_mode = S_IFREG | S_IRUGO;
2011-
inode->i_fop = fops;
2015+
if (iops)
2016+
inode->i_op = iops;
2017+
if (fops)
2018+
inode->i_fop = fops;
20122019
inode->i_private = data;
20132020
simple_inode_init_ts(inode);
20142021

@@ -2030,6 +2037,7 @@ static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
20302037
* @stashed: where to retrieve or stash dentry
20312038
* @ino: inode number to use
20322039
* @mnt: mnt of the filesystems to use
2040+
* @iops: inode operations to use
20332041
* @fops: file operations to use
20342042
* @data: data to store in inode->i_private
20352043
* @path: path to create
@@ -2048,7 +2056,8 @@ static struct dentry *stash_dentry(struct dentry **stashed, unsigned long ino,
20482056
*/
20492057
int path_from_stashed(struct dentry **stashed, unsigned long ino,
20502058
struct vfsmount *mnt, const struct file_operations *fops,
2051-
void *data, struct path *path)
2059+
const struct inode_operations *iops, void *data,
2060+
struct path *path)
20522061
{
20532062
struct dentry *dentry;
20542063
int ret = 0;
@@ -2057,7 +2066,7 @@ int path_from_stashed(struct dentry **stashed, unsigned long ino,
20572066
if (dentry)
20582067
goto out_path;
20592068

2060-
dentry = stash_dentry(stashed, ino, mnt->mnt_sb, fops, data);
2069+
dentry = stash_dentry(stashed, ino, mnt->mnt_sb, fops, iops, data);
20612070
if (IS_ERR(dentry))
20622071
return PTR_ERR(dentry);
20632072
ret = 1;

fs/nsfs.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ int ns_get_path_cb(struct path *path, ns_get_path_helper_t *ns_get_cb,
6767
if (!ns)
6868
return -ENOENT;
6969
ret = path_from_stashed(&ns->stashed, ns->inum, nsfs_mnt,
70-
&ns_file_operations, ns, path);
70+
&ns_file_operations, NULL, ns, path);
7171
if (ret <= 0 && ret != -EAGAIN)
7272
ns->ops->put(ns);
7373
} while (ret == -EAGAIN);
@@ -122,8 +122,9 @@ int open_related_ns(struct ns_common *ns,
122122
return PTR_ERR(relative);
123123
}
124124

125-
err = path_from_stashed(&relative->stashed, relative->inum, nsfs_mnt,
126-
&ns_file_operations, relative, &path);
125+
err = path_from_stashed(&relative->stashed, relative->inum,
126+
nsfs_mnt, &ns_file_operations, NULL,
127+
relative, &path);
127128
if (err <= 0 && err != -EAGAIN)
128129
relative->ops->put(relative);
129130
} while (err == -EAGAIN);

fs/pidfs.c

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#include <linux/seq_file.h>
1515
#include <uapi/linux/pidfd.h>
1616

17+
#include "internal.h"
18+
1719
static int pidfd_release(struct inode *inode, struct file *file)
1820
{
1921
#ifndef CONFIG_FS_PID
@@ -186,9 +188,21 @@ static char *pidfs_dname(struct dentry *dentry, char *buffer, int buflen)
186188
d_inode(dentry)->i_ino);
187189
}
188190

191+
static void pidfs_prune_dentry(struct dentry *dentry)
192+
{
193+
struct inode *inode;
194+
195+
inode = d_inode(dentry);
196+
if (inode) {
197+
struct pid *pid = inode->i_private;
198+
WRITE_ONCE(pid->stashed, NULL);
199+
}
200+
}
201+
189202
static const struct dentry_operations pidfs_dentry_operations = {
190203
.d_delete = always_delete_dentry,
191204
.d_dname = pidfs_dname,
205+
.d_prune = pidfs_prune_dentry,
192206
};
193207

194208
static int pidfs_init_fs_context(struct fs_context *fc)
@@ -213,34 +227,28 @@ static struct file_system_type pidfs_type = {
213227
struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags)
214228
{
215229

216-
struct inode *inode;
217230
struct file *pidfd_file;
231+
struct path path;
232+
int ret;
218233

219-
inode = iget_locked(pidfs_sb, pid->ino);
220-
if (!inode)
221-
return ERR_PTR(-ENOMEM);
222-
223-
if (inode->i_state & I_NEW) {
234+
do {
224235
/*
225236
* Inode numbering for pidfs start at RESERVED_PIDS + 1.
226237
* This avoids collisions with the root inode which is 1
227238
* for pseudo filesystems.
228239
*/
229-
inode->i_ino = pid->ino;
230-
inode->i_mode = S_IFREG | S_IRUGO;
231-
inode->i_op = &pidfs_inode_operations;
232-
inode->i_fop = &pidfs_file_operations;
233-
inode->i_flags |= S_IMMUTABLE;
234-
inode->i_private = get_pid(pid);
235-
simple_inode_init_ts(inode);
236-
unlock_new_inode(inode);
237-
}
238-
239-
pidfd_file = alloc_file_pseudo(inode, pidfs_mnt, "", flags,
240-
&pidfs_file_operations);
241-
if (IS_ERR(pidfd_file))
242-
iput(inode);
243-
240+
ret = path_from_stashed(&pid->stashed, pid->ino, pidfs_mnt,
241+
&pidfs_file_operations,
242+
&pidfs_inode_operations, get_pid(pid),
243+
&path);
244+
if (ret <= 0 && ret != -EAGAIN)
245+
put_pid(pid);
246+
} while (ret == -EAGAIN);
247+
if (ret < 0)
248+
return ERR_PTR(ret);
249+
250+
pidfd_file = dentry_open(&path, flags, current_cred());
251+
path_put(&path);
244252
return pidfd_file;
245253
}
246254

@@ -253,6 +261,11 @@ void __init pidfs_init(void)
253261
pidfs_sb = pidfs_mnt->mnt_sb;
254262
}
255263

264+
bool is_pidfs_sb(const struct super_block *sb)
265+
{
266+
return sb == pidfs_mnt->mnt_sb;
267+
}
268+
256269
#else /* !CONFIG_FS_PID */
257270

258271
struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags)
@@ -269,4 +282,8 @@ struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags)
269282
}
270283

271284
void __init pidfs_init(void) { }
285+
bool is_pidfs_sb(const struct super_block *sb)
286+
{
287+
return false;
288+
}
272289
#endif

include/linux/pid.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ struct pid
5656
unsigned int level;
5757
spinlock_t lock;
5858
#ifdef CONFIG_FS_PID
59+
struct dentry *stashed;
5960
unsigned long ino;
6061
#endif
6162
/* lists of tasks that use this pid */

include/linux/pidfs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@
44

55
struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags);
66
void __init pidfs_init(void);
7+
bool is_pidfs_sb(const struct super_block *sb);
78

89
#endif /* _LINUX_PID_FS_H */

kernel/pid.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
281281
if (!(ns->pid_allocated & PIDNS_ADDING))
282282
goto out_unlock;
283283
#ifdef CONFIG_FS_PID
284+
pid->stashed = NULL;
284285
pid->ino = ++pidfs_ino;
285286
#endif
286287
for ( ; upid >= pid->numbers; --upid) {

0 commit comments

Comments
 (0)