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

[Topi Miettinen <toiwoton@gmail.com>]

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