| Message ID | 20220428061707.768468-1-tweek@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v3] firmware_loader: use kernel credentials when reading firmware | expand |
On Thu, Apr 28, 2022 at 04:17:07PM +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). > > Previously, Android configurations were not setting up the > firmware_class.path command line argument and were relying on the > userspace fallback mechanism. In this case, the security context of the > userspace daemon (i.e. ueventd) was consistently used to read firmware > files. More Android devices are now found to set firmware_class.path > which gives the kernel the opportunity to read the firmware directly > (via kernel_read_file_from_path_initns). In this scenario, the current > process credentials were used, even if unrelated to the loading of the > firmware file. > > Signed-off-by: Thiébaud Weksteen <tweek@google.com> > --- As stated before, should this go to stable kernels? If so, how far back? And you forgot to cc: John? thanks, greg k-h
> As stated before, should this go to stable kernels? If so, how far > back? Yes, it should go to stable kernels. It should apply cleanly on 5.10. Ideally, up to 4.19 but it will require a fixup. I can send a backported patch for 5.4 and 4.19 branches once it lands in the main tree. > > And you forgot to cc: John? Done Thanks
On Fri, Apr 29, 2022 at 02:54:24PM +1000, Thiébaud Weksteen wrote: > > As stated before, should this go to stable kernels? If so, how far > > back? > > Yes, it should go to stable kernels. It should apply cleanly on 5.10. > Ideally, up to 4.19 but it will require a fixup. I can send a > backported patch for 5.4 and 4.19 branches once it lands in the main > tree. Great, can you resend it with a cc: stable tag added to it? > > And you forgot to cc: John? > > Done On the patch itself? Or just this email? confused, greg k-h
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 94d1789a233e..406a907a4cae 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -735,6 +735,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name, size_t offset, u32 opt_flags) { struct firmware *fw = NULL; + struct cred *kern_cred = NULL; + const struct cred *old_cred; bool nondirect = false; int ret; @@ -751,6 +753,18 @@ _request_firmware(const struct firmware **firmware_p, const char *name, if (ret <= 0) /* error or already assigned */ goto out; + /* + * We are about to try to access the firmware file. Because we may have been + * called by a driver when serving an unrelated request from userland, we use + * the kernel credentials to read the file. + */ + kern_cred = prepare_kernel_cred(NULL); + if (!kern_cred) { + ret = -ENOMEM; + goto out; + } + old_cred = override_creds(kern_cred); + ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL); /* Only full reads can support decompression, platform, and sysfs. */ @@ -776,6 +790,9 @@ _request_firmware(const struct firmware **firmware_p, const char *name, } else ret = assign_fw(fw, device); + revert_creds(old_cred); + put_cred(kern_cred); + out: if (ret < 0) { fw_abort_batch_reqs(fw);
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). Previously, Android configurations were not setting up the firmware_class.path command line argument and were relying on the userspace fallback mechanism. In this case, the security context of the userspace daemon (i.e. ueventd) was consistently used to read firmware files. More Android devices are now found to set firmware_class.path which gives the kernel the opportunity to read the firmware directly (via kernel_read_file_from_path_initns). In this scenario, the current process credentials were used, even if unrelated to the loading of the firmware file. Signed-off-by: Thiébaud Weksteen <tweek@google.com> --- v3: - Add call to put_cred to avoid a memory leak. Confirmed that no new memory leak occurs on a Pixel 4a. - Update commit log. v2: Add comment drivers/base/firmware_loader/main.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)