* [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
@ 2025-04-28 19:09 Eric Biggers
2025-04-29 5:54 ` Greg Kroah-Hartman
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Eric Biggers @ 2025-04-28 19:09 UTC (permalink / raw)
To: Luis Chamberlain, Russ Weight, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel
Cc: linux-crypto, Arnd Bergmann, Amadeusz Sławiński
From: Eric Biggers <ebiggers@google.com>
This user of SHA-256 does not support any other algorithm, so the
crypto_shash abstraction provides no value. Just use the SHA-256
library API instead, which is much simpler and easier to use.
Also take advantage of printk's built-in hex conversion using %*phN.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
This patch is targeting the firmware_loader tree for 6.16.
drivers/base/firmware_loader/Kconfig | 4 +---
drivers/base/firmware_loader/main.c | 34 ++++------------------------
2 files changed, 5 insertions(+), 33 deletions(-)
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index a037016742651..752b9a9bea038 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -1,12 +1,11 @@
# SPDX-License-Identifier: GPL-2.0
menu "Firmware loader"
config FW_LOADER
tristate "Firmware loading facility" if EXPERT
- select CRYPTO_HASH if FW_LOADER_DEBUG
- select CRYPTO_SHA256 if FW_LOADER_DEBUG
+ select CRYPTO_LIB_SHA256 if FW_LOADER_DEBUG
default y
help
This enables the firmware loading facility in the kernel. The kernel
will first look for built-in firmware, if it has any. Next, it will
look for the requested firmware in a series of filesystem paths:
@@ -26,11 +25,10 @@ config FW_LOADER
You also want to be sure to enable this built-in if you are going to
enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
config FW_LOADER_DEBUG
bool "Log filenames and checksums for loaded firmware"
- depends on CRYPTO = FW_LOADER || CRYPTO=y
depends on DYNAMIC_DEBUG
depends on FW_LOADER
default FW_LOADER
help
Select this option to use dynamic debug to log firmware filenames and
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index cb0912ea3e627..44486b2c71728 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -804,45 +804,19 @@ static void fw_abort_batch_reqs(struct firmware *fw)
fw_state_aborted(fw_priv);
mutex_unlock(&fw_lock);
}
#if defined(CONFIG_FW_LOADER_DEBUG)
-#include <crypto/hash.h>
#include <crypto/sha2.h>
static void fw_log_firmware_info(const struct firmware *fw, const char *name, struct device *device)
{
- struct shash_desc *shash;
- struct crypto_shash *alg;
- u8 *sha256buf;
- char *outbuf;
+ u8 digest[SHA256_DIGEST_SIZE];
- alg = crypto_alloc_shash("sha256", 0, 0);
- if (IS_ERR(alg))
- return;
-
- sha256buf = kmalloc(SHA256_DIGEST_SIZE, GFP_KERNEL);
- outbuf = kmalloc(SHA256_BLOCK_SIZE + 1, GFP_KERNEL);
- shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(alg), GFP_KERNEL);
- if (!sha256buf || !outbuf || !shash)
- goto out_free;
-
- shash->tfm = alg;
-
- if (crypto_shash_digest(shash, fw->data, fw->size, sha256buf) < 0)
- goto out_free;
-
- for (int i = 0; i < SHA256_DIGEST_SIZE; i++)
- sprintf(&outbuf[i * 2], "%02x", sha256buf[i]);
- outbuf[SHA256_BLOCK_SIZE] = 0;
- dev_dbg(device, "Loaded FW: %s, sha256: %s\n", name, outbuf);
-
-out_free:
- kfree(shash);
- kfree(outbuf);
- kfree(sha256buf);
- crypto_free_shash(alg);
+ sha256(fw->data, fw->size, digest);
+ dev_dbg(device, "Loaded FW: %s, sha256: %*phN\n",
+ name, SHA256_DIGEST_SIZE, digest);
}
#else
static void fw_log_firmware_info(const struct firmware *fw, const char *name,
struct device *device)
{}
base-commit: 33035b665157558254b3c21c3f049fd728e72368
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
2025-04-28 19:09 [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API Eric Biggers
@ 2025-04-29 5:54 ` Greg Kroah-Hartman
2025-04-29 16:42 ` Eric Biggers
2025-04-30 20:08 ` Danilo Krummrich
2026-03-06 16:37 ` Youssef Samir
2 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2025-04-29 5:54 UTC (permalink / raw)
To: Eric Biggers
Cc: Luis Chamberlain, Russ Weight, Danilo Krummrich,
Rafael J . Wysocki, linux-kernel, linux-crypto, Arnd Bergmann,
Amadeusz Sławiński
On Mon, Apr 28, 2025 at 12:09:09PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> This user of SHA-256 does not support any other algorithm, so the
> crypto_shash abstraction provides no value. Just use the SHA-256
> library API instead, which is much simpler and easier to use.
>
> Also take advantage of printk's built-in hex conversion using %*phN.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>
> This patch is targeting the firmware_loader tree for 6.16.
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
2025-04-29 5:54 ` Greg Kroah-Hartman
@ 2025-04-29 16:42 ` Eric Biggers
0 siblings, 0 replies; 7+ messages in thread
From: Eric Biggers @ 2025-04-29 16:42 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Luis Chamberlain, Russ Weight, Danilo Krummrich,
Rafael J . Wysocki, linux-kernel, linux-crypto, Arnd Bergmann,
Amadeusz Sławiński
On Tue, Apr 29, 2025 at 07:54:42AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 28, 2025 at 12:09:09PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > This user of SHA-256 does not support any other algorithm, so the
> > crypto_shash abstraction provides no value. Just use the SHA-256
> > library API instead, which is much simpler and easier to use.
> >
> > Also take advantage of printk's built-in hex conversion using %*phN.
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >
> > This patch is targeting the firmware_loader tree for 6.16.
>
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Greg, it looks like you normally take patches to this file. You can go ahead
and take it if you want. Thanks!
- Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
2025-04-28 19:09 [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API Eric Biggers
2025-04-29 5:54 ` Greg Kroah-Hartman
@ 2025-04-30 20:08 ` Danilo Krummrich
2026-03-06 16:37 ` Youssef Samir
2 siblings, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2025-04-30 20:08 UTC (permalink / raw)
To: Eric Biggers
Cc: Luis Chamberlain, Russ Weight, Greg Kroah-Hartman,
Rafael J . Wysocki, linux-kernel, linux-crypto, Arnd Bergmann,
Amadeusz Sławiński
On Mon, Apr 28, 2025 at 12:09:09PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> This user of SHA-256 does not support any other algorithm, so the
> crypto_shash abstraction provides no value. Just use the SHA-256
> library API instead, which is much simpler and easier to use.
>
> Also take advantage of printk's built-in hex conversion using %*phN.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Applied to driver-core-testing, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
2025-04-28 19:09 [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API Eric Biggers
2025-04-29 5:54 ` Greg Kroah-Hartman
2025-04-30 20:08 ` Danilo Krummrich
@ 2026-03-06 16:37 ` Youssef Samir
2026-03-06 21:20 ` Eric Biggers
2 siblings, 1 reply; 7+ messages in thread
From: Youssef Samir @ 2026-03-06 16:37 UTC (permalink / raw)
To: ebiggers
Cc: amadeuszx.slawinski, arnd, dakr, gregkh, linux-crypto,
linux-kernel, mcgrof, rafael, russ.weight, jeff.hugo, troy.hanson
On 4/28/2025 8:09 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> This user of SHA-256 does not support any other algorithm, so the
> crypto_shash abstraction provides no value. Just use the SHA-256
> library API instead, which is much simpler and easier to use.
>
> Also take advantage of printk's built-in hex conversion using %*phN.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>
> This patch is targeting the firmware_loader tree for 6.16.
Hi Eric,
An issue has been found on kernel versions older than v6.16, where a firmware
file larger than 2GB and is not divisible by SHA256_BLOCK_SIZE (64b) will always lead to a page fault. The first size that fits this criteria is 2147483649b. It is also worth noting that any subsequent loads regardless of the size or divisibility by 64, will lead to another page fault.
I've mainly tested this with drivers/accel/qaic on 6.8.0-62-generic, but technically this should affect any code that uses the firmware loader on a kernel version older than v6.16 with CONFIG_FW_LOADER_DEBUG enabled, including the stable kernels.
This can be reproduced by creating a dummy binary file of a size that fits the criteria listed above, then compress it using zstd to allow _request_firmware() to open it.
This patch appears to have fixed the issue so I suggest backporting it, but
I also noticed that it relies on changes that were introduced in this series:
https://lore.kernel.org/lkml/cover.1745734678.git.herbert@gondor.apana.org.au/
Below is the BUG splat:
[1667258.914177] BUG: unable to handle page fault for address: ffffb731b3fbcd40
[1667258.914188] #PF: supervisor read access in kernel mode
[1667258.914193] #PF: error_code(0x0000) - not-present page
[1667258.914198] PGD 100000067 P4D 100000067 PUD 1002d4067 PMD 529eec067 PTE 0
[1667258.914208] Oops: 0000 [#3] PREEMPT SMP PTI
[1667258.914214] CPU: 11 PID: 1252644 Comm: kworker/11:1 Tainted: P D W OE 6.8.0-62-generic #65-Ubuntu
[1667258.914223] Hardware name: Wiwynn Twin Lakes MP/Twin Lakes Passive MP, BIOS YMGPE07 12/23/2019
[1667258.914229] Workqueue: events sahara_processing [qaic]
[1667258.914257] RIP: 0010:memcpy_orig+0x105/0x130
[1667258.914267] Code: 0f 1f 44 00 00 83 fa 04 72 1b 8b 0e 44 8b 44 16 fc 89 0f 44 89 44 17 fc c3 cc cc cc cc 0f 1f 84 00 00 00 00 00 83 ea 01 72 19 <0f> b6 0e 74 12 4c 0f b6 46 01 4c 0f b6 0c 16 44 88 47 01 44 88 0c
[1667258.914278] RSP: 0018:ffffb731a3c57c78 EFLAGS: 00010202
[1667258.914284] RAX: ffffa0ac564f41b0 RBX: ffffa0ac564f41b0 RCX: 00000000d7af7212
[1667258.914290] RDX: 0000000000000001 RSI: ffffb731b3fbcd40 RDI: ffffa0ac564f41b0
[1667258.914295] RBP: ffffb731a3c57ca8 R08: 000000005ab6c582 R09: 0000000072a12f7b
[1667258.914301] R10: 0000000064f65b73 R11: 000000001cb47ae9 R12: ffffffff93d71d40
[1667258.914306] R13: ffffb731b3fbcd40 R14: 0000000000000002 R15: ffffb7322024b000
[1667258.914311] FS: 0000000000000000(0000) GS:ffffa0bb7f580000(0000) knlGS:0000000000000000
[1667258.914318] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[1667258.914323] CR2: ffffb731b3fbcd40 CR3: 0000000b09e3c005 CR4: 00000000007706f0
[1667258.914329] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[1667258.914334] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[1667258.914339] PKRU: 55555554
[1667258.914342] Call Trace:
[1667258.914346] <TASK>
[1667258.914350] ? show_regs+0x6d/0x80
[1667258.914358] ? __die+0x24/0x80
[1667258.914364] ? page_fault_oops+0x99/0x1b0
[1667258.914372] ? kernelmode_fixup_or_oops.isra.0+0x69/0x90
[1667258.914380] ? __bad_area_nosemaphore+0x19e/0x2c0
[1667258.914388] ? bad_area_nosemaphore+0x16/0x30
[1667258.914394] ? do_kern_addr_fault+0x7b/0xa0
[1667258.914400] ? exc_page_fault+0x1a4/0x1b0
[1667258.914407] ? asm_exc_page_fault+0x27/0x30
[1667258.914417] ? memcpy_orig+0x105/0x130
[1667258.914425] ? lib_sha256_base_do_update.isra.0+0x5d/0x1d0 [sha256_ssse3]
[1667258.914433] ? __pfx_sha256_transform_rorx+0x10/0x10 [sha256_ssse3]
[1667258.914440] sha256_finup+0xf5/0x150 [sha256_ssse3]
[1667258.914447] sha256_avx2_digest+0x55/0x70 [sha256_ssse3]
[1667258.914453] crypto_shash_digest+0x2a/0x60
[1667258.914460] fw_log_firmware_info+0x113/0x1b0
[1667258.914469] _request_firmware+0x19d/0x4b0
[1667258.914476] firmware_request_nowarn+0x36/0x60
[1667258.914482] sahara_processing+0x399/0x710 [qaic]
[1667258.914501] process_one_work+0x181/0x3a0
[1667258.914508] worker_thread+0x306/0x440
[1667258.914514] ? _raw_spin_lock_irqsave+0xe/0x20
[1667258.914521] ? __pfx_worker_thread+0x10/0x10
[1667258.914526] kthread+0xef/0x120
[1667258.914533] ? __pfx_kthread+0x10/0x10
[1667258.914540] ret_from_fork+0x44/0x70
[1667258.914546] ? __pfx_kthread+0x10/0x10
[1667258.914552] ret_from_fork_asm+0x1b/0x30
[1667258.914561] </TASK>
[1667258.914564] Modules linked in: tls nfsv3 rpcsec_gss_krb5 nfsv4 nfs netfs snd_seq_dummy snd_hrtimer snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore qrtr_mhi(OE) qrtr(OE) xt_conntrack xt_MASQUERADE bridge stp llc xt_set ip_set nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_addrtype nft_compat nf_tables xfrm_user xfrm_algo openafs(POE-) overlay cfg80211 binfmt_misc nls_iso8859_1 intel_rapl_msr intel_rapl_common intel_uncore_frequency intel_uncore_frequency_common isst_if_common skx_edac skx_edac_common nfit x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel ipmi_ssif kvm cmdlinepart spi_nor irqbypass mtd qaic(OE) mei_me rapl intel_cstate i2c_i801 mhi(OE) spi_intel_pci mei switchtec(OE) ioatdma spi_intel i2c_smbus intel_pch_thermal dca ipmi_si acpi_power_meter acpi_ipmi ipmi_devintf ipmi_msghandler acpi_pad mac_hid sch_fq_codel dm_multipath nfsd msr parport_pc auth_rpcgss nfs_acl lockd ppdev grace lp parport sunrpc efi_pstore nfnetlink dmi_sysfs
[1667258.914665] ip_tables x_tables autofs4 btrfs blake2b_generic xor raid6_pq libcrc32c dm_mirror dm_region_hash dm_log crct10dif_pclmul crc32_pclmul polyval_clmulni polyval_generic nvme ghash_clmulni_intel sha256_ssse3 bnxt_en sha1_ssse3 nvme_core xhci_pci nvme_auth xhci_pci_renesas wmi aesni_intel crypto_simd cryptd
[1667258.914741] CR2: ffffb731b3fbcd40
[1667258.914746] ---[ end trace 0000000000000000 ]---
Thanks
- Youssef
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
2026-03-06 16:37 ` Youssef Samir
@ 2026-03-06 21:20 ` Eric Biggers
2026-03-10 15:12 ` Youssef Samir
0 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2026-03-06 21:20 UTC (permalink / raw)
To: Youssef Samir
Cc: amadeuszx.slawinski, arnd, dakr, gregkh, linux-crypto,
linux-kernel, mcgrof, rafael, russ.weight, jeff.hugo, troy.hanson
On Fri, Mar 06, 2026 at 05:37:43PM +0100, Youssef Samir wrote:
> On 4/28/2025 8:09 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > This user of SHA-256 does not support any other algorithm, so the
> > crypto_shash abstraction provides no value. Just use the SHA-256
> > library API instead, which is much simpler and easier to use.
> >
> > Also take advantage of printk's built-in hex conversion using %*phN.
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >
> > This patch is targeting the firmware_loader tree for 6.16.
> Hi Eric,
>
> An issue has been found on kernel versions older than v6.16, where a firmware
> file larger than 2GB and is not divisible by SHA256_BLOCK_SIZE (64b) will always lead to a page fault. The first size that fits this criteria is 2147483649b. It is also worth noting that any subsequent loads regardless of the size or divisibility by 64, will lead to another page fault.
> I've mainly tested this with drivers/accel/qaic on 6.8.0-62-generic, but technically this should affect any code that uses the firmware loader on a kernel version older than v6.16 with CONFIG_FW_LOADER_DEBUG enabled, including the stable kernels.
>
> This can be reproduced by creating a dummy binary file of a size that fits the criteria listed above, then compress it using zstd to allow _request_firmware() to open it.
>
> This patch appears to have fixed the issue so I suggest backporting it, but
> I also noticed that it relies on changes that were introduced in this series:
> https://lore.kernel.org/lkml/cover.1745734678.git.herbert@gondor.apana.org.au/
I guess this further shows that the upgrade to size_t lengths was a good
idea...
There was recently a similar bug report where on old kernels kexec
crashed in crypto_sha256_update when loading a file larger than ~2 GB.
It had already been fixed upstream by the upgrade to size_t lengths.
However, due to the large number of backports that would have been
needed, for the 6.1, 6.6, and 6.12 LTS kernels we just went with the
one-line fix "crypto: sha256 - fix crash at kexec"
(https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-6.12.y&id=70165dc3ec8cff702da7b8b122c44575ee3111d6).
That increased the supported length in 6.1, and 6.6, and 6.12 from ~2 GB
to ~4 GB. Your "6.8.0-62-generic" distro kernel must be missing that
commit. You should first ask your distro to cherry-pick that commit
from 6.12, and it will fix the problem for sizes < ~4 GB.
Do you need support for sizes > ~4 GB? If so, then we can come up with
a solution for that in the LTS kernels. (Besides just doing a lot of
backports, one option would be to replace the call to
crypto_shash_digest() with a multi-step incremental computation.)
- Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API
2026-03-06 21:20 ` Eric Biggers
@ 2026-03-10 15:12 ` Youssef Samir
0 siblings, 0 replies; 7+ messages in thread
From: Youssef Samir @ 2026-03-10 15:12 UTC (permalink / raw)
To: ebiggers
Cc: amadeuszx.slawinski, arnd, dakr, gregkh, jeff.hugo, linux-crypto,
linux-kernel, mcgrof, rafael, russ.weight, troy.hanson
On 3/6/2026 9:20 PM, Eric Biggers wrote:
>
> I guess this further shows that the upgrade to size_t lengths was a good
> idea...
>
> There was recently a similar bug report where on old kernels kexec
> crashed in crypto_sha256_update when loading a file larger than ~2 GB.
> It had already been fixed upstream by the upgrade to size_t lengths.
> However, due to the large number of backports that would have been
> needed, for the 6.1, 6.6, and 6.12 LTS kernels we just went with the
> one-line fix "crypto: sha256 - fix crash at kexec"
> (https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-6.12.y&id=70165dc3ec8cff702da7b8b122c44575ee3111d6).
>
> That increased the supported length in 6.1, and 6.6, and 6.12 from ~2 GB
> to ~4 GB. Your "6.8.0-62-generic" distro kernel must be missing that
> commit. You should first ask your distro to cherry-pick that commit
> from 6.12, and it will fix the problem for sizes < ~4 GB.
Thanks, that fixes the issue :) I will request from them to cherry-pick it.
>
> Do you need support for sizes > ~4 GB? If so, then we can come up with
> a solution for that in the LTS kernels. (Besides just doing a lot of
> backports, one option would be to replace the call to
> crypto_shash_digest() with a multi-step incremental computation.)
No need, 4GB is way larger than our image size already.
Thanks
- Youssef
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-10 15:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-28 19:09 [PATCH] firmware_loader: use SHA-256 library API instead of crypto_shash API Eric Biggers
2025-04-29 5:54 ` Greg Kroah-Hartman
2025-04-29 16:42 ` Eric Biggers
2025-04-30 20:08 ` Danilo Krummrich
2026-03-06 16:37 ` Youssef Samir
2026-03-06 21:20 ` Eric Biggers
2026-03-10 15:12 ` Youssef Samir
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox