linux-kernel - Re: [PATCH] firmware_loader: use kernel credentials when reading firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+zpnLfA7-eWb8dO3=VsOzyzUa8ycZxmxG9T2bkWVOcFVp4gKQ@mail.gmail.com>
Date:   Tue, 5 Apr 2022 12:23:59 +1000
From:   Thiébaud Weksteen <tweek@...gle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Luis Chamberlain <mcgrof@...nel.org>,
        Jeffrey Vander Stoep <jeffv@...gle.com>,
        Saravana Kannan <saravanak@...gle.com>,
        Alistair Delva <adelva@...gle.com>,
        Adam Shih <adamshih@...gle.com>,
        SElinux list <selinux@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] firmware_loader: use kernel credentials when reading firmware

On Mon, Apr 4, 2022 at 6:33 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Mon, Apr 04, 2022 at 03:46:42PM +1000, Thiébaud Weksteen wrote:
> > Device drivers may decide to not load firmware when probed to avoid
> > slowing down the boot process should the firmware filesystem not be
> > available yet. In this case, the firmware loading request may be done
> > when a device file associated with the driver is first accessed. The
> > credentials of the userspace process accessing the device file may be
> > used to validate access to the firmware files requested by the driver.
> > Ensure that the kernel assumes the responsibility of reading the
> > firmware.
> >
> > This was observed on Android for a graphic driver loading their firmware
> > when the device file (e.g. /dev/mali0) was first opened by userspace
> > (i.e. surfaceflinger). The security context of surfaceflinger was used
> > to validate the access to the firmware file (e.g.
> > /vendor/firmware/mali.bin).
> >
> > Because previous configurations were relying on the userspace fallback
> > mechanism, the security context of the userspace daemon (i.e. ueventd)
> > was consistently used to read firmware files. More devices are found to
> > use the command line argument firmware_class.path which gives the kernel
> > the opportunity to read the firmware directly, hence surfacing this
> > misattribution.
> >
> > Signed-off-by: Thiébaud Weksteen <tweek@...gle.com>
> > ---
> >  drivers/base/firmware_loader/main.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
>
> Is this a bugfix?  if so, what commit does this fix?  If not, how has
> this never been a problem in the past (i.e. what changed to cause
> problems?)

Not a bug fix. I think this is the combination of 3 factors that
surfaces the issue:

1. As mentioned in the last paragraph of the commit, previously, the
userspace fallback was used (that is, udev/ueventd would read the
firmware file so only this process would need the correct permission
to access the files). More devices are using firmware_class.path=
which causes the kernel to try to load the firmware directly[1].

2. Drivers are calling request_firmware when handling a userspace
request (such as open() on a device file). Historically,
request_firmware seems to have been called when probe() was invoked by
the kernel.

3. The precise MAC policy on Android (as opposed to DAC where the
firmware files are own by root and a root process accesses the device
file).

Thanks,

[1] https://www.kernel.org/doc/html/v5.17/driver-api/firmware/fw_search_path.html

> thanks,
>
> greg k-h