Re: [PATCH] firmware_loader: load files from the mount namespace of init

[Topi Miettinen]

On 17.1.2020 23.14, Greg Kroah-Hartman wrote:
> 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.

r8169 requests firmware only when opening the device, so the firmware is 
loaded from systemd-networkd namespace.

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

OK, sent v2 of the patch with the tests. They assume a writable 
/lib/firmware, is that OK? Maybe I should change that to overmount it 
temporarily with a writable tmpfs instead.

-Topi