/proc/pid/mem and mem_write

["Charles P. Wright" <cwright@cs.sunysb.edu>]

The /proc/pid/mem file provides a much more convenient and efficient
interface for accesses a process's memory than PTRACE_PEEKDATA using
read.  In the past, it also provided write access, but this was (most
recently disabled in 2.4.0-test10) with the vague comment that it is
a "security hazard."

After digging a little bit deeper, it was disabled in the 2.2 kernel
series (with a more descriptive comment):
/*
 * mem_write isn't really a good idea right now. It needs
 * to check a lot more: if the process we try to write to
 * dies in the middle right now, mem_write will overwrite
 * kernel memory.. This disables it altogether.
 */

In Linux 2.3.27 it mem_write was rewritten (to be close to its current
form) and enabled again.  Then in 2.4.0-test10 it was disabled, and with
the comment "/* This is a security hazard */".  I tried searching
mailing list archives to see what the reason for this comment is, but
couldn't find anything.

The functionality provided is no different than what PTRACE_POKEDATA
does, so the problems must be implementation related.  The version of
the function that is in the latest kernels has clearly atrophied, but if
the mem_read function is secure, with updates the mem_write function
should be secure as well.

The problem that was originally described has since been solved in
mem_read (by adding a get_mm), but not in mem_write.  The following
patch updates mem_write to be similar to the mem_read function, and
removes the #define that comments it out.

Charles

Signed-off-by: Charles P. Wright <cwright@cs.sunysb.edu>
--- linux-2.6.15-vanilla/fs/proc/base.c	2006-01-02 22:21:10.000000000 -0500
+++ linux-2.6.15-mem_write/fs/proc/base.c	2006-03-10 14:40:22.000000000 -0500
@@ -825,49 +825,67 @@
 	return ret;
 }
 
-#define mem_write NULL
-
-#ifndef mem_write
-/* This is a security hazard */
 static ssize_t mem_write(struct file * file, const char * buf,
 			 size_t count, loff_t *ppos)
 {
-	int copied = 0;
-	char *page;
 	struct task_struct *task = proc_task(file->f_dentry->d_inode);
+	char *page;
 	unsigned long dst = *ppos;
+	int ret = -ESRCH;
+	struct mm_struct *mm;
 
 	if (!MAY_PTRACE(task) || !ptrace_may_attach(task))
-		return -ESRCH;
+		goto out;
 
+	ret = -ENOMEM;
 	page = (char *)__get_free_page(GFP_USER);
 	if (!page)
-		return -ENOMEM;
+		goto out;
+
+	ret = 0;
+
+	mm = get_task_mm(task);
+	if (!mm)
+		goto out_free;
+
+	ret = -EIO;
+
+	if (file->private_data != (void*)((long)current->self_exec_id))
+		goto out_put;
+
+	ret = 0;
 
 	while (count > 0) {
 		int this_len, retval;
 
 		this_len = (count > PAGE_SIZE) ? PAGE_SIZE : count;
 		if (copy_from_user(page, buf, this_len)) {
-			copied = -EFAULT;
+			ret = -EFAULT;
 			break;
 		}
 		retval = access_process_vm(task, dst, page, this_len, 1);
-		if (!retval) {
-			if (!copied)
-				copied = -EIO;
+
+		if (!retval || !MAY_PTRACE(task) || !ptrace_may_attach(task)) {
+			if (!ret)
+				ret = -EIO;
 			break;
 		}
-		copied += retval;
-		buf += retval;
+
+		ret += retval;
 		dst += retval;
-		count -= retval;			
+		buf += retval;
+		count -= retval;
 	}
 	*ppos = dst;
+
+
+out_put:
+	mmput(mm);
+out_free:
 	free_page((unsigned long) page);
-	return copied;
+out:
+	return ret;
 }
-#endif
 
 static loff_t mem_lseek(struct file * file, loff_t offset, int orig)
 {

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/