Skip to contentSkip to conversation

sysctl: control functionality of /proc/pid/mem

PATCH Superseded Kees Cook 2 participants 3 messages Jan 21, 2012

Subsystems (3)
Documentation Proc filesystem Proc sysctl

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 sysctl: control functionality of /proc/pid/mem

Add the "proc_pid_mem" sysctl to control whether or not /proc/pid/mem is allowed to work: 0: disabled, 1: read only, 2: read/write.

Signed-off-by Kees Cook <[email protected]>
---
 Documentation/sysctl/kernel.txt |   14 ++++++++++++++
 fs/proc/base.c                  |   11 +++++++++++
 kernel/sysctl.c                 |   14 ++++++++++++++
 3 files changed, 39 insertions(+), 0 deletions(-)
Documentation/sysctl/kernel.txt +14 -0
index 8c20fbd..6d52dba 100644
@@ -56,6 +56,7 @@ show up in /proc/sys/kernel:
 - printk_delay
 - printk_ratelimit
 - printk_ratelimit_burst
+- proc_pid_mem
 - randomize_va_space
 - real-root-dev               ==> Documentation/initrd.txt
 - reboot-cmd                  [ SPARC only ]

@@ -477,6 +478,19 @@ send before ratelimiting kicks in.
 
 ==============================================================
 
+proc_pid_mem:
+
+This option can be used to select the level of access given to potential
+ptracers when using the per-process "mem" file in /proc/pid/mem.
+
+0 - Disable entirely.
+
+1 - Allow potential ptracers read access to process memory, but not writes.
+
+2 - Allow potential ptracers read and write access to process memory.
+
+==============================================================
+
 randomize_va_space:
 
 This option can be used to select the type of process address
fs/proc/base.c +11 -0
index 9cde9ed..35f4651 100644
@@ -109,6 +109,8 @@ struct pid_entry {
 	union proc_op op;
 };
 
+int sysctl_proc_pid_mem = 2;
+
 #define NOD(NAME, MODE, IOP, FOP, OP) {			\
 	.name = (NAME),					\
 	.len  = sizeof(NAME) - 1,			\

@@ -702,6 +704,9 @@ static int mem_open(struct inode* inode, struct file* file)
 	struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode);
 	struct mm_struct *mm;
 
+	if (sysctl_proc_pid_mem < 1)
+		return -EACCES;
+
 	if (!task)
 		return -ESRCH;
 

@@ -726,6 +731,9 @@ static ssize_t mem_read(struct file * file, char __user * buf,
 	unsigned long src = *ppos;
 	struct mm_struct *mm = file->private_data;
 
+	if (sysctl_proc_pid_mem < 1)
+		return -EACCES;
+
 	if (!mm)
 		return 0;
 

@@ -770,6 +778,9 @@ static ssize_t mem_write(struct file * file, const char __user *buf,
 	unsigned long dst = *ppos;
 	struct mm_struct *mm = file->private_data;
 
+	if (sysctl_proc_pid_mem < 2)
+		return -EACCES;
+
 	if (!mm)
 		return 0;
kernel/sysctl.c +14 -0
index f487f25..dda911f 100644
@@ -109,6 +109,9 @@ extern int sysctl_nr_trim_pages;
 #ifdef CONFIG_BLOCK
 extern int blk_iopoll_enabled;
 #endif
+#ifdef CONFIG_PROC_FS
+extern int sysctl_proc_pid_mem;
+#endif
 
 /* Constants used for minimum and  maximum */
 #ifdef CONFIG_LOCKUP_DETECTOR

@@ -1004,6 +1007,17 @@ static struct ctl_table kern_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 #endif
+#ifdef CONFIG_PROC_FS
+	{
+		.procname	= "proc_pid_mem",
+		.data		= &sysctl_proc_pid_mem,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &two,
+	},
+#endif
 	{ }
 };
Signature · 1 line
1.7.0.4
#1 lore raw

fs/proc/base.c static int mem_open(struct inode* inode, struct file* file) · line 708

Quoted 6 lines · Kees Cook
fs/proc/base.c +2 -0
@@ -702,6 +704,9 @@ static int mem_open(struct inode* inode, struct file* file)
 	struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode);
 	struct mm_struct *mm;
 
+	if (sysctl_proc_pid_mem < 1)
+		return -EACCES;

This looks like an unprivileged user-space triggerable memory leak DoS. The task reference is already acquired at that point.

See how easy it is to introduce bugs with potential security side-effects, even when the intention is to increase security? :-)

( Btw., this is why i generally prefer 'invariant' local
variable definition blocks that have no serious side effects - it makes the flow more obvious. Local variables slip off the eye rather often and don't register as 'real' code.)

Thanks,

Ingo

author
#2 lore raw
Quoted 11 lines · Ingo Molnar

fs/proc/base.c static int mem_open(struct inode* inode, struct file* file) · line 708

Quoted 6 lines · Kees Cook
fs/proc/base.c +2 -0
@@ -702,6 +704,9 @@ static int mem_open(struct inode* inode, struct file* file)
       struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode);
       struct mm_struct *mm;
 
+     if (sysctl_proc_pid_mem < 1)
+             return -EACCES;

This looks like an unprivileged user-space triggerable memory leak DoS. The task reference is already acquired at that point.

Ah, good catch; thanks. I'll respin.

-Kees

Signature · 2 lines
Kees Cook
ChromeOS Security