Re: [PATCH] proc: maps protection

Kees Cook
2007-03-06T00:45:48Z

Implement the same logic for the checks done on /proc/$pid/mem, but
extend them to /proc/$pid/{maps,smaps,numa_maps}. This means that only
processes and their ptrace parents can read their maps files.

Signed-off-by: Kees Cook <kees@outflux.net>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
On Mon, Mar 05, 2007 at 12:15:11PM -0800, Kees Cook wrote:
> What do others think of this?

Whoops, Arjan caught a mistake, new patch included, using -EACCES
everywhere. (I had a straggling -EPERM from an earlier version.)

---
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1a979ea..9bf7585 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -65,8 +65,6 @@
#include <linux/rcupdate.h>
#include <linux/kallsyms.h>
#include <linux/mount.h>
-#include <linux/security.h>
-#include <linux/ptrace.h>
#include <linux/seccomp.h>
#include <linux/cpuset.h>
#include <linux/audit.h>
@@ -189,13 +187,6 @@ static int proc_root_link(struct inode *inode, struct dentry **dentry, struct vf
return result;
}

-#define MAY_PTRACE(task) \
- (task == current || \
- (task->parent == current && \
- (task->ptrace & PT_PTRACED) && \
- (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \
- security_ptrace(current,task) == 0))
-
static int proc_pid_environ(struct task_struct *task, char * buffer)
{
int res = 0;
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 987c773..3c5ccc9 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -10,6 +10,8 @@
*/

#include <linux/proc_fs.h>
+#include <linux/security.h>
+#include <linux/ptrace.h>

struct vmalloc_info {
unsigned long used;
@@ -31,6 +33,13 @@ do { \
extern int nommu_vma_show(struct seq_file *, struct vm_area_struct *);
#endif

+#define MAY_PTRACE(task) \
+ (task == current || \
+ (task->parent == current && \
+ (task->ptrace & PT_PTRACED) && \
+ (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \
+ security_ptrace(current,task) == 0))
+
extern void create_seq_entry(char *name, mode_t mode, const struct file_operations *f);
extern int proc_exe_link(struct inode *, struct dentry **, struct vfsmount **);
extern int proc_tid_stat(struct task_struct *, char *);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 55ade0d..85486d4 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -134,6 +134,9 @@ static int show_map_internal(struct seq_file *m, void *v, struct mem_size_stats
dev_t dev = 0;
int len;

+ if (!MAY_PTRACE(task) || !ptrace_may_attach(task))
+ return -EACCES;
+
if (file) {
struct inode *inode = vma->vm_file->f_path.dentry->d_inode;
dev = inode->i_sb->s_dev;
@@ -444,11 +447,22 @@ struct file_operations proc_maps_operations = {
#ifdef CONFIG_NUMA
extern int show_numa_map(struct seq_file *m, void *v);

+static int show_numa_map_checked(struct seq_file *m, void *v)
+{
+ struct proc_maps_private *priv = m->private;
+ struct task_struct *task = priv->task;
+
+ if (!MAY_PTRACE(task) || !ptrace_may_attach(task))
+ return -EACCES;
+
+ return show_numa_map(m, v);
+}
+
static struct seq_operations proc_pid_numa_maps_op = {
.start = m_start,
.next = m_next,
.stop = m_stop,
- .show = show_numa_map
+ .show = show_numa_map_checked
};

static int numa_maps_open(struct inode *inode, struct file *file)
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index fcc5caf..985a6ff 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -143,6 +143,12 @@ out:
static int show_map(struct seq_file *m, void *_vml)
{
struct vm_list_struct *vml = _vml;
+ struct proc_maps_private *priv = m->private;
+ struct task_struct *task = priv->task;
+
+ if (!MAY_PTRACE(task) || !ptrace_may_attach(task))
+ return -EACCES;
+
return nommu_vma_show(m, vml->vma);
}

--
Kees Cook