Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 17 Jan 2020 22:14:19 +0100
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Topi Miettinen <toiwoton@gmail.com>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] firmware_loader: load files from the mount namespace of
 init
Message-ID: <20200117211419.GA2042215@kroah.com>
References: <bb46ebae-4746-90d9-ec5b-fce4c9328c86@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <bb46ebae-4746-90d9-ec5b-fce4c9328c86@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 17, 2020 at 08:36:13PM +0200, Topi Miettinen wrote:
> Hi,
> 
> 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.

I like this, but:

> 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.

I think you answered your question of why firmware is loaded from the
namespace of systemd-udevd at times, it happens due to a module being
asked to be loaded which then called out and asked for firmware as part
of its probe process.

Now saying that the firmware load namespace is going to be tied always
to the modprobe namespace is problematic, as we can't guarantee that
will always happen for all bus and driver types.

So resetting this all back to the init namespace seems to make sense to
me, and odds are it will not break anything.

But, as you are adding a new firmware feature, any chance you can write
an additional test to the firmware self-tests so that we can verify that
this really is working the way you are saying it does, so we can trust
it and verify it doesn't break in the future?

thanks,

greg k-h