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.
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.
-Topi
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
Hi Topi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on linux/master linus/master v5.5-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Topi-Miettinen/firmware_loader-load-files-from-the-mount-namespace-of-init/20200118-100042
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git a37f4958f7b63d2b3cd17a76151fdfc29ce1da5f
config: mips-markeins_defconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
>> ERROR: "kernel_read_file_from_path_initns" [drivers/base/firmware_loader/firmware_class.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
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