diff mbox series

[1/2,Focal,linux] devtmpfs: mount with noexec and nosuid

Message ID 20221011162912.47796-2-tim.gardner@canonical.com
State New
Headers show
Series [1/2,Focal,linux] devtmpfs: mount with noexec and nosuid | expand

Commit Message

Tim Gardner Oct. 11, 2022, 4:29 p.m. UTC
From: Kees Cook <keescook@chromium.org>

BugLink: https://bugs.launchpad.net/bugs/1991975

devtmpfs is writable. Add the noexec and nosuid as default mount flags
to prevent code execution from /dev. The systems who don't use systemd
and who rely on CONFIG_DEVTMPFS_MOUNT=y are the ones to be protected by
this patch. Other systems are fine with the udev solution.

No sane program should be relying on executing from /dev. So this patch
reduces the attack surface. It doesn't prevent any specific attack, but
it reduces the possibility that someone can use /dev as a place to put
executable code. Chrome OS has been carrying this patch for several
years. It seems trivial and simple solution to improve the protection of
/dev when CONFIG_DEVTMPFS_MOUNT=y.

Original patch:
https://lore.kernel.org/lkml/20121120215059.GA1859@www.outflux.net/

Cc: ellyjones@chromium.org
Cc: Kay Sievers <kay@vrfy.org>
Cc: Roland Eggner <edvx1@systemanalysen.net>
Co-developed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Link: https://lore.kernel.org/r/YcMfDOyrg647RCmd@debian-BULLSEYE-live-builder-AMD64
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(backported from commit 28f0c335dd4a1a4b44b3e6c6402825a93132e1a4)
[rtg - Use ksys_mount() because init_mount() does not yet exist. ]
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/base/Kconfig    | 11 +++++++++++
 drivers/base/devtmpfs.c | 10 ++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

Comments

Dimitri John Ledkov Oct. 12, 2022, 9:03 p.m. UTC | #1
Futher discussion in
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1991975

shows that systemd upstream has dropped mounting /dev with noexec,
because that breaks SGX see
https://github.com/systemd/systemd/commit/4eb105fa4aae30566d23382e8c9430eddf1a3dd4
and also

ditto initramfs-tools mount /dev with nosuid without noexec, see
https://salsa.debian.org/kernel-team/initramfs-tools/-/commit/33c10ef43b03dc6d9ee09a46c598f6ee34ad0b81
 & https://lore.kernel.org/linux-sgx/20201209000321.GA62845@kernel.org/T/

imho all pieces should be consistent, meaning kernel should mount /dev
with nosuid, but without noexec as well.

On Tue, 11 Oct 2022 at 17:30, Tim Gardner <tim.gardner@canonical.com> wrote:
>
> From: Kees Cook <keescook@chromium.org>
>
> BugLink: https://bugs.launchpad.net/bugs/1991975
>
> devtmpfs is writable. Add the noexec and nosuid as default mount flags
> to prevent code execution from /dev. The systems who don't use systemd
> and who rely on CONFIG_DEVTMPFS_MOUNT=y are the ones to be protected by
> this patch. Other systems are fine with the udev solution.
>
> No sane program should be relying on executing from /dev. So this patch
> reduces the attack surface. It doesn't prevent any specific attack, but
> it reduces the possibility that someone can use /dev as a place to put
> executable code. Chrome OS has been carrying this patch for several
> years. It seems trivial and simple solution to improve the protection of
> /dev when CONFIG_DEVTMPFS_MOUNT=y.
>
> Original patch:
> https://lore.kernel.org/lkml/20121120215059.GA1859@www.outflux.net/
>
> Cc: ellyjones@chromium.org
> Cc: Kay Sievers <kay@vrfy.org>
> Cc: Roland Eggner <edvx1@systemanalysen.net>
> Co-developed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Link: https://lore.kernel.org/r/YcMfDOyrg647RCmd@debian-BULLSEYE-live-builder-AMD64
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> (backported from commit 28f0c335dd4a1a4b44b3e6c6402825a93132e1a4)
> [rtg - Use ksys_mount() because init_mount() does not yet exist. ]
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  drivers/base/Kconfig    | 11 +++++++++++
>  drivers/base/devtmpfs.c | 10 ++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 28b92e3cc570..94077975cbd9 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -59,6 +59,17 @@ config DEVTMPFS_MOUNT
>           rescue mode with init=/bin/sh, even when the /dev directory
>           on the rootfs is completely empty.
>
> +config DEVTMPFS_SAFE
> +       bool "Use nosuid,noexec mount options on devtmpfs"
> +       depends on DEVTMPFS
> +       help
> +         This instructs the kernel to include the MS_NOEXEC and MS_NOSUID mount
> +         flags when mounting devtmpfs.
> +
> +         Notice: If enabled, things like /dev/mem cannot be mmapped
> +         with the PROT_EXEC flag. This can break, for example, non-KMS
> +         video drivers.
> +
>  config STANDALONE
>         bool "Select only drivers that don't need compile-time external firmware"
>         default y
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index 5e9b00711357..82fc8ea81c4b 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -29,6 +29,12 @@
>  #include <uapi/linux/mount.h>
>  #include "base.h"
>
> +#ifdef CONFIG_DEVTMPFS_SAFE
> +#define DEVTMPFS_MFLAGS       (MS_SILENT | MS_NOEXEC | MS_NOSUID)

I would prefer to drop MS_NOEXEC from this define, i guess as an
UBUNTU SAUCE patch referencing above mentioned linux-sgx ,
initramfs-tools, systemd URLs.

> +#else
> +#define DEVTMPFS_MFLAGS       (MS_SILENT)
> +#endif
> +
>  static struct task_struct *thread;
>
>  #if defined CONFIG_DEVTMPFS_MOUNT
> @@ -377,7 +383,7 @@ int devtmpfs_mount(const char *mntdir)
>         if (!thread)
>                 return 0;
>
> -       err = ksys_mount("devtmpfs", mntdir, "devtmpfs", MS_SILENT, NULL);
> +       err = ksys_mount("devtmpfs", mntdir, "devtmpfs", DEVTMPFS_MFLAGS, NULL);
>         if (err)
>                 printk(KERN_INFO "devtmpfs: error mounting %i\n", err);
>         else
> @@ -402,7 +408,7 @@ static int devtmpfsd(void *p)
>         *err = ksys_unshare(CLONE_NEWNS);
>         if (*err)
>                 goto out;
> -       *err = ksys_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, NULL);
> +       *err = ksys_mount("devtmpfs", "/", "devtmpfs", DEVTMPFS_MFLAGS, NULL);
>         if (*err)
>                 goto out;
>         ksys_chdir("/.."); /* will traverse into overmounted root */
> --
> 2.34.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 28b92e3cc570..94077975cbd9 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -59,6 +59,17 @@  config DEVTMPFS_MOUNT
 	  rescue mode with init=/bin/sh, even when the /dev directory
 	  on the rootfs is completely empty.
 
+config DEVTMPFS_SAFE
+	bool "Use nosuid,noexec mount options on devtmpfs"
+	depends on DEVTMPFS
+	help
+	  This instructs the kernel to include the MS_NOEXEC and MS_NOSUID mount
+	  flags when mounting devtmpfs.
+
+	  Notice: If enabled, things like /dev/mem cannot be mmapped
+	  with the PROT_EXEC flag. This can break, for example, non-KMS
+	  video drivers.
+
 config STANDALONE
 	bool "Select only drivers that don't need compile-time external firmware"
 	default y
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 5e9b00711357..82fc8ea81c4b 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -29,6 +29,12 @@ 
 #include <uapi/linux/mount.h>
 #include "base.h"
 
+#ifdef CONFIG_DEVTMPFS_SAFE
+#define DEVTMPFS_MFLAGS       (MS_SILENT | MS_NOEXEC | MS_NOSUID)
+#else
+#define DEVTMPFS_MFLAGS       (MS_SILENT)
+#endif
+
 static struct task_struct *thread;
 
 #if defined CONFIG_DEVTMPFS_MOUNT
@@ -377,7 +383,7 @@  int devtmpfs_mount(const char *mntdir)
 	if (!thread)
 		return 0;
 
-	err = ksys_mount("devtmpfs", mntdir, "devtmpfs", MS_SILENT, NULL);
+	err = ksys_mount("devtmpfs", mntdir, "devtmpfs", DEVTMPFS_MFLAGS, NULL);
 	if (err)
 		printk(KERN_INFO "devtmpfs: error mounting %i\n", err);
 	else
@@ -402,7 +408,7 @@  static int devtmpfsd(void *p)
 	*err = ksys_unshare(CLONE_NEWNS);
 	if (*err)
 		goto out;
-	*err = ksys_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, NULL);
+	*err = ksys_mount("devtmpfs", "/", "devtmpfs", DEVTMPFS_MFLAGS, NULL);
 	if (*err)
 		goto out;
 	ksys_chdir("/.."); /* will traverse into overmounted root */