dsm_create() initially checks pdev->bus when computing segment_id:
u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;
But the next two lines unconditionally dereference pdev->bus via
pcie_find_root_port() and especially pci_dev_id(pdev), which expands
to PCI_DEVID(dev->bus->number, dev->devfn). If pdev->bus is in fact
NULL, segment_id is initialised to 0 but the very next statement
crashes the kernel.
smatch flags this:
drivers/crypto/ccp/sev-dev-tsm.c:253 dsm_create() error: we
previously assumed 'pdev->bus' could be null (see line 251)
Make the NULL handling consistent: if pdev->bus is NULL the device
has no PCI context to work with and SEV TIO setup cannot proceed,
so return -ENODEV before any of the bus-dependent lookups. The
remaining initialisation now runs only on the path where pdev->bus
is known to be valid.
No change for callers where pdev->bus is non-NULL, which is the
only case where dsm_create() did meaningful work before this change.
Fixes: 4be423572d ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use the simpler min() macro since the values are all unsigned and
compatible.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: David Laight <david.laght.linux@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Andrew reports build breakage of arm allmodconfig, reproducible with gcc
14.2.0 and 15.2.0:
crypto/ecc.c: In function 'ecc_point_mult':
crypto/ecc.c:1380:1: error: the frame size of 1360 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
gcc aggressively inlines functions called by ecc_point_mult() (without
there being any explicit inline declarations), which pushes stack usage
close to the limit imposed by CONFIG_FRAME_WARN. allmodconfig implies
CONFIG_KASAN_STACK=y, which increases the stack above that limit.
In the bugzilla entry linked below, gcc maintainers explain that gcc
estimates extra stack usage caused by inlining, but ASAN instrumentation
is added in post-IPA passes and thus the inlining heuristics cannot
account for it.
It could be argued that -Werror=frame-larger-than=1280 instructs the
compiler to avoid inlining beyond that limit lest the build breaks,
which would imply gcc behaves incorrectly. But gcc maintainers reject
this notion and believe that a warning switch should never affect code
generation, even if it is promoted to an error.
One way to unbreak the build is to limit inlining via -finline-limit=100
or by explicitly declaring some functions noinline. However while it
does keep stack usage of individual functions below the limit, *total*
stack usage increases.
A longterm solution is to refactor ecc.c for reduced stack usage. It
currently performs ECC point multiplication with a Montgomery ladder
which uses co-Z (conjugate) addition to trade off memory for speed.
The algorithm is susceptible to timing attacks and needs to be replaced
with a constant time Montgomery ladder, which should consume less memory
and thus resolve the stack usage issue as a side effect.
In the interim, raise the limit for ecc.c, as is already done for
several other files in the source tree.
Constrain to gcc because clang 19.1.7 does not exhibit the issue. It
makes do with a 724 bytes stack frame even though it inlines almost the
same functions as gcc.
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124949
Reported-by: Andrew Morton <akpm@linux-foundation.org> # off-list
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Replace snprintf("%s") with the faster and more direct strscpy().
In cc_aead.c, group the includes while at it.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Replace if-else with an early return to reduce code nesting, and move
the variable declarations to the top of the function.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use a flexible array member to combine allocations.
Add __counted_by for extra runtime analysis.
Error in case of no channels as they are required.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
"Prediction resistance", i.e. the property that the RNG's output is
unpredictable even after a state compromise, might sound like a nice
property to have. In reality, it's not very practical, as it requires
that fresh entropy be pulled on every request. (The normal Linux RNG
doesn't provide prediction resistance.) In the case of drbg.c, that
means pulling from "jitterentropy", which is extremely slow.
For some perspective, running a simple benchmark, generating 32 random
bytes takes the following amount of time:
get_random_bytes(): 90 ns
drbg_nopr_hmac_sha512: 3707 ns
drbg_pr_hmac_sha512: 773082 ns
So at least in this case, the "pr" (prediction-resistant) DRBG is over
200 times slower than the "nopr" (non-prediction-resistant) DRBG, or
over 8000 times slower than the normal Linux RNG. While anyone using
drbg.c has always had to tolerate that it's slower than the normal Linux
RNG, the "pr" DRBG is clearly at another level of slowness.
Thus, the following is also entirely unsurprising:
- FIPS 140-3 doesn't actually require that SP800-90A DRBG
implementations support prediction resistance. The non-prediction
resistant DRBGs can be, and have been, certified.
- drbg.c registers "drbg_nopr_hmac_sha512" with a higher cra_priority
than "drbg_pr_hmac_sha512". So "drbg_nopr_hmac_sha512" is already
the one actually being used in practice.
Given these considerations, it's clear that "drbg_pr_hmac_sha512" isn't
actually useful, and it essentially just existed as another curiosity in
the museum of crypto algorithms. Remove it to simplify the code.
Suggested-by: Joachim Vandersmissen <joachim@jvdsn.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Give this constant a name which is clearer and consistent with
DRBG_MAX_REQUEST_BYTES. No functional change.
Suggested-by: Joachim Vandersmissen <joachim@jvdsn.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
By convention MODULE_DEVICE_TABLE() immediately follows the ID table it
exports, because this is easier to read and verify. It also makes more
sense since #ifdef for ACPI or OF could hide both of them.
Most of the privers already have this correctly placed, so adjust
the missing ones. No functional impact.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Replace strlcat() with sysfs_emit_at() in rng_available_show() and add
'int len' to keep track of the number of bytes written. sysfs_emit_at()
is preferred for formatting sysfs output because it provides safer
bounds checking.
Inline mutex_lock_interruptible() and drop the now-unused local error
variable. Remove the unnecessary 'buf' NUL initialization. Return 'len'
directly instead of strlen(buf).
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Replace the open-coded variant with MAX().
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The wait parameter in rng_get_data() is a boolean flag - use bool
instead of int to better reflect its actual type.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The forward declarations for drop_current_rng() and rng_get_data() are
not needed - remove them.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
In order to prevent arithmetic overflows when checking the TX
buffer size, cap the associated data length to 0x80000000.
Reported-by: Yiming Qian <yimingqian591@gmail.com>
Fixes: 400c40cf78 ("crypto: algif - add AEAD support")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
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>
