| Message ID | 20110131191109.5727.87742.sendpatchset@squad5-lp1.lab.bos.redhat.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Mon, Jan 31, 2011 at 2:16 PM, 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; > +} > + Er, should this be toggled via CONFIG_STRICT_DEVMEM somehow? Or I guess I'm missing why the config option had to be added if not. josh
On Mon, Jan 31, 2011 at 2:25 PM, Josh Boyer <jwboyer@gmail.com> wrote: > On Mon, Jan 31, 2011 at 2:16 PM, 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; >> +} >> + > > Er, should this be toggled via CONFIG_STRICT_DEVMEM somehow? Or I > guess I'm missing why the config option had to be added if not. Nevermind. I see that it's done in the drivers/char/mem.c file. Should have looked more first. josh
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, possibly including desktoppy stuff like X servers, as well as lots of stuff that goes on in 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
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
On Tue, 1 Feb 2011 12:21:45 -0500 Steve Best <sfbest@us.ibm.com> wrote: > > On Mon, 2011-01-31 at 13:40 -0600, Scott Wood wrote: > > 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? It could be anything. You're shutting off, by default, a longstanding userspace interface, that already has adequate security protection. Even x86 doesn't default it to yes (though it does say "if in doubt say Y"), and when enabled x86 only restricts access to memory, not I/O. > > 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? All sorts of custom stuff -- userspace drivers, special memory regions reserved at boot, etc. If you really want this, I suggest prohibiting access only when it's an actual RAM page tracked by the kernel (maybe only when PageReserved is unset as well?), or when iomem_is_exclusive returns true. -Scott
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; +} + #ifdef CONFIG_PPC_SMLPAR void arch_free_page(struct page *page, int order); #define HAVE_ARCH_FREE_PAGE
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>