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;
Subject: Re: [PATCH] firmware_loader: load files from the mount namespace of
 init
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <bb46ebae-4746-90d9-ec5b-fce4c9328c86@gmail.com>
 <20200117211419.GA2042215@kroah.com>
From:   Topi Miettinen <toiwoton@gmail.com>
Message-ID: <c1a325f1-ad85-11ee-091a-aa179c124eaa@gmail.com>
Date:   Sat, 18 Jan 2020 18:38:04 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20200117211419.GA2042215@kroah.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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