NAK: [PATCH][Focal linux] devtmpfs: mount with noexec and nosuid

[Tim Gardner]

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 at canonical.com> wrote:
>>
>> From: Kees Cook <keescook at 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 at chromium.org
>> Cc: Kay Sievers <kay at vrfy.org>
>> Cc: Roland Eggner <edvx1 at systemanalysen.net>
>> Co-developed-by: Muhammad Usama Anjum <usama.anjum at collabora.com>
>> Signed-off-by: Kees Cook <keescook at chromium.org>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum at collabora.com>
>> Link: https://lore.kernel.org/r/YcMfDOyrg647RCmd@debian-BULLSEYE-live-builder-AMD64
>> Signed-off-by: Greg Kroah-Hartman <gregkh at 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 at 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 at lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> 
> 
> 

-- 
-----------
Tim Gardner
Canonical, Inc