linux-kernel - Re: [PATCH v3] firmware_loader: load files from the mount namespace of init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200123110359.GA922639@kroah.com>
Date:   Thu, 23 Jan 2020 12:03:59 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Topi Miettinen <toiwoton@...il.com>
Cc:     Luis Chamberlain <mcgrof@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] firmware_loader: load files from the mount namespace
 of init

On Thu, Jan 23, 2020 at 12:35:06PM +0200, Topi Miettinen wrote:
> I have an experimental setup where almost every possible system
> service (even early startup ones) runs in separate namespace, using a
> dedicated, minimal file system. In process of minimizing the contents
> of the file systems with regards to modules and firmware files, I
> noticed that in my system, the firmware files are loaded from three
> different mount namespaces, those of systemd-udevd, init and
> systemd-networkd. The logic of the source namespace is not very clear,
> it seems to depend on the driver, but the namespace of the current
> process is used.
> 
> So, this patch tries to make things a bit clearer and changes the
> loading of firmware files only from the mount namespace of init. This
> may also improve security, though I think that using firmware files as
> attack vector could be too impractical anyway.
> 
> Later, it might make sense to make the mount namespace configurable,
> for example with a new file in /proc/sys/kernel/firmware_config/. That
> would allow a dedicated file system only for firmware files and those
> need not be present anywhere else. This configurability would make
> more sense if made also for kernel modules and /sbin/modprobe. Modules
> are already loaded from init namespace (usermodehelper uses kthreadd
> namespace) except when directly loaded by systemd-udevd.
> 
> Instead of using the mount namespace of the current process to load
> firmware files, use the mount namespace of init process.
> 
> Link:
> https://lore.kernel.org/lkml/bb46ebae-4746-90d9-ec5b-fce4c9328c86@gmail.com/
> Link:
> https://lore.kernel.org/lkml/0e3f7653-c59d-9341-9db2-c88f5b988c68@gmail.com/
> Signed-off-by: Topi Miettinen <toiwoton@...il.com>
> ---
> Changelog:
> v1->v2: added selfests
> v2->v3: added comments, don't assume writable /lib/firmware
> ---
>  drivers/base/firmware_loader/main.c           |   6 +-
>  fs/exec.c                                     |  26 +++
>  include/linux/fs.h                            |   2 +
>  tools/testing/selftests/firmware/Makefile     |   9 +-
>  .../testing/selftests/firmware/fw_namespace.c | 151 ++++++++++++++++++
>  .../selftests/firmware/fw_run_tests.sh        |   4 +
>  6 files changed, 190 insertions(+), 8 deletions(-)
>  create mode 100644 tools/testing/selftests/firmware/fw_namespace.c
> 
> diff --git a/drivers/base/firmware_loader/main.c
> b/drivers/base/firmware_loader/main.c
> index 249add8c5e05..01f5315fae53 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -493,8 +493,10 @@ fw_get_filesystem_firmware(struct device *device,
> struct fw_priv *fw_priv,
>                 }
> 
>                 fw_priv->size = 0;
> -               rc = kernel_read_file_from_path(path, &buffer, &size,
> -                                               msize, id);

Something happened to your patch :(

It is line-wrapped and all tabs are turned into spaces, making it
impossible to apply.  You can't use the web interface of gmail for
patches, as it corrupts them as is shown here.

Can you just use 'git send-email' instead?

thanks,

greg k-h