From: "Tyler Hicks (Microsoft)" code@tyhicks.com
The following commits loosened the permissions of /proc/<PID>/fdinfo/ directory, as well as the files within it, from 0500 to 0555 while also introducing a PTRACE_MODE_READ check between the current task and <PID>'s task:
- commit 7bc3fa0172a4 ("procfs: allow reading fdinfo with PTRACE_MODE_READ") - commit 1927e498aee1 ("procfs: prevent unprivileged processes accessing fdinfo dir")
Before those changes, inode based system calls like inotify_add_watch(2) would fail when the current task didn't have sufficient read permissions:
[...] lstat("/proc/1/task/1/fdinfo", {st_mode=S_IFDIR|0500, st_size=0, ...}) = 0 inotify_add_watch(64, "/proc/1/task/1/fdinfo", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE| IN_ONLYDIR|IN_DONT_FOLLOW|IN_EXCL_UNLINK) = -1 EACCES (Permission denied) [...]
This matches the documented behavior in the inotify_add_watch(2) man page:
ERRORS EACCES Read access to the given file is not permitted.
After those changes, inotify_add_watch(2) started succeeding despite the current task not having PTRACE_MODE_READ privileges on the target task:
[...] lstat("/proc/1/task/1/fdinfo", {st_mode=S_IFDIR|0555, st_size=0, ...}) = 0 inotify_add_watch(64, "/proc/1/task/1/fdinfo", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE| IN_ONLYDIR|IN_DONT_FOLLOW|IN_EXCL_UNLINK) = 1757 openat(AT_FDCWD, "/proc/1/task/1/fdinfo", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 EACCES (Permission denied) [...]
This change in behavior broke .NET prior to v7. See the github link below for the v7 commit that inadvertently/quietly (?) fixed .NET after the kernel changes mentioned above.
Return to the old behavior by moving the PTRACE_MODE_READ check out of the file .open operation and into the inode .permission operation:
[...] lstat("/proc/1/task/1/fdinfo", {st_mode=S_IFDIR|0555, st_size=0, ...}) = 0 inotify_add_watch(64, "/proc/1/task/1/fdinfo", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE| IN_ONLYDIR|IN_DONT_FOLLOW|IN_EXCL_UNLINK) = -1 EACCES (Permission denied) [...]
Reported-by: Kevin Parsons (Microsoft) parsonskev@gmail.com Link: https://github.com/dotnet/runtime/commit/89e5469ac591b82d38510fe7de98346cce7... Link: https://stackoverflow.com/questions/75379065/start-self-contained-net6-build... Fixes: 7bc3fa0172a4 ("procfs: allow reading fdinfo with PTRACE_MODE_READ") Cc: stable@vger.kernel.org Cc: Christian Brauner brauner@kernel.org Cc: "Christian König" christian.koenig@amd.com Cc: Jann Horn jannh@google.com Cc: Kalesh Singh kaleshsingh@google.com Cc: Hardik Garg hargar@linux.microsoft.com Cc: Allen Pais apais@linux.microsoft.com Signed-off-by: Tyler Hicks (Microsoft) code@tyhicks.com --- fs/proc/fd.c | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/fs/proc/fd.c b/fs/proc/fd.c index 6e72e5ad42bc..f4b1c8b42a51 100644 --- a/fs/proc/fd.c +++ b/fs/proc/fd.c @@ -74,7 +74,18 @@ static int seq_show(struct seq_file *m, void *v) return 0; }
-static int proc_fdinfo_access_allowed(struct inode *inode) +static int seq_fdinfo_open(struct inode *inode, struct file *file) +{ + return single_open(file, seq_show, inode); +} + +/** + * Shared /proc/pid/fdinfo and /proc/pid/fdinfo/fd permission helper to ensure + * that the current task has PTRACE_MODE_READ in addition to the normal + * POSIX-like checks. + */ +static int proc_fdinfo_permission(struct mnt_idmap *idmap, struct inode *inode, + int mask) { bool allowed = false; struct task_struct *task = get_proc_task(inode); @@ -88,18 +99,13 @@ static int proc_fdinfo_access_allowed(struct inode *inode) if (!allowed) return -EACCES;
- return 0; + return generic_permission(idmap, inode, mask); }
-static int seq_fdinfo_open(struct inode *inode, struct file *file) -{ - int ret = proc_fdinfo_access_allowed(inode); - - if (ret) - return ret; - - return single_open(file, seq_show, inode); -} +static const struct inode_operations proc_fdinfo_file_inode_operations = { + .permission = proc_fdinfo_permission, + .setattr = proc_setattr, +};
static const struct file_operations proc_fdinfo_file_operations = { .open = seq_fdinfo_open, @@ -388,6 +394,8 @@ static struct dentry *proc_fdinfo_instantiate(struct dentry *dentry, ei = PROC_I(inode); ei->fd = data->fd;
+ inode->i_op = &proc_fdinfo_file_inode_operations; + inode->i_fop = &proc_fdinfo_file_operations; tid_fd_update_inode(task, inode, 0);
@@ -407,23 +415,13 @@ static int proc_readfdinfo(struct file *file, struct dir_context *ctx) proc_fdinfo_instantiate); }
-static int proc_open_fdinfo(struct inode *inode, struct file *file) -{ - int ret = proc_fdinfo_access_allowed(inode); - - if (ret) - return ret; - - return 0; -} - const struct inode_operations proc_fdinfo_inode_operations = { .lookup = proc_lookupfdinfo, + .permission = proc_fdinfo_permission, .setattr = proc_setattr, };
const struct file_operations proc_fdinfo_operations = { - .open = proc_open_fdinfo, .read = generic_read_dir, .iterate_shared = proc_readfdinfo, .llseek = generic_file_llseek,
On Tue, 30 Apr 2024 19:56:46 -0500, Tyler Hicks wrote:
The following commits loosened the permissions of /proc/<PID>/fdinfo/ directory, as well as the files within it, from 0500 to 0555 while also introducing a PTRACE_MODE_READ check between the current task and <PID>'s task:
- commit 7bc3fa0172a4 ("procfs: allow reading fdinfo with PTRACE_MODE_READ")
- commit 1927e498aee1 ("procfs: prevent unprivileged processes accessing fdinfo dir")
[...]
Hm, a bit unfortunate that this will mean we risk regressions by fixing a regression. But this looks sane to me and having a permission handler for fdinfo does seem like the more natural approach instead of doing the permission check at open time.
---
Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc 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.misc
[1/1] proc: Move fdinfo PTRACE_MODE_READ check into the inode .permission operation https://git.kernel.org/vfs/vfs/c/0a960ba49869
linux-stable-mirror@lists.linaro.org