The zero-copy support is one of the riskiest aspects of AF_ALG. It
allows userspace to request cryptographic operations directly on
pagecache pages of files like the 'su' binary. It also allows userspace
to concurrently modify the memory which is being operated on, a recipe
for TOCTOU vulnerabilities.
While zero-copy support is more valuable in other areas of the kernel
like the frequently used networking and file I/O code, it has far less
value in AF_ALG, which is a niche UAPI. AF_ALG primarily just exists
for backwards compatibility with a small set of userspace programs such
as 'iwd' that haven't yet been fixed to use userspace crypto code.
Originally AF_ALG was intended to be used to access hardware crypto
accelerators. However, it isn't an efficient interface for that anyway,
and it turned out to be rarely used in this way in practice.
Thus, the risks of the zero-copy support in AF_ALG vastly outweigh its
benefits. Let's just remove it.
This commit removes it from the "skcipher" and "aead" algorithm types.
"hash" will be handled separately.
This is a soft break, not a hard break. Even after this commit, it
still works to use splice() or sendfile() to transfer data to an AF_ALG
request socket from a pipe or any file, respectively. What changes is
just that the kernel now makes an internal, stable copy of the data
before doing the crypto operation. So performance is slightly reduced,
but the UAPI isn't broken. And, very importantly, it's much safer.
Tested with libkcapi/test.sh. All its test cases still pass. I also
verified that this would have prevented the copy.fail exploit as well.
I also used a custom test program to verify that sendfile() still works.
Fixes: 8ff590903d ("crypto: algif_skcipher - User-space interface for skcipher operations")
Fixes: 400c40cf78 ("crypto: algif - add AEAD support")
Reported-by: Taeyang Lee <0wn@theori.io>
Link: https://copy.fail/
Reported-by: Feng Ning <feng@innora.ai>
Closes: https://lore.kernel.org/r/afYcc-tZFwvZZo76@ans-MacBook-Pro.local
Reviewed-by: Demi Marie Obenour <demiobenour@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
When handling a PDH export, treat a zero-length userspace cert chain buffer
as a request to query the length of the relevant blobs. Failure to account
for the zero-length buffer trips a BUG_ON() when running with
CONFIG_DEBUG_VIRTUAL=y due to trying to get the physical address of the
ZERO_SIZE_PTR (returned by kzalloc() on the bogus allocation).
kernel BUG at arch/x86/mm/physaddr.c:28 !
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
CPU: 30 UID: 0 PID: 28580 Comm: syz.2.18 Kdump: loaded
Tainted: G W 6.18.16-smp-DEV #1 NONE
Tainted: [W]=WARN
Hardware name: Google, Inc. Arcadia_IT_80/Arcadia_IT_80, BIOS 12.62.0-0 11/19/2025
RIP: 0010:__phys_addr+0x16a/0x180 arch/x86/mm/physaddr.c:28
RSP: 0018:ffffc9008329fc80 EFLAGS: 00010293
RAX: ffffffff8179110a RBX: 0000778000000010 RCX: ffff8884e6992600
RDX: 0000000000000000 RSI: 0000000080000010 RDI: 0000778000000010
RBP: ffffc9008329fdf0 R08: 0000000000000dc0 R09: 00000000ffffffff
R10: dffffc0000000000 R11: fffffbfff126d297 R12: dffffc0000000000
R13: 1ffff92010653fc8 R14: 0000000080000010 R15: dffffc0000000000
FS: 0000555556bec9c0(0000) GS:ffff88aa4ce1c000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fd3159e7000 CR3: 00000004fbc44000 CR4: 0000000000350ef0
Call Trace:
<TASK>
[<ffffffff853d3869>] sev_ioctl_do_pdh_export+0x559/0x7a0 drivers/crypto/ccp/sev-dev.c:2308
[<ffffffff853d1fdd>] sev_ioctl+0x2cd/0x480 drivers/crypto/ccp/sev-dev.c:2556
[<ffffffff82549ebc>] vfs_ioctl fs/ioctl.c:52 [inline]
[<ffffffff82549ebc>] __do_sys_ioctl fs/ioctl.c:598 [inline]
[<ffffffff82549ebc>] __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
[<ffffffff8630115f>] do_syscall_x64 arch/x86/entry/syscall_64.c:64 [inline]
[<ffffffff8630115f>] do_syscall_64+0x9f/0xf40 arch/x86/entry/syscall_64.c:98
[<ffffffff81000136>] entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7fd3158eac39
</TASK>
Thankfully, the bug is benign outside of CONFIG_DEBUG_VIRTUAL=y as getting
the physical address is just arithmetic, and the PSP errors out before
trying to write to the garbage address (which it must, otherwise querying
the blob lengths would clobber memory at pfn=0).
Fixes: 76a2b524a4 ("crypto: ccp: Implement SEV_PDH_CERT_EXPORT ioctl command")
Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Remove repeated "+".
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
Reviewed-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Sashiko notes:
> if SEV initialization fails and KVM is actively running normal VMs, could a
> userspace process trigger this code path via /dev/sev ioctls (e.g.,
> SEV_PDH_GEN) and zero out MSR_VM_HSAVE_PA globally? Would the next VMRUN
> execution for an active VM trigger a general protection fault and crash the
> host?
Refuse to re-try initialization if SNP is not already initialized for
SNP_CONFIG.
This is technically an ABI break: before if SNP initialization failed it
could be transparently retriggered by this ioctl, and if no VMs were
running, everything worked fine. Hopefully this is enough of a corner case
that nobody will notice, but someone does, there are a few options:
* do something like symbol_get() for kvm and refuse to initialize if KVM is
loaded
* check each cpu's HSAVE_PA for non-zero data before re-initializing
* once initialization has failed, continue to refuse to initialize until
the ccp module is unloaded
Fixes: ceac7fb89e ("crypto: ccp - Ensure implicit SEV/SNP init and shutdown in ioctls")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
CC: <stable@vger.kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Sashiko notes:
> if SEV initialization fails and KVM is actively running normal VMs, could a
> userspace process trigger this code path via /dev/sev ioctls (e.g.,
> SEV_PDH_GEN) and zero out MSR_VM_HSAVE_PA globally? Would the next VMRUN
> execution for an active VM trigger a general protection fault and crash the
> host?
The SEV firmware docs for SNP_VLEK_LOAD note:
> On SNP_SHUTDOWN, the VLEK is deleted.
That is, the initialization/shutdown wrapper here is pointless, because the
firmware immediately throws away the key anyway. Instead, refuse to do
anything if SNP has not been previously initialized.
This is an ABI break: before, this was a no-op and almost certainly a
mistake by userspace, and now it returns -ENODEV. ABI compatibility could be
maintained here by simply returning 0 in the check instead.
Fixes: ceac7fb89e ("crypto: ccp - Ensure implicit SEV/SNP init and shutdown in ioctls")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
CC: <stable@vger.kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Sashiko notes:
> if SEV initialization fails and KVM is actively running normal VMs, could a
> userspace process trigger this code path via /dev/sev ioctls (e.g.,
> SEV_PDH_GEN) and zero out MSR_VM_HSAVE_PA globally? Would the next VMRUN
> execution for an active VM trigger a general protection fault and crash the
> host?
The SNP_COMMIT command does not require the firmware to be in any
particular state. Skip initializing it if it was previously uninitialized.
The SEV-SNP firmware specification doc 56860 does not mention SNP_COMMIT in
Table 5 as a command that is allowed in the UNINIT state, but it is in fact
allowed and a future documentation update will reflect that.
Fixes: ceac7fb89e ("crypto: ccp - Ensure implicit SEV/SNP init and shutdown in ioctls")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
CC: <stable@vger.kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Sashiko notes:
> if SEV initialization fails and KVM is actively running normal VMs, could a
> userspace process trigger this code path via /dev/sev ioctls (e.g.,
> SEV_PDH_GEN) and zero out MSR_VM_HSAVE_PA globally? Would the next VMRUN
> execution for an active VM trigger a general protection fault and crash the
> host?
sev_move_to_init_state() is called for ioctls requiring only SEV firmware:
SEV_PEK_GEN, SEV_PDH_GEN, SEV_PEK_CSR, SEV_PEK_CERT_IMPORT, and
SEV_PDH_CERT_EXPORT. After the firmware command, it does SEV_SHUTDOWN on
the SEV firmware. Since these commands do not require SNP to be
initialized, skip it by calling __sev_platform_init_locked() which only
initializes the SEV firmware. This way SNP is not Initialized at all, and
HSAVE_PA is not cleared.
The previous code saved any SEV initialization firmware error to
init_args.error and then threw it away and hardcoded the return value of
INVALID_PLATFORM_STATE regardless of the real firmware error. This patch
changes it to surface the underlying error, which is hopefully both more
useful and doesn't cause any problems.
Note that it is still safe to call __sev_firmware_shutdown() directly: it
calls __sev_snp_shutdown_locked(), which skips SNP shutdown if SNP was not
initialized.
Fixes: ceac7fb89e ("crypto: ccp - Ensure implicit SEV/SNP init and shutdown in ioctls")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
CC: <stable@vger.kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Assigning .driver_data for drivers that don't use this struct member is
just noise that can better be dropped. The same applies for an explicit
zero in the terminating entry. Drop these.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The .driver_data member of the struct pci_device_id array was
initialized by list expressions. This isn't easily readable if you're
not into PCI. Using the PCI_DEVICE macro and named initializers is more
explicit and thus easier to parse. Also skip explicit assignment of 0
(which the compiler then takes care of) in the terminating entry.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Explicitly assigning 0 to driver_data in drivers not using this member
has no benefit. Drop these and similarly depend on the compiler to
zero-initialize the list terminator.
This is a preparation for making struct pci_device_id::driver_data an
anonymous union (which makes initializing using a list expression fail),
but it's also a nice cleanup by itself.
It was verified on x86 and arm64 that this change doesn't introduce
changes to the compiled drivers.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The URL https://www.chronox.de/jent.html resolves to a 404 Not Found.
Use https://www.chronox.de/jent/ instead.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Since start_time = end_time - delta, start_time can only equal end_time
when delta is 0, making the explicit end_time == start_time check
redundant. Remove it.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Document the crypto engine on the Qualcomm Nord Platform.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
pefile_digest_pe_contents() computes the trailing-data hash length as
pelen - (hashed_bytes + certs_size). A crafted PE can make the addition
exceed pelen, causing the unsigned subtraction to underflow to ~4 GiB.
This is passed to crypto_shash_update() which reads out of bounds and
panics on unmapped vmalloc guard pages.
BUG: unable to handle page fault for address: ffffc900038d8000
Oops: Oops: 0000 [#1] SMP KASAN NOPTI
RIP: 0010:sha256_blocks_generic (lib/crypto/sha256.c:152)
Call Trace:
<TASK>
__sha256_update (lib/crypto/sha256.c:208)
crypto_sha256_update (crypto/sha256.c:142)
verify_pefile_signature (crypto/asymmetric_keys/verify_pefile.c:436)
kexec_kernel_verify_pe_sig (kernel/kexec_file.c:151)
__do_sys_kexec_file_load (kernel/kexec_file.c:406)
do_syscall_64 (arch/x86/entry/syscall_64.c:94)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
</TASK>
Kernel panic - not syncing: Fatal exception
Validate that the addition does not overflow and the result does not
exceed pelen before the subtraction. Return -ELIBBAD on failure.
Fixes: af316fc442 ("pefile: Digest the PE binary and compare to the PKCS#7 data")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Convert scatterwalk_map_and_copy to memcpy_to/from_sglist as they
are more readable and less error-prone.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
AF_ALG is almost completely unnecessary, and it exposes a massive attack
surface that hasn't been standing up to modern vulnerability discovery
tools. The latest one even has its own website, providing a small
Python script that reliably roots most Linux distros: https://copy.fail/
This isn't sustainable, especially as LLMs have accelerated the rate the
vulnerabilities are coming in. The effort that is being put into this
thing is vastly disproportional to the few programs that actually use
it, and those programs would be better served by userspace code anyway.
These issues have been noted in many mailing list discussions already.
But until now they haven't been reflected in the documentation or
kconfig menu itself, and the vulnerabilities are still coming in.
Let's go ahead and document the deprecation.
This isn't intended to change anything overnight. After all, most Linux
distros won't be able to disable the kconfig options quite yet, mainly
because of iwd. But this should create a bit more impetus for these
userspace programs to be fixed, and the documentation update should also
help prevent more users from appearing.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Commit 8006aff155 ("crypto: atmel-sha204a - Set hwrng quality to
lowest possible") reduced the hwrng quality to 1 based on a review by
Bill Cox [1]. However, despite its title, the review only tested the
ATSHA204, not the ATSHA204A.
In the same thread, Atmel engineer Landon Cox wrote "this behavior has
been eliminated entirely"[2] in the ATSHA204A and "this problem does not
affect the ATECC108 or the ATECC108A (or the ATSHA204A)"[3].
According to the official ATSHA204A datasheet [4], the device contains a
high-quality hardware RNG that combines its output with an internal seed
value stored in EEPROM or SRAM to generate random numbers. The device
also implements all security functions using SHA-256, and the driver
uses the chip's Random command in seed-update mode.
Keep 'quality = 1' for ATSHA204, but drop the explicit hwrng quality
reduction for ATSHA204A and fall back to the hwrng core default.
[1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
[2] https://www.metzdowd.com/pipermail/cryptography/2014-December/023852.html
[3] https://www.metzdowd.com/pipermail/cryptography/2014-December/023886.html
[4] https://ww1.microchip.com/downloads/en/DeviceDoc/ATSHA204A-Data-Sheet-40002025A.pdf
Fixes: 8006aff155 ("crypto: atmel-sha204a - Set hwrng quality to lowest possible")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use list_first_entry_or_null() to simplify starfive_cryp_find_dev() and
remove the now-unused local variable 'struct starfive_cryp_dev *tmp'.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Add a new helper omap_sham_unregister_algs() and replace two for loops
in omap_sham_probe() and omap_sham_remove(), which also ensure
->registered is reset to 0.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Add a new helper omap_des_unregister_algs() and replace two for loops in
omap_des_probe() and omap_des_remove(), which also ensure ->registered
is reset to 0.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Add a new helper omap_aes_unregister_algs() and replace two for loops in
omap_aes_probe() and omap_aes_remove(), which also ensure ->registered
is reset to 0.
Replace two additional for loops with crypto_engine_unregister_aeads()
while at it and reset ->registered to 0 explicitly.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use print_hex_dump_devel() for dumping sensitive key material in
*_setkey() to avoid leaking secrets at runtime when CONFIG_DYNAMIC_DEBUG
is enabled.
Fixes: 8d818c1055 ("crypto: caam/qi2 - add DPAA2-CAAM driver")
Fixes: 226853ac3e ("crypto: caam/qi2 - add skcipher algorithms")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use print_hex_dump_devel() for dumping sensitive key material in
*_setkey() and gen_split_key() to avoid leaking secrets at runtime when
CONFIG_DYNAMIC_DEBUG is enabled.
Fixes: 6e00550319 ("crypto: caam - print debug messages at debug level")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The blocking and non-blocking paths were failing to provide valid entropy
due to improper buffer management. Reading the buffer starting from byte 1,
only fetch the 32 bytes of random data from the return message.
Tested on an Atmel SHA204A device.
Before (here for blocking), tests showed repeatedly reading reduced bytes.
$ head -c 32 /dev/hwrng | hexdump -C
00000000 02 28 85 b3 47 40 f2 ee 00 00 00 00 00 00 00 00 |.(..G@..........|
00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000020
After, the result will be similar to the following:
$ head -c 32 /dev/hwrng | hexdump -C
00000000 5a fc 3f 13 14 68 fe 06 68 0a bd 04 83 6e 09 69 |Z.?..h..h....n.i|
00000010 75 ff cf 87 10 84 3b c9 c1 df ae eb 45 53 4c c3 |u.....;.....ESL.|
00000020
Fixes: da001fb651 ("crypto: atmel-i2c - add support for SHA204A random number generator")
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Tested-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use a flexible array member to combine and simplify allocation.
Move struct mv_cesa_dev down as flexible array members require full
definitions.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Document glymur compatible for the True Random Number Generator.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
When booted with fips=1, RSA signature generation using SHA-1 must not be
available. However, pkcs1pad(rsa,sha1) can currently be instantiated
because it is not present in alg_test_descs; alg_test() falls through the
no_test path and succeeds, after which the algorithm appears in
/proc/crypto as fips-capable.
Add explicit alg_test_descs entries for pkcs1pad(rsa,sha1) and
pkcs1(rsa,sha1) without marking them fips_allowed, so they are treated as
not FIPS-allowed when fips=1 is enabled.
Include both names to cover kernels where RSA sign/verify is provided via
the pkcs1(...) signature template, while pkcs1pad(...) remains for the
traditional wrapper naming and/or RSAES operations.
Signed-off-by: Jeff Barnes <jeffbarnes@linux.microsoft.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
chainup_buffers() builds a linked list of buffer descriptors for a
scatterlist. If dma_pool_alloc() fails while constructing the list, the
current code sets buf to NULL and later dereferences it unconditionally
at the end of the function:
buf->next = NULL;
buf->phys_next = 0;
This can lead to a null-pointer dereference on allocation failure.
If the failure happens after part of the descriptor chain has already
been allocated and DMA-mapped, the partially constructed chain also
needs to be released.
Fix this by terminating the partially constructed chain on allocation
failure and letting the callers unwind it via their existing cleanup
paths. Also fix ablk_perform() to preserve the hook pointers before
checking for failure, so partially built chains can be freed correctly.
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Acked-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
While the sun4i-ss and sun8i-ce drivers started selecting CRYPTO_RNG,
the sun8i-ss variant does not, and causes a link failure:
aarch64-linux-ld: drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.o: in function `sun8i_ss_unregister_algs':
sun8i-ss-core.c:(.text.sun8i_ss_unregister_algs+0x94): undefined reference to `crypto_unregister_rng'
aarch64-linux-ld: drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.o: in function `sun8i_ss_probe':
sun8i-ss-core.c:(.text.sun8i_ss_probe+0x40c): undefined reference to `crypto_register_rng'
Looking more closely, I see that all of the allwinner crypto drivers have the
same logic where the rng and hash parts of the driver are optional, but then the
generic code is still selected, which is a bit inconsistent, aside from the
missing CRYPTO_RNG select on sun8i-ss.
Change the approach so only the bits that are actually used are built, using
ifdef checks around the optional portions that match the optional references
to the sub-drivers.
Ideally the drivers would get reworked in a way that keeps all the bits
related to the skcipher/ahash/rng codecs in the respective sub-drivers,
rather than having a common driver that knows about all of these.
Fixes: cdadc14359 ("crypto: cryptomgr - Select algorithm types only when CRYPTO_SELFTESTS")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Newer versions of ARM TrustedFirmware-A on MediaTek's ARMv8 SoCs no longer
allow accessing the TRNG from outside of the trusted firmware.
Instead, a vendor-defined custom Secure Monitor Call can be used to
acquire random bytes.
Add support for newer SoCs (MT7981, MT7987, MT7988).
As TF-A for the MT7986 may either follow the old or the new
convention, the best bet is to test if firmware blocks direct access
to the hwrng and if so, expect the SMCC interface to be usable.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Add compatible strings for MediaTek SoCs where the hardware random number
generator is accessed via a vendor-defined Secure Monitor Call (SMC)
rather than direct MMIO register access:
- mediatek,mt7981-rng
- mediatek,mt7987-rng
- mediatek,mt7988-rng
These variants require no reg, clocks, or clock-names properties since
the RNG hardware is managed by ARM Trusted Firmware-A.
Relax the $nodename pattern to also allow 'rng' in addition to the
existing 'rng@...' pattern.
Add a second example showing the minimal SMC variant binding.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use 4 spaces for each level indentation, remove unused label, and add
missing empty line between header include and body.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The cipher_null crypto_alg cipher is never used in a meaningful way,
given that it is always wrapped in ecb(), which has its own dedicated
implementation. IOW, the cipher_null crypto_alg should never be used to
implement the ecb(cipher_null) skcipher, and using it for other things
is bogus.
However, it is accessible from user space, and due to the nature of the
AF_ALG interface, it may be wrapped in arbitrary ways, exposing issues
in template code that wasn't written with block ciphers with a block
size of '1' in mind.
So drop this code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This loop is a bit hard to read, with the loop counter that's used in
the HMAC being separate from the actual loop counter, which counts
backwards for some reason. Just replace it with a regular loop.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
A few miscellaneous cleanups to make the code a bit more readable:
- Replace (buf, buflen) with (out, outlen)
- Update (out, outlen) as we go along
- Use size_t for lengths
- Use min()
- Adjust some comments and log messages
- Rename a variable named 'len' to 'err', since it isn't a length
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
We're now incorporating 32 bytes from get_random_bytes() in the
additional input string on every request. The additional input string
is processed with a call to drbg_hmac_update(), which is exactly how the
seed is processed. Thus, in reality this is as good as a reseed.
From the perspective of FIPS 140-3, it isn't as good as a reseed. But
it doesn't actually matter, because from FIPS's point of view
get_random_bytes() provides zero entropy anyway.
Thus, neither the reseed with more get_random_bytes() every 300s, nor
the logic that reseeds more frequently before rng_is_initialized(), is
actually needed anymore. Remove it to simplify the code significantly.
(Technically the use of get_random_bytes() in drbg_seed() itself could
be removed too. But it's safer to keep it there for now.)
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Currently a formal reseed happens only after each 1048576 requests.
That's quite a high number. Let's follow the example of BoringSSL and
use a more conservative value of 4096.
Note that in practice this makes little difference, now that we're
including 32 bytes from get_random_bytes() in the additional input on
every request anyway, which is a de facto reseed.
But for the same reason, we might as well decrease the actual reseed
interval to something more reasonable.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Woodage & Shumow (2018) (https://eprint.iacr.org/2018/349.pdf) showed
that contrary to the claims made by NIST in SP800-90A, HMAC_DRBG doesn't
satisfy the formal definition of forward secrecy (i.e. "backtracking
resistance") when it's called with an empty additional input string.
The actual attack seems pretty benign, as it doesn't actually give the
attacker any previous RNG output, but rather just allows them to test
whether their guess of the previous block of RNG output is correct.
Regardless, it's an annoying design flaw, and it's yet another example
of why NIST's DRBGs aren't all that great.
Meanwhile, the kernel's HMAC_DRBG code also tries to reseed itself
automatically after random.c has reseeded itself. But the
implementation is buggy, as it just checks whether 300 seconds have
elapsed, rather than looking at the actual generation counter.
Let's just follow the example of BoringSSL and use the conservative
approach of always including 32 bytes of "regular" random data in the
additional input string. This fixes both issues described above.
This does reduce performance. But this should be tolerable, since:
- Due to earlier changes, the kernel code that was previously using
drbg.c regardless of FIPS mode is now using it only in FIPS mode.
- The additional input string is processed only once per request. So
if a lot of bytes are generated at once, the cost is amortized.
- The NIST DRBGs are notoriously slow anyway.
Note that this fix should have no impact (either positive or negative)
on FIPS 140 certifiability. From FIPS's point of view the code added by
this commit simply doesn't matter: it adds zero entropy to something
that doesn't need to contain entropy.
Fixes: 541af946fe ("crypto: drbg - SP800-90A Deterministic Random Bit Generator")
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
drbg_kcapi_seed() calls drbg_uninstantiate() only to free drbg->jent and
set drbg->instantiated = false. However, the latter is necessary only
because drbg_kcapi_seed() sets drbg->instantiated = true too early. Fix
that, then just inline the freeing of drbg->jent.
Then, simplify the actual "uninstantiate" in drbg_kcapi_exit(). Just
free drbg->jent (note that this is a no-op on error and null pointers),
then memzero_explicit() the entire drbg_state.
Note that in reality the memzero_explicit() is redundant, since the
crypto_rng API zeroizes the memory anyway. But the way SP800-90A is
worded, it's easy to imagine that someone assessing conformance with it
would be looking for code in drbg.c that says it does an "Uninstantiate"
and does the zeroization. So it's probably worth keeping it somewhat
explicit, even though that means double zeroization in practice.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Fold drbg_prepare_hrng() into its only caller.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Clearly separate the code for the "reseed" and "instantiate" cases,
since what they need to do is quite different.
Note that the mutex guard makes the mutex start being held during the
call to drbg_uninstantiate(), which is fine.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Fold drbg_instantiate() into its only caller.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Put the DRBG implementation of the rng_alg methods in the order in which
they're called (cra_init => set_ent => seed => generate => cra_exit) so
that it's easier to understand the flow.
Also rename drbg_kcapi_random to drbg_kcapi_generate, and
drbg_kcapi_cleanup to drbg_kcapi_exit, so they match the method names.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Simplify drbg_generate_long() to use a more straightforward loop, and
then fold it into its only caller. No functional change.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use straightforward (buffer, len) parameters instead of struct
drbg_string or lists of strings. This simplifies the code considerably.
For now struct drbg_string is still used in crypto_drbg_ctr_df(), so
move its definition to crypto/df_sp80090a.h.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Currently some of the steps of "Instantiate the DRBG" occur in a
convoluted way in different places in the call stack:
drbg_instantiate()
=> drbg_seed(reseed=0) sets the raw HMAC key drbg_state::C to its
correct initial value, and it sets the state value
drbg_state::V to an *incorrect* initial value.
=> drbg_hmac_update(reseed=0) overwrites drbg_state::V with the
correct initial value, then prepares the hmac_sha512_key
drbg_state::key from the initial raw HMAC key drbg_state::C.
Later, each time the HMAC key is updated, drbg_hmac_update() also uses
drbg_state::C to temporarily store the new raw key.
Simplify all of this by:
- Making drbg_instantiate() set the correct initial values of
drbg_state::V and drbg_state::key.
- Converting drbg_hmac_update() to generate the raw key in a
temporary on-stack array instead of drbg_state::C.
- Removing drbg_state::C.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Move the MODULE_ALIAS_CRYPTO lines in the middle of the file to the end
so that they are in the usual place and next to the other one.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Set rng_alg::seed to different functions for the prediction-resistant
and non-prediction-resistant algorithms, so that the function does not
need to parse the algorithm name to figure out which algorithm it is.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Now that none of the information in struct drbg_core is used, remove it.
The null-ity of the pointer drbg_state::core was used to keep track of
whether the DRBG has been instantiated. Replace it with a boolean.
No functional change.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
