linux-kernel - [RFC PATCH 1/2] proc: force dcache drop on unauthorized access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 10 Sep 2011 20:40:01 +0400
From:	Vasiliy Kulikov <segoon@...nwall.com>
To:	kernel-hardening@...ts.openwall.com,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Cyrill Gorcunov <gorcunov@...il.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Pekka Enberg <penberg@...nel.org>,
	Matt Mackall <mpm@...enic.com>, linux-kernel@...r.kernel.org
Subject: [RFC PATCH 1/2] proc: force dcache drop on unauthorized access

The patch "proc: fix races against execve() of /proc/PID/fd**" is still
a partial fix for a setxid problem.  link(2) is a yet another way to
identify whether a specific fd is opened by a privileged process.  By
calling link(2) against /proc/PID/fd/* an attacker may identify whether
the fd number is valid for PID by analysing link(2) return code.

Both getattr() and link() can be used by the attacker iff the dentry is
present in the dcache.  In this case ->lookup() is not called and the
only way to check ptrace permissions is either operation handler or
->revalidate().  The easiest solution to prevent any unauthorized access
to /proc/PID/fd*/ files is to force the dentry drop on each unauthorized
access attempt.

If an attacker keeps opened fd of /proc/PID/fd/ and dcache contains
a specific dentry for some /proc/PID/fd/XXX, any future attemp to use the
dentry by the attacker would lead to the dentry drop as a result of a
failed ptrace check in ->revalidate().  Then the attacker cannot spawn a
dentry for the specific fd number because of ptrace check in ->lookup().

The dentry drop can be still observed by an attacker by analysing
information from /proc/slabinfo, which is addressed in the successive
patch.

Signed-off-by: Vasiliy Kulikov <segoon@...nwall.com>
---
 fs/proc/base.c |   42 ++++++------------------------------------
 1 files changed, 6 insertions(+), 36 deletions(-)

--
diff --git a/fs/proc/base.c b/fs/proc/base.c
index d44c701..8b72293 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1665,46 +1665,12 @@ out:
 	return error;
 }
 
-static int proc_pid_fd_link_getattr(struct vfsmount *mnt, struct dentry *dentry,
-		struct kstat *stat)
-{
-	struct inode *inode = dentry->d_inode;
-	struct task_struct *task = get_proc_task(inode);
-	int rc;
-
-	if (task == NULL)
-		return -ESRCH;
-
-	rc = -EACCES;
-	if (lock_trace(task))
-		goto out_task;
-
-	generic_fillattr(inode, stat);
-	unlock_trace(task);
-	rc = 0;
-out_task:
-	put_task_struct(task);
-	return rc;
-}
-
 static const struct inode_operations proc_pid_link_inode_operations = {
 	.readlink	= proc_pid_readlink,
 	.follow_link	= proc_pid_follow_link,
 	.setattr	= proc_setattr,
 };
 
-static const struct inode_operations proc_fdinfo_link_inode_operations = {
-	.setattr	= proc_setattr,
-	.getattr	= proc_pid_fd_link_getattr,
-};
-
-static const struct inode_operations proc_fd_link_inode_operations = {
-	.readlink	= proc_pid_readlink,
-	.follow_link	= proc_pid_follow_link,
-	.setattr	= proc_setattr,
-	.getattr	= proc_pid_fd_link_getattr,
-};
-
 
 /* building an inode */
 
@@ -2013,6 +1979,11 @@ static int tid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
 	task = get_proc_task(inode);
 	fd = proc_fd(inode);
 
+	if (!ptrace_may_access(task, PTRACE_MODE_READ)) {
+		put_task_struct(task);
+		task = NULL;
+	}
+
 	if (task) {
 		files = get_files_struct(task);
 		if (files) {
@@ -2085,7 +2056,7 @@ static struct dentry *proc_fd_instantiate(struct inode *dir,
 	spin_unlock(&files->file_lock);
 	put_files_struct(files);
 
-	inode->i_op = &proc_fd_link_inode_operations;
+	inode->i_op = &proc_pid_link_inode_operations;
 	inode->i_size = 64;
 	ei->op.proc_get_link = proc_fd_link;
 	d_set_d_op(dentry, &tid_fd_dentry_operations);
@@ -2267,7 +2238,6 @@ static struct dentry *proc_fdinfo_instantiate(struct inode *dir,
 	ei->fd = fd;
 	inode->i_mode = S_IFREG | S_IRUSR;
 	inode->i_fop = &proc_fdinfo_file_operations;
-	inode->i_op = &proc_fdinfo_link_inode_operations;
 	d_set_d_op(dentry, &tid_fd_dentry_operations);
 	d_add(dentry, inode);
 	/* Close the race of the process dying before we return the dentry */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/