[PATCH] devtmpfs: mount with noexec and nosuid

[PATCH] devtmpfs: mount with noexec and nosuid

[Kees Cook]

Since devtmpfs is writable, make the default noexec nosuid as well. This
protects from the case of a privileged process having an arbitrary file
write flaw and an argumentless arbitrary execution (i.e. it would lack
the ability to run "mount -o remount,exec,suid /dev"), with a system
that already has nosuid,noexec on all other writable mounts.

Cc: ellyjones@chromium.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/devtmpfs.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 147d1a4..b7e2e57 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -340,6 +340,7 @@ static int handle_remove(const char *nodename, struct device *dev)
 int devtmpfs_mount(const char *mntdir)
 {
 	int err;
+	int mflags = MS_SILENT | MS_NOEXEC | MS_NOSUID;
 
 	if (!mount_dev)
 		return 0;
@@ -347,7 +348,7 @@ int devtmpfs_mount(const char *mntdir)
 	if (!thread)
 		return 0;
 
-	err = sys_mount("devtmpfs", (char *)mntdir, "devtmpfs", MS_SILENT, NULL);
+	err = sys_mount("devtmpfs", (char *)mntdir, "devtmpfs", mflags, NULL);
 	if (err)
 		printk(KERN_INFO "devtmpfs: error mounting %i\n", err);
 	else
@@ -368,11 +369,12 @@ static int handle(const char *name, umode_t mode, struct device *dev)
 static int devtmpfsd(void *p)
 {
 	char options[] = "mode=0755";
+	int mflags = MS_SILENT | MS_NOEXEC | MS_NOSUID;
 	int *err = p;
 	*err = sys_unshare(CLONE_NEWNS);
 	if (*err)
 		goto out;
-	*err = sys_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, options);
+	*err = sys_mount("devtmpfs", "/", "devtmpfs", mflags, options);
 	if (*err)
 		goto out;
 	sys_chdir("/.."); /* will traverse into overmounted root */
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security

Re: [PATCH] devtmpfs: mount with noexec and nosuid

[Greg Kroah-Hartman]

On Fri, Nov 16, 2012 at 04:20:16PM -0800, Kees Cook wrote:
> Since devtmpfs is writable, make the default noexec nosuid as well. This
> protects from the case of a privileged process having an arbitrary file
> write flaw and an argumentless arbitrary execution (i.e. it would lack
> the ability to run "mount -o remount,exec,suid /dev"), with a system
> that already has nosuid,noexec on all other writable mounts.
> 
> Cc: ellyjones@chromium.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  drivers/base/devtmpfs.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Have you tested this to verify that it doesn't break anything?

Kay, could this cause any problems that you could think of?

thanks,

greg k-h

> 
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index 147d1a4..b7e2e57 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -340,6 +340,7 @@ static int handle_remove(const char *nodename, struct device *dev)
>  int devtmpfs_mount(const char *mntdir)
>  {
>  	int err;
> +	int mflags = MS_SILENT | MS_NOEXEC | MS_NOSUID;
>  
>  	if (!mount_dev)
>  		return 0;
> @@ -347,7 +348,7 @@ int devtmpfs_mount(const char *mntdir)
>  	if (!thread)
>  		return 0;
>  
> -	err = sys_mount("devtmpfs", (char *)mntdir, "devtmpfs", MS_SILENT, NULL);
> +	err = sys_mount("devtmpfs", (char *)mntdir, "devtmpfs", mflags, NULL);
>  	if (err)
>  		printk(KERN_INFO "devtmpfs: error mounting %i\n", err);
>  	else
> @@ -368,11 +369,12 @@ static int handle(const char *name, umode_t mode, struct device *dev)
>  static int devtmpfsd(void *p)
>  {
>  	char options[] = "mode=0755";
> +	int mflags = MS_SILENT | MS_NOEXEC | MS_NOSUID;
>  	int *err = p;
>  	*err = sys_unshare(CLONE_NEWNS);
>  	if (*err)
>  		goto out;
> -	*err = sys_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, options);
> +	*err = sys_mount("devtmpfs", "/", "devtmpfs", mflags, options);
>  	if (*err)
>  		goto out;
>  	sys_chdir("/.."); /* will traverse into overmounted root */
> -- 
> 1.7.9.5
> 
> 
> -- 
> Kees Cook
> Chrome OS Security

Re: [PATCH] devtmpfs: mount with noexec and nosuid

[Kees Cook]

On Fri, Nov 16, 2012 at 4:27 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Fri, Nov 16, 2012 at 04:20:16PM -0800, Kees Cook wrote:
>> Since devtmpfs is writable, make the default noexec nosuid as well. This
>> protects from the case of a privileged process having an arbitrary file
>> write flaw and an argumentless arbitrary execution (i.e. it would lack
>> the ability to run "mount -o remount,exec,suid /dev"), with a system
>> that already has nosuid,noexec on all other writable mounts.
>>
>> Cc: ellyjones@chromium.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  drivers/base/devtmpfs.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> Have you tested this to verify that it doesn't break anything?

It doesn't break Chrome OS nor my test VM. The logic for building
/etc/mtab needs updating (it doesn't show nosuid,noexec), but
/proc/mounts reports it correctly.

-Kees

>
> Kay, could this cause any problems that you could think of?
>
> thanks,
>
> greg k-h
>
>>
>> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
>> index 147d1a4..b7e2e57 100644
>> --- a/drivers/base/devtmpfs.c
>> +++ b/drivers/base/devtmpfs.c
>> @@ -340,6 +340,7 @@ static int handle_remove(const char *nodename, struct device *dev)
>>  int devtmpfs_mount(const char *mntdir)
>>  {
>>       int err;
>> +     int mflags = MS_SILENT | MS_NOEXEC | MS_NOSUID;
>>
>>       if (!mount_dev)
>>               return 0;
>> @@ -347,7 +348,7 @@ int devtmpfs_mount(const char *mntdir)
>>       if (!thread)
>>               return 0;
>>
>> -     err = sys_mount("devtmpfs", (char *)mntdir, "devtmpfs", MS_SILENT, NULL);
>> +     err = sys_mount("devtmpfs", (char *)mntdir, "devtmpfs", mflags, NULL);
>>       if (err)
>>               printk(KERN_INFO "devtmpfs: error mounting %i\n", err);
>>       else
>> @@ -368,11 +369,12 @@ static int handle(const char *name, umode_t mode, struct device *dev)
>>  static int devtmpfsd(void *p)
>>  {
>>       char options[] = "mode=0755";
>> +     int mflags = MS_SILENT | MS_NOEXEC | MS_NOSUID;
>>       int *err = p;
>>       *err = sys_unshare(CLONE_NEWNS);
>>       if (*err)
>>               goto out;
>> -     *err = sys_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, options);
>> +     *err = sys_mount("devtmpfs", "/", "devtmpfs", mflags, options);
>>       if (*err)
>>               goto out;
>>       sys_chdir("/.."); /* will traverse into overmounted root */
>> --
>> 1.7.9.5
>>
>>
>> --
>> Kees Cook
>> Chrome OS Security



-- 
Kees Cook
Chrome OS Security

Re: [PATCH] devtmpfs: mount with noexec and nosuid

[Kay Sievers]

On Sat, Nov 17, 2012 at 1:27 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Fri, Nov 16, 2012 at 04:20:16PM -0800, Kees Cook wrote:
>> Since devtmpfs is writable, make the default noexec nosuid as well. This
>> protects from the case of a privileged process having an arbitrary file
>> write flaw and an argumentless arbitrary execution (i.e. it would lack
>> the ability to run "mount -o remount,exec,suid /dev"), with a system
>> that already has nosuid,noexec on all other writable mounts.
>>
>> Cc: ellyjones@chromium.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  drivers/base/devtmpfs.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> Have you tested this to verify that it doesn't break anything?
>
> Kay, could this cause any problems that you could think of?

It breaks all sorts of old, possibly outdated, stuff, that does things
like mapping /dev/mem executable. It for sure used to break X drivers,
that fiddle with the BIOS of cards.

Kay

Re: [PATCH] devtmpfs: mount with noexec and nosuid

[Kees Cook]

On Fri, Nov 16, 2012 at 4:39 PM, Kay Sievers <kay@vrfy.org> wrote:
> On Sat, Nov 17, 2012 at 1:27 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Fri, Nov 16, 2012 at 04:20:16PM -0800, Kees Cook wrote:
>>> Since devtmpfs is writable, make the default noexec nosuid as well. This
>>> protects from the case of a privileged process having an arbitrary file
>>> write flaw and an argumentless arbitrary execution (i.e. it would lack
>>> the ability to run "mount -o remount,exec,suid /dev"), with a system
>>> that already has nosuid,noexec on all other writable mounts.
>>>
>>> Cc: ellyjones@chromium.org
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>>  drivers/base/devtmpfs.c |    6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> Have you tested this to verify that it doesn't break anything?
>>
>> Kay, could this cause any problems that you could think of?
>
> It breaks all sorts of old, possibly outdated, stuff, that does things
> like mapping /dev/mem executable. It for sure used to break X drivers,
> that fiddle with the BIOS of cards.

Ah, yeah, you're totally right. Attempting an mmap with PROT_EXEC on
/dev/mem would be denied.

Is this something we could put behind a CONFIG?

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] devtmpfs: mount with noexec and nosuid

[Roland Eggner]

[-- Attachment #1: Type: text/plain, Size: 2914 bytes --]

On 2012-11-19 Monday at 10:14 -0800 Kees Cook wrote:
> On Fri, Nov 16, 2012 at 4:39 PM, Kay Sievers <kay@vrfy.org> wrote:
> > On Sat, Nov 17, 2012 at 1:27 AM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >> On Fri, Nov 16, 2012 at 04:20:16PM -0800, Kees Cook wrote:
> >>> Since devtmpfs is writable, make the default noexec nosuid as well. This
> >>> protects from the case of a privileged process having an arbitrary file
> >>> write flaw and an argumentless arbitrary execution (i.e. it would lack
> >>> the ability to run "mount -o remount,exec,suid /dev"), with a system
> >>> that already has nosuid,noexec on all other writable mounts.
> >>>
> >>> Cc: ellyjones@chromium.org
> >>> Signed-off-by: Kees Cook <keescook@chromium.org>
> >>> ---
> >>>  drivers/base/devtmpfs.c |    6 ++++--
> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> Have you tested this to verify that it doesn't break anything?
> >>
> >> Kay, could this cause any problems that you could think of?
> >
> > It breaks all sorts of old, possibly outdated, stuff, that does things
> > like mapping /dev/mem executable. It for sure used to break X drivers,
> > that fiddle with the BIOS of cards.
> 
> Ah, yeah, you're totally right. Attempting an mmap with PROT_EXEC on
> /dev/mem would be denied.

Sidenote:  non-executable devtmpfs + nouveau + KMS + xorg works for me:

uname -mrs
..........
Linux 3.2.33-grsecurity.roland.0 x86_64

grep devtmpfs /etc/{fs,m}tab /proc/{$$/mountinfo,mounts}
........................................................
/etc/fstab:devtmpfs     /dev    devtmpfs        rw,noexec,nosuid,size=8m,nr_inodes=16k,mode=0755        0       0
/etc/mtab:devtmpfs /dev devtmpfs rw,noexec,nosuid,size=8m,nr_inodes=16k,mode=0755 0 0
/proc/10358/mountinfo:18 15 0:5 / /dev rw,nosuid,noexec - devtmpfs devtmpfs rw,size=8192k,nr_inodes=16384,mode=755
/proc/mounts:devtmpfs /dev devtmpfs rw,nosuid,noexec,size=8192k,nr_inodes=16384,mode=755 0 0

lspci -d 10de:0a3c -k -nn
.........................
01:00.0 VGA compatible controller [0300]: nVidia Corporation GT216 [Quadro FX 880M] [10de:0a3c] (rev a2)
        Subsystem: Dell Device [1028:040c]
        Kernel driver in use: nouveau

ps -p $( pgrep -d, xinit ) -F
.............................
UID        PID  PPID  C    SZ   RSS PSR STIME TTY          TIME CMD
roland    9514  9478  0 16583   844   2 Nov02 tty6     00:00:00 xinit /etc/X11/xinit/xinitrc -- /usr/bin/X :0 -auth /home/roland/.serverauth.9478
qemu     11486 11463  0 12723   848   0 Nov02 tty30    00:00:00 xinit /etc/X11/xinit/xinitrc -- /usr/bin/X :1 -auth /home/qemu/.serverauth.11463
opera    12273 12240  0  8973   848   3 Nov02 tty18    00:00:00 xinit /etc/X11/xinit/xinitrc -- /usr/bin/X :2 -auth /home/opera/.serverauth.12240


> Is this something we could put behind a CONFIG?

IMHO would be great :)

-- 
Roland

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] devtmpfs: mount with noexec and nosuid

[Muhammad Usama Anjum]

From: Kees Cook <keescook@chromium.org>

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>
Signed-off-by: Kees Cook <keescook@chromium.org>
Co-developed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.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 ffcbe2bc460e..6f04b831a5c0 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -62,6 +62,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 8be352ab4ddb..1e2c2d3882e2 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;
 
 static int __initdata mount_dev = IS_ENABLED(CONFIG_DEVTMPFS_MOUNT);
@@ -363,7 +369,7 @@ int __init devtmpfs_mount(void)
 	if (!thread)
 		return 0;
 
-	err = init_mount("devtmpfs", "dev", "devtmpfs", MS_SILENT, NULL);
+	err = init_mount("devtmpfs", "dev", "devtmpfs", DEVTMPFS_MFLAGS, NULL);
 	if (err)
 		printk(KERN_INFO "devtmpfs: error mounting %i\n", err);
 	else
@@ -412,7 +418,7 @@ static noinline int __init devtmpfs_setup(void *p)
 	err = ksys_unshare(CLONE_NEWNS);
 	if (err)
 		goto out;
-	err = init_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, NULL);
+	err = init_mount("devtmpfs", "/", "devtmpfs", DEVTMPFS_MFLAGS, NULL);
 	if (err)
 		goto out;
 	init_chdir("/.."); /* will traverse into overmounted root */
-- 
2.30.2