Re: [PATCH] Fix mmap_kmem (was: [question] What's the difference between /dev/kmem and /dev/mem)

Arjan van de Ven
2005-08-13T16:50:20Z

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

On Fri, 2005-08-12 at 09:35 -0700, Linus Torvalds wrote:
>
> On Thu, 11 Aug 2005, Steven Rostedt wrote:
> >
> > Found the problem. It is a bug with mmap_kmem. The order of checks is
> > wrong, so here's the patch. Attached is a little program that reads the
> > System map looking for the variable modprobe_path. If it finds it, then
> > it opens /dev/kmem for read only and mmaping it to read the contents of
> > modprobe_path.
>
> I'm actually more inclined to try to deprecate /dev/kmem.. I don't think
> anybody has ever really used it except for some rootkits. It only exists
> in the first place because it's historical.
>
> We do need to support /dev/mem for X, but even that might go away some
> day.
>
> So I'd be perfectly happy to fix this, but I'd be even happier if we made
> the whole kmem thing a config variable (maybe even default it to "off").

attached is a simple patch that does exactly this...

[-- Attachment #2: mem.patch --]
[-- Type: text/x-patch, Size: 3689 bytes --]

diff -purN linux-before/drivers/char/Kconfig linux-2.6.12/drivers/char/Kconfig
--- linux-before/drivers/char/Kconfig 2005-08-13 18:38:11.210821000 +0200
+++ linux-2.6.12/drivers/char/Kconfig 2005-08-13 18:44:31.182935339 +0200
@@ -1000,6 +1000,12 @@ config MMTIMER
The mmtimer device allows direct userspace access to the
Altix system timer.

+config DEVKMEM
+ tristate "Userspace access to virtual kernel memory (/dev/kmem)"
+ help
+ The /dev/kmem device allows userspace access to kernel virtual
+ memory, which can be useful for kernel debugging.
+
source "drivers/char/tpm/Kconfig"

endmenu
diff -purN linux-before/drivers/char/mem.c linux-2.6.12/drivers/char/mem.c
--- linux-before/drivers/char/mem.c 2005-08-13 18:38:11.261815000 +0200
+++ linux-2.6.12/drivers/char/mem.c 2005-08-13 18:41:09.002351297 +0200
@@ -259,22 +259,6 @@ static int mmap_mem(struct file * file,
return 0;
}

-static int mmap_kmem(struct file * file, struct vm_area_struct * vma)
-{
- unsigned long long val;
- /*
- * RED-PEN: on some architectures there is more mapped memory
- * than available in mem_map which pfn_valid checks
- * for. Perhaps should add a new macro here.
- *
- * RED-PEN: vmalloc is not supported right now.
- */
- if (!pfn_valid(vma->vm_pgoff))
- return -EIO;
- val = (u64)vma->vm_pgoff << PAGE_SHIFT;
- vma->vm_pgoff = __pa(val) >> PAGE_SHIFT;
- return mmap_mem(file, vma);
-}

#ifdef CONFIG_CRASH_DUMP
/*
@@ -313,6 +297,25 @@ static ssize_t read_oldmem(struct file *
extern long vread(char *buf, char *addr, unsigned long count);
extern long vwrite(char *buf, char *addr, unsigned long count);

+#ifdef CONFIG_DEVKMEM
+
+static int mmap_kmem(struct file * file, struct vm_area_struct * vma)
+{
+ unsigned long long val;
+ /*
+ * RED-PEN: on some architectures there is more mapped memory
+ * than available in mem_map which pfn_valid checks
+ * for. Perhaps should add a new macro here.
+ *
+ * RED-PEN: vmalloc is not supported right now.
+ */
+ if (!pfn_valid(vma->vm_pgoff))
+ return -EIO;
+ val = (u64)vma->vm_pgoff << PAGE_SHIFT;
+ vma->vm_pgoff = __pa(val) >> PAGE_SHIFT;
+ return mmap_mem(file, vma);
+}
+
/*
* This function reads the *virtual* memory as seen by the kernel.
*/
@@ -521,6 +524,8 @@ static ssize_t write_kmem(struct file *
return virtr + wrote;
}

+#endif
+
#if (defined(CONFIG_ISA) || !defined(__mc68000__)) && (!defined(CONFIG_PPC_ISERIES) || defined(CONFIG_PCI))
static ssize_t read_port(struct file * file, char __user * buf,
size_t count, loff_t *ppos)
@@ -768,6 +773,7 @@ static struct file_operations mem_fops =
.open = open_mem,
};

+#ifdef CONFIG_DEVKMEM
static struct file_operations kmem_fops = {
.llseek = memory_lseek,
.read = read_kmem,
@@ -775,6 +781,7 @@ static struct file_operations kmem_fops
.mmap = mmap_kmem,
.open = open_kmem,
};
+#endif

static struct file_operations null_fops = {
.llseek = null_lseek,
@@ -843,9 +850,11 @@ static int memory_open(struct inode * in
case 1:
filp->f_op = &mem_fops;
break;
+#ifdef CONFIG_DEVKMEM
case 2:
filp->f_op = &kmem_fops;
break;
+#endif
case 3:
filp->f_op = &null_fops;
break;
@@ -894,7 +903,9 @@ static const struct {
struct file_operations *fops;
} devlist[] = { /* list of minor devices */
{1, "mem", S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops},
+#ifdef CONFIG_DEVKMEM
{2, "kmem", S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops},
+#endif
{3, "null", S_IRUGO | S_IWUGO, &null_fops},
#if (defined(CONFIG_ISA) || !defined(__mc68000__)) && (!defined(CONFIG_PPC_ISERIES) || defined(CONFIG_PCI))
{4, "port", S_IRUSR | S_IWUSR | S_IRGRP, &port_fops},