diff mbox series

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

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

Commit Message

Tim Gardner Oct. 11, 2022, 2:19 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. Added config
change for DEVTMPFS_SAFE=y ]
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 debian.aws/config/config.common.ubuntu |  1 +
 drivers/base/Kconfig                   | 11 +++++++++++
 drivers/base/devtmpfs.c                | 10 ++++++++--
 3 files changed, 20 insertions(+), 2 deletions(-)

Comments

Dimitri John Ledkov Oct. 11, 2022, 3:37 p.m. UTC | #1 
This needs to go into all kernels that have a potential to boot
without initramfs, i.e. linux-kvm linux-gcp linux-azure linux-aws and
so on and so forth.

Ditto the config option should be enforced and changed in basically
all the kernels.

I will double check if we can additionally add this elsewhere in the
initramfs-tools and/or systemd, to ensure this is in place even more.

On Tue, 11 Oct 2022 at 15:21, 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. Added config
> change for DEVTMPFS_SAFE=y ]
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  debian.aws/config/config.common.ubuntu |  1 +
>  drivers/base/Kconfig                   | 11 +++++++++++
>  drivers/base/devtmpfs.c                | 10 ++++++++--
>  3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/debian.aws/config/config.common.ubuntu b/debian.aws/config/config.common.ubuntu
> index edf88a75f8a9..f379c9b47ac6 100644
> --- a/debian.aws/config/config.common.ubuntu
> +++ b/debian.aws/config/config.common.ubuntu
> @@ -1930,6 +1930,7 @@ CONFIG_DEVMEM=y
>  CONFIG_DEVPORT=y
>  CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
> +CONFIG_DEVTMPFS_SAFE=y
>  CONFIG_DEV_APPLETALK=m
>  CONFIG_DEV_COREDUMP=y
>  CONFIG_DEV_DAX=m
> 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 */
> --
> 2.34.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Tim Gardner Oct. 11, 2022, 3:40 p.m. UTC | #2 
I'll resumbit with the config change split out and annotation 
enforcement added.

rtg

On 10/11/22 9:37 AM, Dimitri John Ledkov wrote:
> This needs to go into all kernels that have a potential to boot
> without initramfs, i.e. linux-kvm linux-gcp linux-azure linux-aws and
> so on and so forth.
> 
> Ditto the config option should be enforced and changed in basically
> all the kernels.
> 
> I will double check if we can additionally add this elsewhere in the
> initramfs-tools and/or systemd, to ensure this is in place even more.
> 
> On Tue, 11 Oct 2022 at 15:21, 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. Added config
>> change for DEVTMPFS_SAFE=y ]
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>> ---
>>   debian.aws/config/config.common.ubuntu |  1 +
>>   drivers/base/Kconfig                   | 11 +++++++++++
>>   drivers/base/devtmpfs.c                | 10 ++++++++--
>>   3 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/debian.aws/config/config.common.ubuntu b/debian.aws/config/config.common.ubuntu
>> index edf88a75f8a9..f379c9b47ac6 100644
>> --- a/debian.aws/config/config.common.ubuntu
>> +++ b/debian.aws/config/config.common.ubuntu
>> @@ -1930,6 +1930,7 @@ CONFIG_DEVMEM=y
>>   CONFIG_DEVPORT=y
>>   CONFIG_DEVTMPFS=y
>>   CONFIG_DEVTMPFS_MOUNT=y
>> +CONFIG_DEVTMPFS_SAFE=y
>>   CONFIG_DEV_APPLETALK=m
>>   CONFIG_DEV_COREDUMP=y
>>   CONFIG_DEV_DAX=m
>> 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 */
>> --
>> 2.34.1
>>
>>
>> --
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> 
> 
>
Dave Chiluk Oct. 12, 2022, 7:34 p.m. UTC | #3 
This is the dark magic that prevented me from creating actual patches
for this in the first place.

Does all this just live in the debian.{master,hwe*}/config
directories?  By what mechanism does that get pushed into a config
used during build?

Dave.

On Tue, Oct 11, 2022 at 10:41 AM Tim Gardner <tim.gardner@canonical.com> wrote:
>
> I'll resumbit with the config change split out and annotation
> enforcement added.
>
> rtg
>
> On 10/11/22 9:37 AM, Dimitri John Ledkov wrote:
> > This needs to go into all kernels that have a potential to boot
> > without initramfs, i.e. linux-kvm linux-gcp linux-azure linux-aws and
> > so on and so forth.
> >
> > Ditto the config option should be enforced and changed in basically
> > all the kernels.
> >
> > I will double check if we can additionally add this elsewhere in the
> > initramfs-tools and/or systemd, to ensure this is in place even more.
> >
> > On Tue, 11 Oct 2022 at 15:21, 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. Added config
> >> change for DEVTMPFS_SAFE=y ]
> >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> >> ---
> >>   debian.aws/config/config.common.ubuntu |  1 +
> >>   drivers/base/Kconfig                   | 11 +++++++++++
> >>   drivers/base/devtmpfs.c                | 10 ++++++++--
> >>   3 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/debian.aws/config/config.common.ubuntu b/debian.aws/config/config.common.ubuntu
> >> index edf88a75f8a9..f379c9b47ac6 100644
> >> --- a/debian.aws/config/config.common.ubuntu
> >> +++ b/debian.aws/config/config.common.ubuntu
> >> @@ -1930,6 +1930,7 @@ CONFIG_DEVMEM=y
> >>   CONFIG_DEVPORT=y
> >>   CONFIG_DEVTMPFS=y
> >>   CONFIG_DEVTMPFS_MOUNT=y
> >> +CONFIG_DEVTMPFS_SAFE=y
> >>   CONFIG_DEV_APPLETALK=m
> >>   CONFIG_DEV_COREDUMP=y
> >>   CONFIG_DEV_DAX=m
> >> 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 */
> >> --
> >> 2.34.1
> >>
> >>
> >> --
> >> kernel-team mailing list
> >> kernel-team@lists.ubuntu.com
> >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> >
> >
> >
>
> --
> -----------
> Tim Gardner
> Canonical, Inc
>
Tim Gardner Oct. 12, 2022, 9:14 p.m. UTC | #4 
Honestly, there isn't enough space in an email to explain how the 
configuration system works :)

You can, however, generate the classic config files by running:

fakeroot debian/rules clean genconfigs

Then look in CONFIGS to find the right flavor and architecture config file.

cp CONFIGS/amd64-config.flavour.generic .config
make -j$(nproc)

This skips the packaging phase if all you're interested in is a test build.

rtg

On 10/12/22 13:34, Dave Chiluk wrote:
> This is the dark magic that prevented me from creating actual patches
> for this in the first place.
> 
> Does all this just live in the debian.{master,hwe*}/config
> directories?  By what mechanism does that get pushed into a config
> used during build?
> 
> Dave.
> 
> On Tue, Oct 11, 2022 at 10:41 AM Tim Gardner <tim.gardner@canonical.com> wrote:
>>
>> I'll resumbit with the config change split out and annotation
>> enforcement added.
>>
>> rtg
>>
>> On 10/11/22 9:37 AM, Dimitri John Ledkov wrote:
>>> This needs to go into all kernels that have a potential to boot
>>> without initramfs, i.e. linux-kvm linux-gcp linux-azure linux-aws and
>>> so on and so forth.
>>>
>>> Ditto the config option should be enforced and changed in basically
>>> all the kernels.
>>>
>>> I will double check if we can additionally add this elsewhere in the
>>> initramfs-tools and/or systemd, to ensure this is in place even more.
>>>
>>> On Tue, 11 Oct 2022 at 15:21, 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. Added config
>>>> change for DEVTMPFS_SAFE=y ]
>>>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>>>> ---
>>>>    debian.aws/config/config.common.ubuntu |  1 +
>>>>    drivers/base/Kconfig                   | 11 +++++++++++
>>>>    drivers/base/devtmpfs.c                | 10 ++++++++--
>>>>    3 files changed, 20 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/debian.aws/config/config.common.ubuntu b/debian.aws/config/config.common.ubuntu
>>>> index edf88a75f8a9..f379c9b47ac6 100644
>>>> --- a/debian.aws/config/config.common.ubuntu
>>>> +++ b/debian.aws/config/config.common.ubuntu
>>>> @@ -1930,6 +1930,7 @@ CONFIG_DEVMEM=y
>>>>    CONFIG_DEVPORT=y
>>>>    CONFIG_DEVTMPFS=y
>>>>    CONFIG_DEVTMPFS_MOUNT=y
>>>> +CONFIG_DEVTMPFS_SAFE=y
>>>>    CONFIG_DEV_APPLETALK=m
>>>>    CONFIG_DEV_COREDUMP=y
>>>>    CONFIG_DEV_DAX=m
>>>> 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 */
>>>> --
>>>> 2.34.1
>>>>
>>>>
>>>> --
>>>> kernel-team mailing list
>>>> kernel-team@lists.ubuntu.com
>>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>>>
>>>
>>>
>>
>> --
>> -----------
>> Tim Gardner
>> Canonical, Inc
>>
diff mbox series

Patch

diff --git a/debian.aws/config/config.common.ubuntu b/debian.aws/config/config.common.ubuntu
index edf88a75f8a9..f379c9b47ac6 100644
--- a/debian.aws/config/config.common.ubuntu
+++ b/debian.aws/config/config.common.ubuntu
@@ -1930,6 +1930,7 @@  CONFIG_DEVMEM=y
 CONFIG_DEVPORT=y
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_DEVTMPFS_SAFE=y
 CONFIG_DEV_APPLETALK=m
 CONFIG_DEV_COREDUMP=y
 CONFIG_DEV_DAX=m
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 */