Skip to contentSkip to conversation

Enable CONFIG_STRICT_DEVMEM support for Powerpc

PATCH In review Sukadev Bhattiprolu 2 participants 4 messages Dec 2, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Enable CONFIG_STRICT_DEVMEM support for Powerpc

Quoted 1 line

From aebed2e9fb9fba7dd0a34595780b828d7a545ba2 Mon Sep 17 00:00:00 2001

From: Sukadev Bhattiprolu <[email protected]>
Date: Mon, 29 Aug 2011 14:12:08 -0700
Subject: [PATCH 1/1][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc.

As described in the help text in the patch, this token restricts general access to /dev/mem as a way of increasing security. Specifically, access to exclusive IOMEM and kernel RAM is denied unless CONFIG_STRICT_DEVMEM is set to 'n'.

Implement the 'devmem_is_allowed()' interface for POWER. It will be called from range_is_allowed() when userpsace attempts to access /dev/mem.

This patch is based on an earlier patch from Steve Best and with input from Paul Mackerras.

A test for this patch:

	# cat /proc/rtas/rmo_buffer 
	000000000f190000 10000
	# python -c "print 0x000000000f190000 / 0x10000"
	3865
	# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865
	1+0 records in
	1+0 records out
	65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s
	# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3864
	dd: reading `/dev/mem': Operation not permitted
	0+0 records in
	0+0 records out
	0 bytes (0 B) copied, 0.000226844 s, 0.0 kB/s
	# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3866
	dd: reading `/dev/mem': Operation not permitted
	0+0 records in
	0+0 records out
	0 bytes (0 B) copied, 0.00023542 s, 0.0 kB/s
	# dd if=/dev/mem of=/tmp/foo
	dd: reading `/dev/mem': Operation not permitted
	0+0 records in
	0+0 records out
	0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s

Changelog[v2]:
- [Anton Blanchard] Punch a hole in devmem to allow librtas/dlpar tools access to /dev/mem.

---
 arch/powerpc/Kconfig.debug      |   12 ++++++++++++
 arch/powerpc/include/asm/page.h |    1 +
 arch/powerpc/kernel/rtas.c      |   10 ++++++++++
 arch/powerpc/mm/mem.c           |   20 ++++++++++++++++++++
 drivers/char/mem.c              |    5 +++--
 5 files changed, 46 insertions(+), 2 deletions(-)
arch/powerpc/Kconfig.debug +12 -0
index 067cb84..1cf1b00 100644
@@ -298,4 +298,16 @@ config PPC_EARLY_DEBUG_CPM_ADDR
 	  platform probing is done, all platforms selected must
 	  share the same address.
 
+config STRICT_DEVMEM
+	def_bool y
+	prompt "Filter access to /dev/mem"
+	help
+	  This option restricts access to /dev/mem.  If this option is
+	  disabled, you allow userspace access to all memory, including
+	  kernel and userspace memory. Accidental memory access is likely
+	  to be disastrous.
+	  Memory access is required for experts who want to debug the kernel.
+
+	  If you are unsure, say Y.
+
 endmenu
arch/powerpc/include/asm/page.h +1 -0
index 2cd664e..dc2ec96 100644
@@ -261,6 +261,7 @@ extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg);
 extern void copy_user_page(void *to, void *from, unsigned long vaddr,
 		struct page *p);
 extern int page_is_ram(unsigned long pfn);
+extern int devmem_is_allowed(unsigned long pfn);
 
 #ifdef CONFIG_PPC_SMLPAR
 void arch_free_page(struct page *page, int order);
arch/powerpc/kernel/rtas.c +10 -0
index d5ca823..07cf2cf 100644
@@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
 	return 0;
 }
 
+int page_is_rtas(unsigned long pfn)
+{
+	unsigned long paddr = (pfn << PAGE_SHIFT);
+
+	if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
+		return 1;
+
+	return 0;
+}
+
 /*
  * Call early during boot, before mem init or bootmem, to retrieve the RTAS
  * informations from the device-tree and allocate the RMO buffer for userland
arch/powerpc/mm/mem.c +20 -0
index c781bbc..d21dbc6 100644
@@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
 	hash_preload(vma->vm_mm, address, access, trap);
 #endif /* CONFIG_PPC_STD_MMU */
 }
+
+extern int page_is_rtas(unsigned long pfn);
+
+/*
+ * devmem_is_allowed(): check to see if /dev/mem access to a certain address
+ * is valid. The argument is a physical page number.
+ *
+ * Access has to be given to non-kernel-ram areas as well, these contain the
+ * PCI mmio resources as well as potential bios/acpi data regions.
+ */
+int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	if (page_is_rtas(pfn))
+		return 1;
+	return 0;
+}
drivers/char/mem.c +3 -2
index 8fc04b4..64917e8 100644
@@ -29,6 +29,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
+#include <asm/page.h>
 
 #ifdef CONFIG_IA64
 # include <linux/efi.h>
@@ -66,8 +67,8 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 	while (cursor < to) {
 		if (!devmem_is_allowed(pfn)) {
 			printk(KERN_INFO
-		"Program %s tried to access /dev/mem between %Lx->%Lx.\n",
-				current->comm, from, to);
+		"Program %s tried to access /dev/mem between %Lx->%Lx and "
+			"pfn %lu.\n", current->comm, from, to, pfn);
 			return 0;
 		}
 		cursor += PAGE_SIZE;
Signature · 1 line
1.7.0.4
#1 lore raw
Quoted 10 lines · Sukadev Bhattiprolu
Quoted 1 line

From aebed2e9fb9fba7dd0a34595780b828d7a545ba2 Mon Sep 17 00:00:00 2001

From: Sukadev Bhattiprolu <[email protected]>
Date: Mon, 29 Aug 2011 14:12:08 -0700
Subject: [PATCH 1/1][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc.

As described in the help text in the patch, this token restricts general access to /dev/mem as a way of increasing security. Specifically, access to exclusive IOMEM and kernel RAM is denied unless CONFIG_STRICT_DEVMEM is set to 'n'.

Quoted 5 lines

Implement the 'devmem_is_allowed()' interface for POWER. It will be called from range_is_allowed() when userpsace attempts to access /dev/mem.

This patch is based on an earlier patch from Steve Best and with input from Paul Mackerras.

A slightly different version of that patch is already in my -next branch since earlier this week. Please send a followup patch adding the missing rtas bit.

Note that I've dropped the generic printk change which has nothing to do in that patch and can/should be submitted separately if it's really needed.

Cheers,
Ben.

Quoted 149 lines

A test for this patch:

	# cat /proc/rtas/rmo_buffer 
	000000000f190000 10000
	# python -c "print 0x000000000f190000 / 0x10000"
	3865
	# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865
	1+0 records in
	1+0 records out
	65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s
	# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3864
	dd: reading `/dev/mem': Operation not permitted
	0+0 records in
	0+0 records out
	0 bytes (0 B) copied, 0.000226844 s, 0.0 kB/s
	# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3866
	dd: reading `/dev/mem': Operation not permitted
	0+0 records in
	0+0 records out
	0 bytes (0 B) copied, 0.00023542 s, 0.0 kB/s
	# dd if=/dev/mem of=/tmp/foo
	dd: reading `/dev/mem': Operation not permitted
	0+0 records in
	0+0 records out
	0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s

Changelog[v2]:
- [Anton Blanchard] Punch a hole in devmem to allow librtas/dlpar tools access to /dev/mem.

---
 arch/powerpc/Kconfig.debug      |   12 ++++++++++++
 arch/powerpc/include/asm/page.h |    1 +
 arch/powerpc/kernel/rtas.c      |   10 ++++++++++
 arch/powerpc/mm/mem.c           |   20 ++++++++++++++++++++
 drivers/char/mem.c              |    5 +++--
 5 files changed, 46 insertions(+), 2 deletions(-)
arch/powerpc/Kconfig.debug +12 -0
index 067cb84..1cf1b00 100644
@@ -298,4 +298,16 @@ config PPC_EARLY_DEBUG_CPM_ADDR
 	  platform probing is done, all platforms selected must
 	  share the same address.
 
+config STRICT_DEVMEM
+	def_bool y
+	prompt "Filter access to /dev/mem"
+	help
+	  This option restricts access to /dev/mem.  If this option is
+	  disabled, you allow userspace access to all memory, including
+	  kernel and userspace memory. Accidental memory access is likely
+	  to be disastrous.
+	  Memory access is required for experts who want to debug the kernel.
+
+	  If you are unsure, say Y.
+
 endmenu
arch/powerpc/include/asm/page.h +1 -0
index 2cd664e..dc2ec96 100644
@@ -261,6 +261,7 @@ extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg);
 extern void copy_user_page(void *to, void *from, unsigned long vaddr,
 		struct page *p);
 extern int page_is_ram(unsigned long pfn);
+extern int devmem_is_allowed(unsigned long pfn);
 
 #ifdef CONFIG_PPC_SMLPAR
 void arch_free_page(struct page *page, int order);
arch/powerpc/kernel/rtas.c +10 -0
index d5ca823..07cf2cf 100644
@@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
 	return 0;
 }
 
+int page_is_rtas(unsigned long pfn)
+{
+	unsigned long paddr = (pfn << PAGE_SHIFT);
+
+	if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
+		return 1;
+
+	return 0;
+}
+
 /*
  * Call early during boot, before mem init or bootmem, to retrieve the RTAS
  * informations from the device-tree and allocate the RMO buffer for userland
arch/powerpc/mm/mem.c +20 -0
index c781bbc..d21dbc6 100644
@@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
 	hash_preload(vma->vm_mm, address, access, trap);
 #endif /* CONFIG_PPC_STD_MMU */
 }
+
+extern int page_is_rtas(unsigned long pfn);
+
+/*
+ * devmem_is_allowed(): check to see if /dev/mem access to a certain address
+ * is valid. The argument is a physical page number.
+ *
+ * Access has to be given to non-kernel-ram areas as well, these contain the
+ * PCI mmio resources as well as potential bios/acpi data regions.
+ */
+int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	if (page_is_rtas(pfn))
+		return 1;
+	return 0;
+}
drivers/char/mem.c +3 -2
index 8fc04b4..64917e8 100644
@@ -29,6 +29,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
+#include <asm/page.h>
 
 #ifdef CONFIG_IA64
 # include <linux/efi.h>
@@ -66,8 +67,8 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 	while (cursor < to) {
 		if (!devmem_is_allowed(pfn)) {
 			printk(KERN_INFO
-		"Program %s tried to access /dev/mem between %Lx->%Lx.\n",
-				current->comm, from, to);
+		"Program %s tried to access /dev/mem between %Lx->%Lx and "
+			"pfn %lu.\n", current->comm, from, to, pfn);
 			return 0;
 		}
 		cursor += PAGE_SIZE;
#2 lore raw
In reply to Sukadev Bhattiprolu

And an additional comment regarding the rtas bit:

Quoted 9 lines

#ifdef CONFIG_PPC_SMLPAR
void arch_free_page(struct page *page, int order);

arch/powerpc/kernel/rtas.c +0 -0
index d5ca823..07cf2cf 100644
@@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
 	return 0;
 }

arch/powerpc/kernel/rtas.c asmlinkage int ppc_rtas(struct rtas_args __user *uargs) · line 948

Quoted 10 lines
arch/powerpc/kernel/rtas.c +9 -0
 
+int page_is_rtas(unsigned long pfn)
+{
+	unsigned long paddr = (pfn << PAGE_SHIFT);
+
+	if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
+		return 1;
+
+	return 0;
+}

So this only exist whenever rtas.c is compiled... but ...

Quoted 22 lines
arch/powerpc/kernel/rtas.c +1 -0
+
 /*
  * Call early during boot, before mem init or bootmem, to retrieve the RTAS
  * informations from the device-tree and allocate the RMO buffer for userland
arch/powerpc/mm/mem.c +10 -0
index c781bbc..d21dbc6 100644
@@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
 	hash_preload(vma->vm_mm, address, access, trap);
 #endif /* CONFIG_PPC_STD_MMU */
 }
+
+extern int page_is_rtas(unsigned long pfn);
+
+/*
+ * devmem_is_allowed(): check to see if /dev/mem access to a certain address
+ * is valid. The argument is a physical page number.
+ *
+ * Access has to be given to non-kernel-ram areas as well, these contain the
+ * PCI mmio resources as well as potential bios/acpi data regions.
+ */

arch/powerpc/mm/mem.c void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, · line 571

Quoted 10 lines
arch/powerpc/mm/mem.c +10 -0
+int devmem_is_allowed(unsigned long pfn)
+{
+	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+		return 0;
+	if (!page_is_ram(pfn))
+		return 1;
+	if (page_is_rtas(pfn))
+		return 1;
+	return 0;
+}

This calls it unconditionally... you just broke the build of all !rtas platforms. Additionally, putting an extern definition like that in a .c file is gross at best....

Please instead, put in a header something like

#ifdef CONFIG_PPC_RTAS
extern int page_is_rtas(unsigned long pfn); #else
static inline int page_is_rtas(unsigned long pfn) { } #endif

And while at it, call it page_is_rtas_user_buf(); to make it clear what we are talking about, ie, not RTAS core per-se but specifically the RMO buffer.

Cheers,
Ben.

#3 lore raw

Benjamin Herrenschmidt [[email protected]] wrote:

| And an additional comment regarding the rtas bit:
| > +int devmem_is_allowed(unsigned long pfn)
| > +{
| > +	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
| > +		return 0;
| > +	if (!page_is_ram(pfn))
| > +		return 1;
| > +	if (page_is_rtas(pfn))
| > +		return 1;
| > +	return 0;
| > +}
| 
| This calls it unconditionally... you just broke the build of all !rtas
| platforms. Additionally, putting an extern definition like that in a .c
| file is gross at best....

Oh, Sorry.

| 
| Please instead, put in a header something like
| 
| #ifdef CONFIG_PPC_RTAS
| extern int page_is_rtas(unsigned long pfn);
| #else
| static inline int page_is_rtas(unsigned long pfn) { }
| #endif
| 
| And while at it, call it page_is_rtas_user_buf(); to make it clear what
| we are talking about, ie, not RTAS core per-se but specifically the RMO
| buffer.

Ok. I will rename, move the declaration to <asm/rtas.h> and resend the incremental patch.

Sukadev