Patchwork sparc64 - strict devmem

login
register
mail settings
Submitter Bob Picco
Date March 3, 2014, 5 p.m.
Message ID <1393866035-7939-1-git-send-email-bpicco@meloft.net>
Download mbox | patch
Permalink /patch/325903/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Bob Picco - March 3, 2014, 5 p.m.
From: bob picco <bpicco@meloft.net>

This patch adds support to enable STRICT_DEVMEM for sparc64. By selecting the
new config option you enable this feature.

CC: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Bob Picco <bob.picco@oracle.com>
---
 arch/sparc/Kconfig.debug         |    8 ++++++++
 arch/sparc/include/asm/page_64.h |    1 +
 arch/sparc/mm/init_64.c          |   11 +++++++++++
 3 files changed, 20 insertions(+), 0 deletions(-)
David Miller - March 4, 2014, 8:46 p.m.
From: Bob Picco <bpicco@meloft.net>
Date: Mon,  3 Mar 2014 12:00:35 -0500

> From: bob picco <bpicco@meloft.net>
> 
> This patch adds support to enable STRICT_DEVMEM for sparc64. By selecting the
> new config option you enable this feature.
> 
> CC: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Bob Picco <bob.picco@oracle.com>

Ok, I looked more deeply into this and I'm going to rescind my request
to use kern_addr_valid().

This devmem_is_allowed() seems to also want to allow non-RAM MMIO
accesses too.

So using the iomem resource is probably the way to go.

Two things:

1) We have to make sure caching will be disabled and the side-effect
   bit will be set in accesses to non-RAM pages.

2) Our implementation of devmem_is_allowed() should be aligned as
   much as possible with other implementations.  Perhaps powerpc
   is the best example to use.

Thanks.


--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bob Picco - March 5, 2014, 1:11 p.m.
Hi,
David Miller wrote:	[Tue Mar 04 2014, 03:46:26PM EST]
> From: Bob Picco <bpicco@meloft.net>
> Date: Mon,  3 Mar 2014 12:00:35 -0500
> 
> > From: bob picco <bpicco@meloft.net>
> > 
> > This patch adds support to enable STRICT_DEVMEM for sparc64. By selecting the
> > new config option you enable this feature.
> > 
> > CC: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Bob Picco <bob.picco@oracle.com>
> 
> Ok, I looked more deeply into this and I'm going to rescind my request
> to use kern_addr_valid().
Okay.
> 
> This devmem_is_allowed() seems to also want to allow non-RAM MMIO
> accesses too.
Yes, this is my bad. I expected this issue to come up in the first round but
forgot. I wasn't certain general MMIO access via /dev/mem is of value to
sparc64. One could always turn off the config option.
> 
> So using the iomem resource is probably the way to go.
> 
> Two things:
> 
> 1) We have to make sure caching will be disabled and the side-effect
>    bit will be set in accesses to non-RAM pages.
> 
> 2) Our implementation of devmem_is_allowed() should be aligned as
>    much as possible with other implementations.  Perhaps powerpc
>    is the best example to use.
I'll take a peek at powerpc.

thanx and you're welcome,

bob
> 
> Thanks.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bob Picco - March 5, 2014, 8:18 p.m.
Hi,
David Miller wrote:	[Tue Mar 04 2014, 03:46:26PM EST]
> From: Bob Picco <bpicco@meloft.net>
> Date: Mon,  3 Mar 2014 12:00:35 -0500
> 
> > From: bob picco <bpicco@meloft.net>
> > 
> > This patch adds support to enable STRICT_DEVMEM for sparc64. By selecting the
> > new config option you enable this feature.
> > 
> > CC: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Bob Picco <bob.picco@oracle.com>
> 
> Ok, I looked more deeply into this and I'm going to rescind my request
> to use kern_addr_valid().
> 
> This devmem_is_allowed() seems to also want to allow non-RAM MMIO
> accesses too.
> 
> So using the iomem resource is probably the way to go.
> 
> Two things:
> 
> 1) We have to make sure caching will be disabled and the side-effect
>    bit will be set in accesses to non-RAM pages.
> 
> 2) Our implementation of devmem_is_allowed() should be aligned as
>    much as possible with other implementations.  Perhaps powerpc
>    is the best example to use.
Alright, hold off on strict_devmem. I realize after looking at powerpc
that 1 1/2 years ago my exhaustion consumed me. I plugged up bus error
with strict_devmem by inverting the purpose of strict_devmem. In other words,
I allow memory access but don't allow MMIO and HOLES. Very sorry about this!

Let me plug up the bus error differently.

Hm, I'm not certain to be all that thrilled with strict_devmem any longer.
Though I'm certain some security person would be.

This is what happens when your brain is fried which it was 1 1/2 years ago.

thanx,

bob
> 
> Thanks.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/Kconfig.debug b/arch/sparc/Kconfig.debug
index 6db35fb..92f2388 100644
--- a/arch/sparc/Kconfig.debug
+++ b/arch/sparc/Kconfig.debug
@@ -6,6 +6,14 @@  config TRACE_IRQFLAGS_SUPPORT
 
 source "lib/Kconfig.debug"
 
+config STRICT_DEVMEM
+	bool "Filter access to /dev/mem"
+	---help---
+	  If this option is disabled, you allow userspace (root) access to all
+	  of memory, including kernel and userspace memory. Accidental
+	  access to this is obviously disastrous, but specific access can
+	  be used by people debugging the kernel.
+
 config DEBUG_DCFLUSH
 	bool "D-cache flush debugging"
 	depends on SPARC64 && DEBUG_KERNEL
diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h
index aac53fc..e54ee50 100644
--- a/arch/sparc/include/asm/page_64.h
+++ b/arch/sparc/include/asm/page_64.h
@@ -36,6 +36,7 @@  extern void hugetlb_setup(struct pt_regs *regs);
 
 #define WANT_PAGE_VIRTUAL
 
+extern int devmem_is_allowed(unsigned long pagenr);
 extern void _clear_page(void *page);
 #define clear_page(X)	_clear_page((void *)(X))
 struct page;
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index a4a9b69..0a38a42 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -2759,3 +2759,14 @@  static int __init report_memory(void)
 	return 0;
 }
 device_initcall(report_memory);
+
+#ifdef CONFIG_STRICT_DEVMEM
+int devmem_is_allowed(unsigned long pagenr)
+{
+	unsigned long phys_addr = pagenr << PAGE_SHIFT;
+	unsigned long virt_addr = (unsigned long) __va(phys_addr);
+	int rc = kern_addr_valid(virt_addr);
+
+	return rc;
+}
+#endif /* CONFIG_STRICT_DEVMEM */