[PATCH linux dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device

[PATCH linux dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device

[Cédric Le Goater]

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(+)

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

Re: [PATCH linux dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device

[Joel Stanley]

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
>

Re: [PATCH linux dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device

[Andrew Jeffery]

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

Re: [PATCH linux dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device

[Cédric Le Goater]

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. 

Re: [PATCH linux dev-4.18] /dev/mem: add a devmem kernel parameter to activate the device

[Andrew Jeffery]

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.