2012-11-17 00:20:24

by Kees Cook

[permalink] [raw]
Subject: [PATCH] devtmpfs: mount with noexec and nosuid

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: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
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


2012-11-17 00:27:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] devtmpfs: mount with noexec and nosuid

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: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
> ---
> 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

2012-11-17 00:34:39

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] devtmpfs: mount with noexec and nosuid

On Fri, Nov 16, 2012 at 4:27 PM, Greg Kroah-Hartman
<[email protected]> 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: [email protected]
>> Signed-off-by: Kees Cook <[email protected]>
>> ---
>> 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

2012-11-17 00:39:24

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] devtmpfs: mount with noexec and nosuid

On Sat, Nov 17, 2012 at 1:27 AM, Greg Kroah-Hartman
<[email protected]> 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: [email protected]
>> Signed-off-by: Kees Cook <[email protected]>
>> ---
>> 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

2012-11-19 18:14:49

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] devtmpfs: mount with noexec and nosuid

On Fri, Nov 16, 2012 at 4:39 PM, Kay Sievers <[email protected]> wrote:
> On Sat, Nov 17, 2012 at 1:27 AM, Greg Kroah-Hartman
> <[email protected]> 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: [email protected]
>>> Signed-off-by: Kees Cook <[email protected]>
>>> ---
>>> 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

2012-11-20 00:45:21

by Roland Eggner

[permalink] [raw]
Subject: Re: [PATCH] devtmpfs: mount with noexec and nosuid

On 2012-11-19 Monday at 10:14 -0800 Kees Cook wrote:
> On Fri, Nov 16, 2012 at 4:39 PM, Kay Sievers <[email protected]> wrote:
> > On Sat, Nov 17, 2012 at 1:27 AM, Greg Kroah-Hartman
> > <[email protected]> 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: [email protected]
> >>> Signed-off-by: Kees Cook <[email protected]>
> >>> ---
> >>> 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


Attachments:
(No filename) (2.79 kB)
(No filename) (198.00 B)
Download all attachments