Unverified Commit 0a960ba4 authored by Tyler Hicks (Microsoft)'s avatar Tyler Hicks (Microsoft) Committed by Christian Brauner
Browse files

proc: Move fdinfo PTRACE_MODE_READ check into the inode .permission operation



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 7bc3fa01 ("procfs: allow reading fdinfo with PTRACE_MODE_READ")
 - commit 1927e498 ("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: default avatarKevin Parsons (Microsoft) <parsonskev@gmail.com>
Link: https://github.com/dotnet/runtime/commit/89e5469ac591b82d38510fe7de98346cce74ad4f
Link: https://stackoverflow.com/questions/75379065/start-self-contained-net6-build-exe-as-service-on-raspbian-system-unauthorizeda


Fixes: 7bc3fa01 ("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: default avatarTyler Hicks (Microsoft) <code@tyhicks.com>
Link: https://lore.kernel.org/r/20240501005646.745089-1-code@tyhicks.com


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 55394d29
Loading
Loading
Loading
Loading
+20 −22
Original line number Diff line number Diff line
@@ -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,