Re: [PATCH] powerpc/mm: add devmem_is_allowed() for STRICT_DEVMEM checking

[Steve Best <sfbest@us.ibm.com>]

On Mon, 2011-01-31 at 13:40 -0600, Scott Wood wrote:
> On Mon, 31 Jan 2011 14:16:00 -0500
> Steve Best <sfbest@us.ibm.com> wrote:
> 
> >     Provide devmem_is_allowed() routine to restrict access to kernel
> >     memory from userspace.
> >     Set CONFIG_STRICT_DEVMEM config option to switch on checking.
> > 
> > Signed-off-by: Steve Best <sfbest@us.ibm.com>
> > 
> > diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> > index 2d38a50..6805d5d 100644
> > --- a/arch/powerpc/Kconfig.debug
> > +++ b/arch/powerpc/Kconfig.debug
> > @@ -299,4 +299,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
> > diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> > index 53b64be..f225032 100644
> > --- a/arch/powerpc/include/asm/page.h
> > +++ b/arch/powerpc/include/asm/page.h
> > @@ -262,6 +262,11 @@ extern void copy_user_page(void *to, void *from, unsigned long vaddr,
> >  		struct page *p);
> >  extern int page_is_ram(unsigned long pfn);
> >  
> > +static inline int devmem_is_allowed(unsigned long pfn)
> > +{
> > +        return 0;
> > +}
> > +
> 
> I don't see how this is a sane thing to turn on by default (you're not
> restricting it, BTW -- you're completely disabling it with that
> implementation of devmem_is_allowed).  It will break anything that
> uses /dev/mem to access I/O, 

could you expand on what I/O depends on /dev/mem, so I can take
that into account?

> possibly including desktoppy stuff like X
> servers, 

you are right just found out that X needs to access it. will 
take that into account
> as well as lots of stuff that goes on in embedded setups.

could you explain more about what needs access to /dev/mem in 
the embedded setups?

> 
> You need to be root to access /dev/mem, and root has plenty of
> other options for causing "disastrous" results.  You don't just stumble
> onto /dev/mem by accident.
> 
> -Scott

-Steve