Message ID | 20181012080723.15825-1-clg@kaod.org |
---|---|
State | Accepted, archived |
Headers | show |
Series | [linux,dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device | expand |
On Fri, 12 Oct 2018 at 18:37, Cédric Le Goater <clg@kaod.org> wrote: > > For security reasons, some configuration needs to run without /dev/mem > but on some occasions, to debug HW for instance, it's still useful to > be able to reboot the system with access to physical memory. > > Add a kernel parameter which activates the /dev/mem device only when > 'mem.devmem' is enabled. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Thanks Cédric. I've put this in the 4.18 tree. Can you submit this upstream too please? Cheers, Joel > --- > drivers/char/mem.c | 12 ++++++++++++ > Documentation/admin-guide/kernel-parameters.txt | 3 +++ > drivers/char/Kconfig | 9 +++++++++ > 3 files changed, 24 insertions(+) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index df66a9dd0aae..8c021a559e6c 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -10,6 +10,7 @@ > */ > > #include <linux/mm.h> > +#include <linux/moduleparam.h> > #include <linux/miscdevice.h> > #include <linux/slab.h> > #include <linux/vmalloc.h> > @@ -36,6 +37,7 @@ > # include <linux/efi.h> > #endif > > +#define DEVMEM_MINOR 1 > #define DEVPORT_MINOR 4 > > static inline unsigned long size_inside_page(unsigned long start, > @@ -912,6 +914,12 @@ static char *mem_devnode(struct device *dev, umode_t *mode) > return NULL; > } > > +#ifdef CONFIG_DEVMEM_BOOTPARAM > +static bool devmem; > +module_param(devmem, bool, 0444); > +MODULE_PARM_DESC(devmem, "kernel parameter to activate /dev/mem"); > +#endif > + > static struct class *mem_class; > > static int __init chr_dev_init(void) > @@ -930,6 +938,10 @@ static int __init chr_dev_init(void) > if (!devlist[minor].name) > continue; > > +#ifdef CONFIG_DEVMEM_BOOTPARAM > + if (minor == DEVMEM_MINOR && !devmem) > + continue; > +#endif > /* > * Create /dev/port? > */ > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 1370b424a453..a8ed12d0d678 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2402,6 +2402,9 @@ > deep - Suspend-To-RAM or equivalent (if supported) > See Documentation/admin-guide/pm/sleep-states.rst. > > + mem.devmem= Activate the /dev/mem device > + Format: <bool> (1/Y/y=enable, 0/N/n=disable) > + > meye.*= [HW] Set MotionEye Camera parameters > See Documentation/media/v4l-drivers/meye.rst. > > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig > index 212f447938ae..08c56148190b 100644 > --- a/drivers/char/Kconfig > +++ b/drivers/char/Kconfig > @@ -16,6 +16,15 @@ config DEVMEM > memory. > When in doubt, say "Y". > > +config DEVMEM_BOOTPARAM > + bool "mem.devmem boot parameter" > + depends on DEVMEM > + default n > + help > + This option adds a 'mem.devmem' kernel parameter which activates > + the /dev/mem device when enabled. > + When in doubt, say "N". > + > config DEVKMEM > bool "/dev/kmem virtual device support" > # On arm64, VMALLOC_START < PAGE_OFFSET, which confuses kmem read/write > -- > 2.17.1 >
On Mon, 22 Oct 2018, at 08:54, Joel Stanley wrote: > On Fri, 12 Oct 2018 at 18:37, Cédric Le Goater <clg@kaod.org> wrote: > > > > For security reasons, some configuration needs to run without /dev/mem > > but on some occasions, to debug HW for instance, it's still useful to > > be able to reboot the system with access to physical memory. > > > > Add a kernel parameter which activates the /dev/mem device only when > > 'mem.devmem' is enabled. > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > Thanks Cédric. I've put this in the 4.18 tree. > > Can you submit this upstream too please? Have this been done? Just following up out of interest. I do wonder about it though. /dev/mem is accessible if you're root, but given pretty much everything runs as root on the BMC we turn it off. But if it's just a kernel commandline parameter away, it's also just a `fw_setenv ... && reboot` away, at which point all the security is gone? If you're somehow not root on the BMC then you wouldn't have access even if it were present, and you can't change the u-boot environment either. Andrew
On 11/8/18 2:11 AM, Andrew Jeffery wrote: > On Mon, 22 Oct 2018, at 08:54, Joel Stanley wrote: >> On Fri, 12 Oct 2018 at 18:37, Cédric Le Goater <clg@kaod.org> wrote: >>> >>> For security reasons, some configuration needs to run without /dev/mem >>> but on some occasions, to debug HW for instance, it's still useful to >>> be able to reboot the system with access to physical memory. >>> >>> Add a kernel parameter which activates the /dev/mem device only when >>> 'mem.devmem' is enabled. >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> >> Thanks Cédric. I've put this in the 4.18 tree. >> >> Can you submit this upstream too please? > > Have this been done? Just following up out of interest. no. > I do wonder about it though. /dev/mem is accessible if you're root, but given > pretty much everything runs as root on the BMC we turn it off. But if it's just > a kernel commandline parameter away, it's also just a > `fw_setenv ... && reboot` away, at which point all the security is gone? If > you're somehow not root on the BMC then you wouldn't have access even if > it were present, and you can't change the u-boot environment either. You seem to be suggesting that we should reactivate /dev/mem ? C.
On Tue, 13 Nov 2018, at 23:33, Cédric Le Goater wrote: > On 11/8/18 2:11 AM, Andrew Jeffery wrote: > > On Mon, 22 Oct 2018, at 08:54, Joel Stanley wrote: > >> On Fri, 12 Oct 2018 at 18:37, Cédric Le Goater <clg@kaod.org> wrote: > >>> > >>> For security reasons, some configuration needs to run without /dev/mem > >>> but on some occasions, to debug HW for instance, it's still useful to > >>> be able to reboot the system with access to physical memory. > >>> > >>> Add a kernel parameter which activates the /dev/mem device only when > >>> 'mem.devmem' is enabled. > >>> > >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> > >> Thanks Cédric. I've put this in the 4.18 tree. > >> > >> Can you submit this upstream too please? > > > > Have this been done? Just following up out of interest. > > no. > > > I do wonder about it though. /dev/mem is accessible if you're root, but given > > pretty much everything runs as root on the BMC we turn it off. But if it's just > > a kernel commandline parameter away, it's also just a > > `fw_setenv ... && reboot` away, at which point all the security is gone? If > > you're somehow not root on the BMC then you wouldn't have access even if > > it were present, and you can't change the u-boot environment either. > > You seem to be suggesting that we should reactivate /dev/mem ? Not really arguing either way, just observing that the patch doesn't gain any security over just enabling it outright. > > C.
diff --git a/drivers/char/mem.c b/drivers/char/mem.c index df66a9dd0aae..8c021a559e6c 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -10,6 +10,7 @@ */ #include <linux/mm.h> +#include <linux/moduleparam.h> #include <linux/miscdevice.h> #include <linux/slab.h> #include <linux/vmalloc.h> @@ -36,6 +37,7 @@ # include <linux/efi.h> #endif +#define DEVMEM_MINOR 1 #define DEVPORT_MINOR 4 static inline unsigned long size_inside_page(unsigned long start, @@ -912,6 +914,12 @@ static char *mem_devnode(struct device *dev, umode_t *mode) return NULL; } +#ifdef CONFIG_DEVMEM_BOOTPARAM +static bool devmem; +module_param(devmem, bool, 0444); +MODULE_PARM_DESC(devmem, "kernel parameter to activate /dev/mem"); +#endif + static struct class *mem_class; static int __init chr_dev_init(void) @@ -930,6 +938,10 @@ static int __init chr_dev_init(void) if (!devlist[minor].name) continue; +#ifdef CONFIG_DEVMEM_BOOTPARAM + if (minor == DEVMEM_MINOR && !devmem) + continue; +#endif /* * Create /dev/port? */ diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 1370b424a453..a8ed12d0d678 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2402,6 +2402,9 @@ deep - Suspend-To-RAM or equivalent (if supported) See Documentation/admin-guide/pm/sleep-states.rst. + mem.devmem= Activate the /dev/mem device + Format: <bool> (1/Y/y=enable, 0/N/n=disable) + meye.*= [HW] Set MotionEye Camera parameters See Documentation/media/v4l-drivers/meye.rst. diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 212f447938ae..08c56148190b 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -16,6 +16,15 @@ config DEVMEM memory. When in doubt, say "Y". +config DEVMEM_BOOTPARAM + bool "mem.devmem boot parameter" + depends on DEVMEM + default n + help + This option adds a 'mem.devmem' kernel parameter which activates + the /dev/mem device when enabled. + When in doubt, say "N". + config DEVKMEM bool "/dev/kmem virtual device support" # On arm64, VMALLOC_START < PAGE_OFFSET, which confuses kmem read/write
For security reasons, some configuration needs to run without /dev/mem but on some occasions, to debug HW for instance, it's still useful to be able to reboot the system with access to physical memory. Add a kernel parameter which activates the /dev/mem device only when 'mem.devmem' is enabled. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- drivers/char/mem.c | 12 ++++++++++++ Documentation/admin-guide/kernel-parameters.txt | 3 +++ drivers/char/Kconfig | 9 +++++++++ 3 files changed, 24 insertions(+)