The carry flag calculation fails when r01.m_high is saturated
(0xFFFFFFFFFFFFFFFF) and addition of lower bits overflows.
The condition (r01.m_high < product.m_high) doesn't handle the case
where r01.m_high == product.m_high and an additional carry exists
from lower-bit overflow.
When commit 3c4b23901a ("crypto: ecdh - Add ECDH software support")
introduced crypto/ecc.c, it split the muladd() function in the
micro-ecc library into separate mul_64_64() and add_128_128() helpers.
It seems the check got lost in translation.
Add proper handling for this boundary by accounting for the carry
from the lower addition.
Fixes: 3c4b23901a ("crypto: ecdh - Add ECDH software support")
Signed-off-by: Anastasia Tishchenko <sv3iry@gmail.com>
Cc: stable@vger.kernel.org # v4.8+
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
In-tree drivers do not need MODULE_VERSION as the kernel release
identifies the version of their code. The static version "0.6.0", which
the QAT drivers currently report, can be misleading as it might suggest
the drivers are outdated.
Remove MODULE_VERSION() from all QAT driver modules and the related
ADF_DRV_VERSION, ADF_MAJOR_VERSION, ADF_MINOR_VERSION and
ADF_BUILD_VERSION macros from adf_common_drv.h.
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Ahsan Atta <ahsan.atta@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Replace two consecutive min() calls with min3() to simplify the code.
And since count is unsigned and cannot be less than zero, adjust the if
check and update the comment accordingly.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use max() to simplify mv_cesa_probe() and improve its readability.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Now that the character device and IOCTL interface have been removed,
adf_ctl_drv.c only contains module_init/module_exit hooks. Rename it
to adf_module.c to better reflect its purpose and rename the init/exit
functions accordingly.
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Ahsan Atta <ahsan.atta@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The QAT driver exposes a character device (qat_adf_ctl) with IOCTLs
for device configuration, start, stop, status query and enumeration.
These IOCTLs are not part of any public uAPI header and have no known
in-tree or out-of-tree users. Device lifecycle is already managed via
sysfs.
The ioctl interface also increases the attack surface and is the
subject of a number of bug reports.
Remove the character device, the IOCTL definitions, and the related
data structures (adf_dev_status_info, adf_user_cfg_key_val,
adf_user_cfg_section, adf_user_cfg_ctl_data). Drop the now-unused
adf_cfg_user.h header and strip adf_ctl_drv.c down to the minimal
module_init/module_exit hooks for workqueue, AER, and crypto/compression
algorithm registration.
Clean up leftover dead code that was only reachable from the removed
IOCTL paths: adf_cfg_del_all(), adf_devmgr_verify_id(),
adf_devmgr_get_num_dev(), adf_devmgr_get_dev_by_id(),
adf_get_vf_real_id() and the unused ADF_CFG macros.
Additionally, drop the entry associated to QAT IOCTLs in
ioctl-number.rst.
Cc: stable@vger.kernel.org
Fixes: d8cba25d2c ("crypto: qat - Intel(R) QAT driver framework")
Reported-by: Zhi Wang <wangzhi@stu.xidian.edu.cn>
Reported-by: Bin Yu <byu@xidian.edu.cn>
Reported-by: MingYu Wang <w15303746062@163.com>
Closes: https://lore.kernel.org/all/61d6d499.ab89.19b9b7f3186.Coremail.wangzhi_xd@stu.xidian.edu.cn/
Link: https://lore.kernel.org/all/20260508034841.256794-1-w15303746062@163.com/
Link: https://lore.kernel.org/all/20260508023542.256299-1-w15303746062@163.com/
Link: https://lore.kernel.org/all/20260504025120.98242-1-w15303746062@163.com/
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Ahsan Atta <ahsan.atta@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Lower the priority of HiSilicon's crypto implementations to allow more
suitable alternatives to be selected. For example, certain kernel
use-cases do not benefit from HiSilicon's symmetric crypto algorithms.
This change ensures that more appropriate options are chosen first while
retaining HiSilicon's implementations as alternatives.
Signed-off-by: lizhi <lizhi206@huawei.com>
Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
Reviewed-by: Longfang Liu <liulongfang@huawei.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
krb5_aead_encrypt(), krb5_aead_decrypt() in rfc3961_simplified.c and
rfc8009_encrypt(), rfc8009_decrypt() in rfc8009_aes2.c set a NULL
completion callback and treat any negative return from
crypto_aead_{encrypt,decrypt}() as terminal, falling through to
kfree_sensitive(buffer). When the encrypt_name resolves to an
async AEAD instance the request returns -EINPROGRESS, the buffer
is freed while the backend's worker still holds a pointer, and the
worker dereferences the freed slab on completion.
KASAN report under UML+SLUB with a synthetic async aead backend
bound to krb5->encrypt_name:
BUG: KASAN: slab-use-after-free in t5_stub_complete+0x7d/0xc7
The helpers were written synchronously, so filter the async
instances out at allocation time instead of plumbing
crypto_wait_req() through every call site.
Reachable via net/rxrpc/rxgk.c, fs/afs/cm_security.c and
net/ceph/crypto.c on systems with an async AEAD provider bound to
the krb5 enctype name.
Fixes: 00244da40f ("crypto/krb5: Implement the Kerberos5 rfc3961 encrypt and decrypt functions")
Fixes: 6c3c0e86c2 ("crypto/krb5: Implement the AES enctypes from rfc8009")
Cc: stable@vger.kernel.org
Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Document True Random Number Generator on Qualcomm Nord SoC.
Signed-off-by: Deepti Jaggi <deepti.jaggi@oss.qualcomm.com>
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Drop the CONFIG_OF preprocessor guard and remove of_match_ptr() because
OF matching is stubbed out when CONFIG_OF=n.
Reformat atmel_ecc_dt_ids for consistency.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Since MODULE_DEVICE_TABLE() keeps atmel_sha204a_dt_ids referenced, drop
the unnecessary __maybe_unused annotation. Also remove of_match_ptr()
because OF matching is stubbed out when CONFIG_OF=n.
Reformat atmel_sha204a_dt_ids to silence a checkpatch error and
atmel_sha204a_id for consistency.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Use designated initializers for the report structs instead of clearing
the struct with memset() and then copying fixed strings with strscpy()
at runtime.
This keeps the structs zero-initialized, lets the compiler diagnose
oversized string literals, and makes the code easier to read.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The memory allocated for priv in safexcel_pci_probe() is not freed in the
error paths, as well as in the PCI remove function. Fix this by using
device managed allocation.
Fixes: 625f269a5a ("crypto: inside-secure - add support for PCI based FPGA development board")
Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
Reviewed-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
When meson_allocate_chanlist() fails after a partial allocation, it already
unwinds the allocated chanlist state through its local error path.
meson_crypto_probe() then jump to error_flow and calls
meson_free_chanlist() again, causing the same per-flow resources to be torn
down twice. In the reproduced failure path, the second teardown
re-entered crypto_engine_exit() on an already destroyed worker and KASAN
reported a slab-use-after-free in kthread_destroy_worker().
Prevent double-free by handling partial allocation failures locally within
meson_allocate_chanlist() and skipping the outer cleanup path.
The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available.
The bug was reproduced in a QEMU x86_64 guest booted with KASAN on v7.1,
using the reproducer under tools/testing/meson_crypto_probe. The reproducer
forces the second dma_alloc_attrs() call in the gxl-crypto probe path to
return NULL, making meson_allocate_chanlist() fail after partial
initialization. On the unpatched kernel this reliably triggered a
slab-use-after-free. With this fix applied, the same reproducer no longer
emits any KASAN report and the probe fails cleanly with -ENOMEM.
==================================================================
BUG: KASAN: slab-use-after-free in kthread_destroy_worker+0xb2/0xd0
Read of size 8 at addr ff1100010c057a68 by task insmod/265
CPU: 1 UID: 0 PID: 265 Comm: insmod Tainted: G O 7.1.0-rc2-00376-g810af9adc907-dirty #10 PREEMPT(lazy)
Tainted: [O]=OOT_MODULE
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x68/0xa0
print_report+0xcb/0x5e0
? __virt_addr_valid+0x21d/0x3f0
? kthread_destroy_worker+0xb2/0xd0
? kthread_destroy_worker+0xb2/0xd0
kasan_report+0xca/0x100
? kthread_destroy_worker+0xb2/0xd0
kthread_destroy_worker+0xb2/0xd0
meson_crypto_probe+0x4d0/0xc10 [amlogic_gxl_crypto]
platform_probe+0x99/0x140
really_probe+0x1c6/0x6a0
? __pfx___device_attach_driver+0x10/0x10
__driver_probe_device+0x248/0x310
? acpi_driver_match_device+0xb0/0x100
driver_probe_device+0x48/0x210
? __pfx___device_attach_driver+0x10/0x10
__device_attach_driver+0x160/0x320
bus_for_each_drv+0x104/0x190
? __pfx_bus_for_each_drv+0x10/0x10
? _raw_spin_unlock_irqrestore+0x2c/0x50
__device_attach+0x19d/0x3b0
? __pfx___device_attach+0x10/0x10
? do_raw_spin_unlock+0x53/0x220
device_initial_probe+0x78/0xa0
bus_probe_device+0x5b/0x130
device_add+0xcfd/0x1430
? __pfx_device_add+0x10/0x10
? insert_resource+0x34/0x50
? lock_release+0xc9/0x290
platform_device_add+0x24e/0x590
? __pfx_meson_crypto_probe_repro_init+0x10/0x10 [meson_crypto_probe_repro]
meson_crypto_probe_repro_init+0x330/0xff0 [meson_crypto_probe_repro]
do_one_initcall+0xc0/0x450
? __pfx_do_one_initcall+0x10/0x10
? _raw_spin_unlock_irqrestore+0x2c/0x50
? __create_object+0x59/0x80
? kasan_unpoison+0x27/0x60
do_init_module+0x27b/0x7d0
? __pfx_do_init_module+0x10/0x10
? kasan_quarantine_put+0x84/0x1d0
? kfree+0x32c/0x510
? load_module+0x561e/0x5ff0
load_module+0x54fe/0x5ff0
? __pfx_load_module+0x10/0x10
? security_file_permission+0x20/0x40
? kernel_read_file+0x23d/0x6e0
? mmap_region+0x235/0x4a0
? __pfx_kernel_read_file+0x10/0x10
? __file_has_perm+0x2c0/0x3e0
init_module_from_file+0x158/0x180
? __pfx_init_module_from_file+0x10/0x10
? __lock_acquire+0x45a/0x1ba0
? idempotent_init_module+0x315/0x610
? lock_release+0xc9/0x290
? lockdep_init_map_type+0x4b/0x220
? do_raw_spin_unlock+0x53/0x220
idempotent_init_module+0x330/0x610
? __pfx_idempotent_init_module+0x10/0x10
? __pfx_cred_has_capability.isra.0+0x10/0x10
? ksys_mmap_pgoff+0x385/0x520
__x64_sys_finit_module+0xbe/0x120
do_syscall_64+0x115/0x690
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f7d6d31690d
Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d f3 b4 0f 00 f7 d8 >
RSP: 002b:00007fffc027ac68 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 000055f7b81967c0 RCX: 00007f7d6d31690d
RDX: 0000000000000000 RSI: 000055f79a0d6cd2 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000003 R11: 0000000000000246 R12: 000055f79a0d6cd2
R13: 000055f7b8196790 R14: 000055f79a0d5888 R15: 000055f7b81968e0
</TASK>
Fixes: 48fe583fe5 ("crypto: amlogic - Add crypto accelerator for amlogic GXL")
Cc: stable@vger.kernel.org
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
platform_get_resource and devm_ioremap effectively open codes this.
The return type of devm_platform_ioremap_resource() is also nice as it
has multiple errors that it can return.
Because it internally calls devm_request_mem_region(), reg values and
sizes cannot overlap. This was manually verified to be the case for all
talitos users.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The problem described in commit 655ef638a2 ("crypto: talitos - fix
SEC1 32k ahash request limitation") also apply for the SEC2 hardware,
but with a limitation of 64k - 1 bytes.
Split ahash_done() into SEC1 and SEC2 paths: SEC1 continues to free the
whole descriptor list at once, while SEC2 now iterates through
descriptors one by one, submitting the next only after the previous
completes, which is required since SEC2 cannot chain descriptors in
hardware.
Cc: stable@vger.kernel.org
Fixes: c662b043cd ("crypto: af_alg/hash: Support MSG_SPLICE_PAGES")
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
ahash_process_req() was a wrapper used in commit 655ef638a2 ("crypto:
talitos - fix SEC1 32k ahash request limitation"). Rename
ahash_process_req_one() to ahash_process_req() and remove the wrapper.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
In talitos_ahash_req_ctx and talitos_export_state, the fields
first_desc and last_desc describe request-level (not descriptor-level)
state. Rename them to first_request and last_request for clarity.
last_desc is also removed from talitos_ahash_req_ctx as it is no
longer used.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Now that SEC1 hash uses hardware descriptor chaining instead of a
workqueue to process requests exceeding TALITOS1_MAX_DATA_LEN, the
workqueue code is no longer needed.
Remove sec1_ahash_process_remaining(), the related fields from
talitos_ahash_req_ctx (request_bufsl, areq, request_sl,
remaining_ahash_request_bytes, current_ahash_request_bytes,
sec1_ahash_process_remaining), the dead code in ahash_done(), and
simplify ahash_process_req() to call ahash_process_req_one() directly
with the original areq->src and areq->nbytes.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Rework the SEC1 ahash implementation to build a chain of hardware
descriptors, replacing the previous approach of submitting one
descriptor at a time via a workqueue, introduced by commit 655ef638a2
("crypto: talitos - fix SEC1 32k ahash request limitation").
Introduce ahash_process_req_prepare() which iterates over the request
data, allocating enough descriptors to cover the entire ahash request.
The new fields (bufsl, src, first, last) are added to talitos_edesc for
this purpose.
common_nonsnoop_hash() no longer calls talitos_submit(); it only
maps and sets up the descriptor. Submission is now done by the caller
after the chain is built.
ahash_free_desc_list_from() takes over calling
common_nonsnoop_hash_unmap() for each descriptor during cleanup.
Compared to the workqueue based solution, request are slightly faster
since there is no more scheduling latency induced by the workqueue, and
only one interrupt is generated by the device at the end of a chain.
Commit 655ef638a2 ("crypto: talitos - fix SEC1 32k ahash request
limitation") :
$ /usr/libexec/libkcapi/sha256sum ./test_5M.bin
013c5609d63c... ./test_5M.bin
real 0m 0.41s
user 0m 0.01s
sys 0m 0.07s
Now :
$ /usr/libexec/libkcapi/sha256sum ./test_5M.bin
013c5609d63c... ./test_5M.bin
real 0m 0.33s
user 0m 0.01s
sys 0m 0.20s
Tested on a system with an MPC885 SoC featuring the SEC1 Lite.
The increase in sys time is due to the fact that commit 37b5e8897e
("crypto: talitos - chain in buffered data for ahash on SEC1") can no
longer be applied.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Currently, when SEC1 has buffered data (nbuf != 0), the ahash code
creates an additional descriptor on the fly inside
common_nonsnoop_hash() to handle the remainder of the data. This
approach is incompatible with the arbitrary-length descriptor chaining
that follows.
Remove the "additional descriptor" logic from common_nonsnoop_hash()
and common_nonsnoop_hash_unmap().
Also remove the nbytes adjustment for SEC1 in ahash_edesc_alloc()
that subtracted nbuf.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Previously added code in current_desc_hdr() in order to add support for
searching an offending descriptor inside a descriptor chain.
Move that code into a standalone function to improve readability.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Previously added code to talitos_submit() in order to map an entire
descriptor chain.
Move that code into a standalone function to improve readability.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Previously added code to flush_channel() in order to unmap an entire
descriptor.
Move that code into a standalone function to improve readability.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
The SEC1 hardware can process a chain of descriptors without host
intervention. Only the hash implementation currently use this feature,
but with a chain of at most 2 descriptors added in commit 37b5e8897e
("crypto: talitos - chain in buffered data for ahash on SEC1").
Add supports for chaining an arbitrary number of descriptors in a chain.
Adapt the ahash implementation to make it compatible.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
In order to know if a descriptor has been processed by the device,
the driver polls the FIFO to see if DESC_HDR_DONE is set on a descriptor
header to confirm completion.
The current code does not make sure that the CPU gets up to date data
before reading the descriptor.
Fix this by calling dma_sync_single_for_cpu() before reading memory
written by the device.
Cc: stable@vger.kernel.org
Fixes: 58cdbc6d22 ("crypto: talitos - fix hash on SEC1.")
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
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>
