- 25 Aug, 2018 8 commits
-
-
Ondrej Mosnacek authored
This patch fixes sleep-in-atomic bugs in AES-CBC and AES-XTS VMX implementations. The problem is that the blkcipher_* functions should not be called in atomic context. The bugs can be reproduced via the AF_ALG interface by trying to encrypt/decrypt sufficiently large buffers (at least 64 KiB) using the VMX implementations of 'cbc(aes)' or 'xts(aes)'. Such operations then trigger BUG in crypto_yield(): [ 891.863680] BUG: sleeping function called from invalid context at include/crypto/algapi.h:424 [ 891.864622] in_atomic(): 1, irqs_disabled(): 0, pid: 12347, name: kcapi-enc [ 891.864739] 1 lock held by kcapi-enc/12347: [ 891.864811] #0: 00000000f5d42c46 (sk_lock-AF_ALG){+.+.}, at: skcipher_recvmsg+0x50/0x530 [ 891.865076] CPU: 5 PID: 12347 Comm: kcapi-enc Not tainted 4.19.0-0.rc0.git3.1.fc30.ppc64le #1 [ 891.865251] Call Trace: [ 891.865340] [c0000003387578c0] [c000000000d67ea4] dump_stack+0xe8/0x164 (unreliable) [ 891.865511] [c000000338757910] [c000000000172a58] ___might_sleep+0x2f8/0x310 [ 891.865679] [c000000338757990] [c0000000006bff74] blkcipher_walk_done+0x374/0x4a0 [ 891.865825] [c0000003387579e0] [d000000007e73e70] p8_aes_cbc_encrypt+0x1c8/0x260 [vmx_crypto] [ 891.865993] [c000000338757ad0] [c0000000006c0ee0] skcipher_encrypt_blkcipher+0x60/0x80 [ 891.866128] [c000000338757b10] [c0000000006ec504] skcipher_recvmsg+0x424/0x530 [ 891.866283] [c000000338757bd0] [c000000000b00654] sock_recvmsg+0x74/0xa0 [ 891.866403] [c000000338757c10] [c000000000b00f64] ___sys_recvmsg+0xf4/0x2f0 [ 891.866515] [c000000338757d90] [c000000000b02bb8] __sys_recvmsg+0x68/0xe0 [ 891.866631] [c000000338757e30] [c00000000000bbe4] system_call+0x5c/0x70 Fixes: 8c755ace ("crypto: vmx - Adding CBC routines for VMX module") Fixes: c07f5d3d ("crypto: vmx - Adding support for XTS") Cc: stable@vger.kernel.org Signed-off-by:Ondrej Mosnacek <omosnace@redhat.com> Signed-off-by:
Herbert Xu <herbert@gondor.apana.org.au>
-
Ard Biesheuvel authored
Commit 71e52c27 ("crypto: arm64/aes-ce-gcm - operate on two input blocks at a time") modified the granularity at which the AES/GCM code processes its input to allow subsequent changes to be applied that improve performance by using aggregation to process multiple input blocks at once. For this reason, it doubled the algorithm's 'chunksize' property to 2 x AES_BLOCK_SIZE, but retained the non-SIMD fallback path that processes a single block at a time. In some cases, this violates the skcipher scatterwalk API, by calling skcipher_walk_done() with a non-zero residue value for a chunk that is expected to be handled in its entirety. This results in a WARN_ON() to be hit by the TLS self test code, but is likely to break other user cases as well. Unfortunately, none of the current test cases exercises this exact code path at the moment. Fixes: 71e52c27 ("crypto: arm64/aes-ce-gcm - operate on two ...") Reported-by:
Vakul Garg <vakul.garg@nxp.com> Signed-off-by:
Ard Biesheuvel <ard.biesheuvel@linaro.org> Tested-by:
-
Dave Watson authored
A regression was reported bisecting to 1476db2d "Move HashKey computation from stack to gcm_context". That diff moved HashKey computation from the stack, which was explicitly aligned in the asm, to a struct provided from the C code, depending on AESNI_ALIGN_ATTR for alignment. It appears some compilers may not align this struct correctly, resulting in a crash on the movdqa instruction when attempting to encrypt or decrypt data. Fix by using unaligned loads for the HashKeys. On modern hardware there is no perf difference between the unaligned and aligned loads. All other accesses to gcm_context_data already use unaligned loads. Reported-by:
Mauro Rossi <issor.oruam@gmail.com> Fixes: 1476db2d ("Move HashKey computation from stack to gcm_context") Cc: <stable@vger.kernel.org> Signed-off-by:
Dave Watson <davejwatson@fb.com> Signed-off-by:
-
Ganesh Goudar authored
call chtls_free_uld() only for the initialized cdev, this fixes NULL dereference in chtls_free_uld() Signed-off-by:
Ganesh Goudar <ganeshgr@chelsio.com> Signed-off-by:
Atul Gupta <atul.gupta@chelsio.com> Signed-off-by:
-
Ard Biesheuvel authored
ARMv8.2 specifies special instructions for the SM3 cryptographic hash and the SM4 symmetric cipher. While it is unlikely that a core would implement one and not the other, we should only use SM4 instructions if the SM4 CPU feature bit is set, and we currently check the SM3 feature bit instead. So fix that. Fixes: e99ce921 ("crypto: arm64 - add support for SM4...") Cc: <stable@vger.kernel.org> Signed-off-by:
-
Horia Geantă authored
Crypto engine needs some temporary locations in external memory for running RSA decrypt forms 2 and 3 (CRT). These are named "tmp1" and "tmp2" in the PDB. Update DMA mapping direction of tmp1 and tmp2 from TO_DEVICE to BIDIRECTIONAL, since engine needs r/w access. Cc: <stable@vger.kernel.org> # 4.13+ Fixes: 52e26d77 ("crypto: caam - add support for RSA key form 2") Fixes: 4a651b12 ("crypto: caam - add support for RSA key form 3") Signed-off-by:
Horia Geantă <horia.geanta@nxp.com> Signed-off-by:
-
Horia Geantă authored
xts setkey callback returns 0 on some error paths. Fix this by returning -EINVAL. Cc: <stable@vger.kernel.org> # 4.12+ Fixes: b189817c ("crypto: caam/qi - add ablkcipher and authenc algorithms") Signed-off-by:
-
Horia Geantă authored
Descriptor address needs to be swapped to CPU endianness before being DMA unmapped. Cc: <stable@vger.kernel.org> # 4.8+ Fixes: 261ea058 ("crypto: caam - handle core endianness != caam endianness") Reported-by:
Laurentiu Tudor <laurentiu.tudor@nxp.com> Signed-off-by:
-
- 07 Aug, 2018 9 commits
-
-
Ard Biesheuvel authored
Enhance the GHASH implementation that uses 64-bit polynomial multiplication by adding support for 4-way aggregation. This more than doubles the performance, from 2.4 cycles per byte to 1.1 cpb on Cortex-A53. Signed-off-by:
-
Ard Biesheuvel authored
Checking the TIF_NEED_RESCHED flag is disproportionately costly on cores with fast crypto instructions and comparatively slow memory accesses. On algorithms such as GHASH, which executes at ~1 cycle per byte on cores that implement support for 64 bit polynomial multiplication, there is really no need to check the TIF_NEED_RESCHED particularly often, and so we can remove the NEON yield check from the assembler routines. However, unlike the AEAD or skcipher APIs, the shash/ahash APIs take arbitrary input lengths, and so there needs to be some sanity check to ensure that we don't hog the CPU for excessive amounts of time. So let's simply cap the maximum input size that is processed in one go to 64 KB. Signed-off-by:
-
kbuild test robot authored
Fixes: 915e4e84 ("crypto: hisilicon - SEC security accelerator driver") Signed-off-by:
kbuild test robot <fengguang.wu@intel.com> Signed-off-by:
-
Colin Ian King authored
Variable esign is being assigned but is never used hence it is redundant and can be removed. Cleans up clang warning: warning: variable 'esign' set but not used [-Wunused-but-set-variable] Signed-off-by:
Colin Ian King <colin.king@canonical.com> Signed-off-by:
-
Ard Biesheuvel authored
Squeeze out another 5% of performance by minimizing the number of invocations of kernel_neon_begin()/kernel_neon_end() on the common path, which also allows some reloads of the key schedule to be optimized away. The resulting code runs at 2.3 cycles per byte on a Cortex-A53. Signed-off-by:
-
Ard Biesheuvel authored
Implement a faster version of the GHASH transform which amortizes the reduction modulo the characteristic polynomial across two input blocks at a time. On a Cortex-A53, the gcm(aes) performance increases 24%, from 3.0 cycles per byte to 2.4 cpb for large input sizes. Signed-off-by:
-
Ard Biesheuvel authored
Update the core AES/GCM transform and the associated plumbing to operate on 2 AES/GHASH blocks at a time. By itself, this is not expected to result in a noticeable speedup, but it paves the way for reimplementing the GHASH component using 2-way aggregation. Signed-off-by:
-
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6Herbert Xu authored
Merge crypto-2.6 to pick up NEON yield revert.
-
Ard Biesheuvel authored
As it turns out, checking the TIF_NEED_RESCHED flag after each iteration results in a significant performance regression (~10%) when running fast algorithms (i.e., ones that use special instructions and operate in the < 4 cycles per byte range) on in-order cores with comparatively slow memory accesses such as the Cortex-A53. Given the speed of these ciphers, and the fact that the page based nature of the AEAD scatterwalk API guarantees that the core NEON transform is never invoked with more than a single page's worth of input, we can estimate the worst case duration of any resulting scheduling blackout: on a 1 GHz Cortex-A53 running with 64k pages, processing a page's worth of input at 4 cycles per byte results in a delay of ~250 us, which is a reasonable upper bound. So let's remove the yield checks from the fused AES-CCM and AES-GCM routines entirely. This reverts commit 7b67ae4d and partially reverts commit 7c50136a. Fixes: 7c50136a ("crypto: arm64/aes-ghash - yield NEON after every ...") Fixes: 7b67ae4d ("crypto: arm64/aes-ccm - yield NEON after every ...") Signed-off-by:
-
- 03 Aug, 2018 23 commits
-
-
Eric Biggers authored
Make it return -EINVAL if crypto_dh_key_len() is incorrect rather than overflowing the buffer. Signed-off-by:
Eric Biggers <ebiggers@google.com> Signed-off-by:
-
Eric Biggers authored
It was forgotten to increase DH_KPP_SECRET_MIN_SIZE to include 'q_size', causing an out-of-bounds write of 4 bytes in crypto_dh_encode_key(), and an out-of-bounds read of 4 bytes in crypto_dh_decode_key(). Fix it, and fix the lengths of the test vectors to match this. Reported-by: syzbot+6d38d558c25b53b8f4ed@syzkaller.appspotmail.com Fixes: e3fe0ae1 ("crypto: dh - add public key verification test") Signed-off-by:
-
Tom Lendacky authored
Should the PSP initialization fail, the PSP data structure will be freed and the value contained in the sp_device struct set to NULL. At module unload, psp_dev_destroy() does not check if the pointer value is NULL and will end up dereferencing a NULL pointer. Add a pointer check of the psp_data field in the sp_device struct in psp_dev_destroy() and return immediately if it is NULL. Cc: <stable@vger.kernel.org> # 4.16.x- Fixes: 2a6170df ("crypto: ccp: Add Platform Security Processor (PSP) device support") Signed-off-by:
Tom Lendacky <thomas.lendacky@amd.com> Acked-by:
Gary R Hook <gary.hook@amd.com> Signed-off-by:
-
Eric Biggers authored
The 4-way ChaCha20 NEON code implements 16-bit rotates with vrev32.16, but the one-way code (used on remainder blocks) implements it with vshl + vsri, which is slower. Switch the one-way code to vrev32.16 too. Signed-off-by:
-
Gilad Ben-Yossef authored
The ccree driver had a sanity check that we are not asked to encrypt an XTS buffer bigger than a sane sector size since XTS IV needs to include the sector number in the IV so this is not expected in any real use case. Unfortunately, this breaks cryptsetup benchmark test which has a synthetic performance test using 64k buffer of data with the same IV. Remove the sanity check and allow the user to hang themselves and/or run benchmarks if they so wish. Reported-by:
Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by:
Gilad Ben-Yossef <gilad@benyossef.com> Signed-off-by:
-
Gilad Ben-Yossef authored
In certain error path req_ctx->iv was being freed despite not being allocated because it was not initialized to NULL. Rather than play whack a mole with the structure various field, zero it before use. This fixes a kernel panic that may occur if an invalid buffer size was requested triggering the bug above. Fixes: 63ee04c8 ("crypto: ccree - add skcipher support") Reported-by:
-
Gilad Ben-Yossef authored
IV generation is not available via the skcipher interface. Remove the left over support of it from the ablkcipher days. Signed-off-by:
-
Gilad Ben-Yossef authored
Drop the explicit setting of CRYPTO_ALG_TYPE_AEAD or CRYPTO_ALG_TYPE_SKCIPHER flags during alg registration as they are set anyway by the framework. Signed-off-by:
-
Eric Biggers authored
Like the skcipher_walk and blkcipher_walk cases: scatterwalk_done() is only meant to be called after a nonzero number of bytes have been processed, since scatterwalk_pagedone() will flush the dcache of the *previous* page. But in the error case of ablkcipher_walk_done(), e.g. if the input wasn't an integer number of blocks, scatterwalk_done() was actually called after advancing 0 bytes. This caused a crash ("BUG: unable to handle kernel paging request") during '!PageSlab(page)' on architectures like arm and arm64 that define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, provided that the input was page-aligned as in that case walk->offset == 0. Fix it by reorganizing ablkcipher_walk_done() to skip the scatterwalk_advance() and scatterwalk_done() if an error has occurred. Reported-by:Liu Chao <liuchao741@huawei.com> Fixes: bf06099d ("crypto: skcipher - Add ablkcipher_walk interfaces") Cc: <stable@vger.kernel.org> # v2.6.35+ Signed-off-by:
-
Eric Biggers authored
Like the skcipher_walk case: scatterwalk_done() is only meant to be called after a nonzero number of bytes have been processed, since scatterwalk_pagedone() will flush the dcache of the *previous* page. But in the error case of blkcipher_walk_done(), e.g. if the input wasn't an integer number of blocks, scatterwalk_done() was actually called after advancing 0 bytes. This caused a crash ("BUG: unable to handle kernel paging request") during '!PageSlab(page)' on architectures like arm and arm64 that define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, provided that the input was page-aligned as in that case walk->offset == 0. Fix it by reorganizing blkcipher_walk_done() to skip the scatterwalk_advance() and scatterwalk_done() if an error has occurred. This bug was found by syzkaller fuzzing. Reproducer, assuming ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE: #include <linux/if_alg.h> #include <sys/socket.h> #include <unistd.h> int main() { struct sockaddr_alg addr = { .salg_type = "skcipher", .salg_name = "ecb(aes-generic)", }; char buffer[4096] __attribute__((aligned(4096))) = { 0 }; int fd; fd = socket(AF_ALG, SOCK_SEQPACKET, 0); bind(fd, (void *)&addr, sizeof(addr)); setsockopt(fd, SOL_ALG, ALG_SET_KEY, buffer, 16); fd = accept(fd, NULL, NULL); write(fd, buffer, 15); read(fd, buffer, 15); } Reported-by: -
Eric Biggers authored
scatterwalk_done() is only meant to be called after a nonzero number of bytes have been processed, since scatterwalk_pagedone() will flush the dcache of the *previous* page. But in the error case of skcipher_walk_done(), e.g. if the input wasn't an integer number of blocks, scatterwalk_done() was actually called after advancing 0 bytes. This caused a crash ("BUG: unable to handle kernel paging request") during '!PageSlab(page)' on architectures like arm and arm64 that define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, provided that the input was page-aligned as in that case walk->offset == 0. Fix it by reorganizing skcipher_walk_done() to skip the scatterwalk_advance() and scatterwalk_done() if an error has occurred. This bug was found by syzkaller fuzzing. Reproducer, assuming ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE: #include <linux/if_alg.h> #include <sys/socket.h> #include <unistd.h> int main() { struct sockaddr_alg addr = { .salg_type = "skcipher", .salg_name = "cbc(aes-generic)", }; char buffer[4096] __attribute__((aligned(4096))) = { 0 }; int fd; fd = socket(AF_ALG, SOCK_SEQPACKET, 0); bind(fd, (void *)&addr, sizeof(addr)); setsockopt(fd, SOL_ALG, ALG_SET_KEY, buffer, 16); fd = accept(fd, NULL, NULL); write(fd, buffer, 15); read(fd, buffer, 15); } Reported-by: -
Eric Biggers authored
Setting 'walk->nbytes = walk->total' in skcipher_walk_first() doesn't make sense because actually walk->nbytes needs to be set to the length of the first step in the walk, which may be less than walk->total. This is done by skcipher_walk_next() which is called immediately afterwards. Also walk->nbytes was already set to 0 in skcipher_walk_skcipher(), which is a better default value in case it's forgotten to be set later. Therefore, remove the unnecessary assignment to walk->nbytes. Signed-off-by:
-
Eric Biggers authored
scatterwalk_samebuf() is never used. Remove it. Signed-off-by:
-
Eric Biggers authored
All callers pass chain=0 to scatterwalk_crypto_chain(). Remove this unneeded parameter. Signed-off-by:
-
Eric Biggers authored
The ALIGN() macro needs to be passed the alignment, not the alignmask (which is the alignment minus 1). Fixes: b286d8b1 ("crypto: skcipher - Add skcipher walk interface") Cc: <stable@vger.kernel.org> # v4.10+ Signed-off-by:
-
Jonathan Cameron authored
Enable all 4 SEC units available on d05 boards. Signed-off-by:
Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by:
-
Jonathan Cameron authored
This accelerator is found inside hisilicon hip06 and hip07 SoCs. Each instance provides a number of queues which feed a different number of backend acceleration units. The queues are operating in an out of order mode in the interests of throughput. The silicon does not do tracking of dependencies between multiple 'messages' or update of the IVs as appropriate for training. Hence where relevant we need to do this in software. Signed-off-by:
-
Jonathan Cameron authored
The hip06 and hip07 SoCs contain a number of these crypto units which accelerate AES and DES operations. Signed-off-by:
Rob Herring <robh@kernel.org> Signed-off-by:
-
Horia Geantă authored
Avoid RCU stalls in the case of non-preemptible kernel and lengthy speed tests by rescheduling when advancing from one block size to another. Signed-off-by:
-
Jia-Ju Bai authored
__virtio_crypto_ablkcipher_do_req() is never called in atomic context. __virtio_crypto_ablkcipher_do_req() is only called by virtio_crypto_ablkcipher_crypt_req(), which is only called by virtcrypto_find_vqs() that is never called in atomic context. __virtio_crypto_ablkcipher_do_req() calls kzalloc_node() with GFP_ATOMIC, which is not necessary. GFP_ATOMIC can be replaced with GFP_KERNEL. This is found by a static analysis tool named DCNS written by myself. I also manually check the kernel code before reporting it. Signed-off-by:
Jia-Ju Bai <baijiaju1990@gmail.com> Signed-off-by:
-
Jia-Ju Bai authored
adf_dev_aer_schedule_reset() is never called in atomic context, as it calls wait_for_completion_timeout(). adf_dev_aer_schedule_reset() calls kzalloc() with GFP_ATOMIC, which is not necessary. GFP_ATOMIC can be replaced with GFP_KERNEL. This is found by a static analysis tool named DCNS written by myself. I also manually check the kernel code before reporting it. Signed-off-by:
-
Jia-Ju Bai authored
crypto_alloc_context() is only called by nitrox_skcipher_init(), which is never called in atomic context. crypto_alloc_context() calls dma_pool_alloc() with GFP_ATOMIC, which is not necessary. GFP_ATOMIC can be replaced with GFP_KERNEL. This is found by a static analysis tool named DCNS written by myself. I also manually check the kernel code before reporting it. Signed-off-by:
-
Stephan Müller authored
The cipher implementations of the kernel crypto API favor in-place cipher operations. Thus, switch the CTR cipher operation in the DRBG to perform in-place operations. This is implemented by using the output buffer as input buffer and zeroizing it before the cipher operation to implement a CTR encryption of a NULL buffer. The speed improvement is quite visibile with the following comparison using the LRNG implementation. Without the patch set: 16 bytes| 12.267661 MB/s| 61338304 bytes | 5000000213 ns 32 bytes| 23.603770 MB/s| 118018848 bytes | 5000000073 ns 64 bytes| 46.732262 MB/s| 233661312 bytes | 5000000241 ns 128 bytes| 90.038042 MB/s| 450190208 bytes | 5000000244 ns 256 bytes| 160.399616 MB/s| 801998080 bytes | 5000000393 ns 512 bytes| 259.878400 MB/s| 1299392000 bytes | 5000001675 ns 1024 bytes| 386.050662 MB/s| 1930253312 bytes | 5000001661 ns 2048 bytes| 493.641728 MB/s| 2468208640 bytes | 5000001598 ns 4096 bytes| 581.835981 MB/s| 2909179904 bytes | 5000003426 ns With the patch set: 16 bytes | 17.051142 MB/s | 85255712 bytes | 5000000854 ns 32 bytes | 32.695898 MB/s | 163479488 bytes | 5000000544 ns 64 bytes | 64.490739 MB/s | 322453696 bytes | 5000000954 ns 128 bytes | 123.285043 MB/s | 616425216 bytes | 5000000201 ns 256 bytes | 233.434573 MB/s | 1167172864 bytes | 5000000573 ns 512 bytes | 384.405197 MB/s | 1922025984 bytes | 5000000671 ns 1024 bytes | 566.313370 MB/s | 2831566848 bytes | 5000001080 ns 2048 bytes | 744.518042 MB/s | 3722590208 bytes | 5000000926 ns 4096 bytes | 867.501670 MB/s | 4337508352 bytes | 5000002181 ns Signed-off-by:Stephan Mueller <smueller@chronox.de> Signed-off-by:
-
