| 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
| ||
|
Date: Mon, 8 Jun 2015 20:39:33 -0700
From: Calvin Owens <calvinowens@...com>
To: Andrew Morton <akpm@...ux-foundation.org>,
Alexey Dobriyan <adobriyan@...il.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Al Viro <viro@...iv.linux.org.uk>,
Miklos Szeredi <miklos@...redi.hu>,
Zefan Li <lizefan@...wei.com>, Oleg Nesterov <oleg@...hat.com>,
Joe Perches <joe@...ches.com>,
David Howells <dhowells@...hat.com>
CC: Calvin Owens <calvinowens@...com>, <linux-kernel@...r.kernel.org>,
<kernel-team@...com>, Andy Lutomirski <luto@...capital.net>,
Cyrill Gorcunov <gorcunov@...nvz.org>,
Kees Cook <keescook@...omium.org>,
"Kirill A. Shutemov" <kirill@...temov.name>
Subject: [PATCH v6] procfs: Always expose /proc/<pid>/map_files/ and make it readable
Currently, /proc/<pid>/map_files/ is restricted to CAP_SYS_ADMIN, and
is only exposed if CONFIG_CHECKPOINT_RESTORE is set.
This interface very useful because it allows userspace to stat()
deleted files that are still mapped by some process, which enables a
much quicker and more accurate answer to the question "How much disk
space is being consumed by files that are deleted but still mapped?"
than is currently possible.
This patch moves map_files/ out from behind CONFIG_CHECKPOINT_RESTORE,
and adjusts the permissions enforced on it as follows:
* proc_map_files_lookup()
* proc_map_files_readdir()
* map_files_d_revalidate()
Remove the CAP_SYS_ADMIN restriction, leaving only the current
restriction requiring PTRACE_MODE_READ.
In earlier versions of this patch, I changed the ptrace checks
in the functions above to enforce MODE_ATTACH instead of
MODE_READ. That was an oversight: all the information exposed
by the above three functions is already available with
MODE_READ from /proc/PID/maps. I was only being asked to
strengthen the protection around functionality provided by
follow_link(), not the above.
So, I've left the checks for MODE_READ as-is, since AFAICS all
objections raised so far are addressed by the new CAP_SYS_ADMIN
check in follow_link(), explained below.
* proc_map_files_follow_link()
This stub has been added, and requires that the user have
CAP_SYS_ADMIN in order to follow the links in map_files/,
since there was concern on LKML both about the potential for
bypassing permissions on ancestor directories in the path to
files pointed to, and about what happens with more exotic
memory mappings created by some drivers (ie dma-buf).
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Andy Lutomirski <luto@...capital.net>
Cc: Cyrill Gorcunov <gorcunov@...nvz.org>
Cc: Joe Perches <joe@...ches.com>
Cc: Kees Cook <keescook@...omium.org>
Cc: Kirill A. Shutemov <kirill@...temov.name>
Signed-off-by: Calvin Owens <calvinowens@...com>
---
Changes in v6: Require CAP_SYS_ADMIN for follow_link(). Leave other
PTRACE_MODE_READ checks as-is, since CAP_SYS_ADMIN
alone addresses all concerns raised AFAICS.
Changes in v5: s/dentry->d_inode/d_inode(dentry)/g
Changes in v4: Return -ESRCH from follow_link() when get_proc_task()
returns NULL.
Changes in v3: Changed permission checks to use PTRACE_MODE_ATTACH
instead of PTRACE_MODE_READ, and added a stub to
enforce MODE_ATTACH on follow_link() as well.
Changes in v2: Removed the follow_link() stub that returned -EPERM if
the caller didn't have CAP_SYS_ADMIN, since the caller
in my chroot() scenario gets -EACCES anyway.
fs/proc/base.c | 42 +++++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 093ca14..0270191 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1641,8 +1641,6 @@ end_instantiate:
return dir_emit(ctx, name, len, 1, DT_UNKNOWN);
}
-#ifdef CONFIG_CHECKPOINT_RESTORE
-
/*
* dname_to_vma_addr - maps a dentry name into two unsigned longs
* which represent vma start and end addresses.
@@ -1669,11 +1667,6 @@ static int map_files_d_revalidate(struct dentry *dentry, unsigned int flags)
if (flags & LOOKUP_RCU)
return -ECHILD;
- if (!capable(CAP_SYS_ADMIN)) {
- status = -EPERM;
- goto out_notask;
- }
-
inode = d_inode(dentry);
task = get_proc_task(inode);
if (!task)
@@ -1762,6 +1755,28 @@ struct map_files_info {
unsigned char name[4*sizeof(long)+2]; /* max: %lx-%lx\0 */
};
+/*
+ * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the
+ * symlinks may be used to bypass permissions on ancestor directories in the
+ * path to the file in question.
+ */
+static void *proc_map_files_follow_link(struct dentry *dentry, struct nameidata *nd)
+{
+ if (!capable(CAP_SYS_ADMIN))
+ return ERR_PTR(-EPERM);
+
+ return proc_pid_follow_link(dentry, nd);
+}
+
+/*
+ * Identical to proc_pid_link_inode_operations except for follow_link()
+ */
+static const struct inode_operations proc_map_files_link_inode_operations = {
+ .readlink = proc_pid_readlink,
+ .follow_link = proc_map_files_follow_link,
+ .setattr = proc_setattr,
+};
+
static int
proc_map_files_instantiate(struct inode *dir, struct dentry *dentry,
struct task_struct *task, const void *ptr)
@@ -1777,7 +1792,7 @@ proc_map_files_instantiate(struct inode *dir, struct dentry *dentry,
ei = PROC_I(inode);
ei->op.proc_get_link = proc_map_files_get_link;
- inode->i_op = &proc_pid_link_inode_operations;
+ inode->i_op = &proc_map_files_link_inode_operations;
inode->i_size = 64;
inode->i_mode = S_IFLNK;
@@ -1801,10 +1816,6 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,
int result;
struct mm_struct *mm;
- result = -EPERM;
- if (!capable(CAP_SYS_ADMIN))
- goto out;
-
result = -ENOENT;
task = get_proc_task(dir);
if (!task)
@@ -1858,10 +1869,6 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
struct map_files_info *p;
int ret;
- ret = -EPERM;
- if (!capable(CAP_SYS_ADMIN))
- goto out;
-
ret = -ENOENT;
task = get_proc_task(file_inode(file));
if (!task)
@@ -2050,7 +2057,6 @@ static const struct file_operations proc_timers_operations = {
.llseek = seq_lseek,
.release = seq_release_private,
};
-#endif /* CONFIG_CHECKPOINT_RESTORE */
static int proc_pident_instantiate(struct inode *dir,
struct dentry *dentry, struct task_struct *task, const void *ptr)
@@ -2549,9 +2555,7 @@ static const struct inode_operations proc_task_inode_operations;
static const struct pid_entry tgid_base_stuff[] = {
DIR("task", S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations),
DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
-#ifdef CONFIG_CHECKPOINT_RESTORE
DIR("map_files", S_IRUSR|S_IXUSR, proc_map_files_inode_operations, proc_map_files_operations),
-#endif
DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
DIR("ns", S_IRUSR|S_IXUGO, proc_ns_dir_inode_operations, proc_ns_dir_operations),
#ifdef CONFIG_NET
--
1.8.1
--
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/
Powered by blists - more mailing lists