* cryptodev linux-next splat
@ 2025-05-16 11:22 Borislav Petkov
2025-05-16 11:34 ` Herbert Xu
0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2025-05-16 11:22 UTC (permalink / raw)
To: Herbert Xu; +Cc: Jain, Ayush, Stephen Rothwell, x86-ml, lkml, linux-crypto
Hi Herbert,
one of our linux-next tests which hotplugs a CPU fails with the below splat.
Reproducing is very easy:
# echo 0 > /sys/devices/system/cpu/cpu1/online
# echo 1 > /sys/devices/system/cpu/cpu1/online
Opcode bytes point to:
02:06:54 [ 3199.416779] Code: 65 c6 05 9a c8 ad 02 01 f7 47 2c 00 40 20 00 74 4f 65 48 c7 05 95 c8 ad 02 00 00 00 00 f6 c3 02 74 0d c7 44 24 04 80 1f 00 00 <0f> ae 54 24 04 83 e3 01 75 47 48 8b 44 24 08 65 48 2b 05 49 97 ad
All code
========
0: 65 c6 05 9a c8 ad 02 movb $0x1,%gs:0x2adc89a(%rip) # 0x2adc8a2
7: 01
8: f7 47 2c 00 40 20 00 testl $0x204000,0x2c(%rdi)
f: 74 4f je 0x60
11: 65 48 c7 05 95 c8 ad movq $0x0,%gs:0x2adc895(%rip) # 0x2adc8b2
18: 02 00 00 00 00
1d: f6 c3 02 test $0x2,%bl
20: 74 0d je 0x2f
22: c7 44 24 04 80 1f 00 movl $0x1f80,0x4(%rsp)
29: 00
2a:* 0f ae 54 24 04 ldmxcsr 0x4(%rsp) <-- trapping instruction
2f: 83 e3 01 and $0x1,%ebx
32: 75 47 jne 0x7b
34: 48 8b 44 24 08 mov 0x8(%rsp),%rax
39: 65 gs
3a: 48 rex.W
3b: 2b .byte 0x2b
3c: 05 .byte 0x5
3d: 49 97 xchg %rax,%r15
3f: ad lods %ds:(%rsi),%eax
Code starting with the faulting instruction
===========================================
0: 0f ae 54 24 04 ldmxcsr 0x4(%rsp)
5: 83 e3 01 and $0x1,%ebx
8: 75 47 jne 0x51
a: 48 8b 44 24 08 mov 0x8(%rsp),%rax
f: 65 gs
10: 48 rex.W
11: 2b .byte 0x2b
12: 05 .byte 0x5
13: 49 97 xchg %rax,%r15
15: ad lods %ds:(%rsi),%eax
And LDMXCSR would #UD for a bunch of conditions.
Reverting cryptodev from linux-next next-20250515 this way:
$ git revert -m 1 ed18a632e45785e3392cf96b9683ca033a74b1f8
fixes the issue so I'm thinking it must be one of the patches you guys have
there.
Ideas?
Thx.
02:05:36 [ 3129.220448] systemd[1]: systemd-timedated.service: Deactivated successfully.
02:05:36 [ 3129.252398] systemd[1]: systemd-hostnamed.service: Deactivated successfully.
02:06:54 [ 3199.347946] smpboot: CPU 1 is now offline
02:06:54 [ 3199.375693] smpboot: Booting Node 0 Processor 1 APIC 0x2
02:06:54 [ 3199.381723] Oops: invalid opcode: 0000 [#1] SMP NOPTI
02:06:54 [ 3199.387364] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Kdump: loaded Not tainted 6.15.0-rc6-next-20250515-484803582c77-1747374910702 #1 PREEMPT(voluntary)
02:06:54 [ 3199.402308] Hardware name: AMD Corporation Cinnabar/Cinnabar, BIOS RCB100DB 08/09/2024
02:06:54 [ 3199.411140] RIP: 0010:kernel_fpu_begin_mask+0x58/0xc0
02:06:54 [ 3199.416779] Code: 65 c6 05 9a c8 ad 02 01 f7 47 2c 00 40 20 00 74 4f 65 48 c7 05 95 c8 ad 02 00 00 00 00 f6 c3 02 74 0d c7 44 24 04 80 1f 00 00 <0f> ae 54 24 04 83 e3 01 75 47 48 8b 44 24 08 65 48 2b 05 49 97 ad
02:06:54 [ 3199.437736] RSP: 0000:ff3a2270c019fd98 EFLAGS: 00010002
02:06:54 [ 3199.443568] RAX: 0000000000000046 RBX: 0000000000000002 RCX: 0000000000000000
02:06:54 [ 3199.451528] RDX: 0000000000000057 RSI: ff380c5800eba000 RDI: ff380c5800eec280
02:06:54 [ 3199.459487] RBP: ff3a2270c019fe30 R08: 0000000000000000 R09: ff380c8684245078
02:06:54 [ 3199.467446] R10: 0000000000000000 R11: 0000000000000000 R12: ff380c5800eba000
02:06:54 [ 3199.475405] R13: 0000000000000057 R14: 00000000000015c0 R15: 00000000000015c0
02:06:54 [ 3199.483364] FS: 0000000000000000(0000) GS:ff380c86d4935000(0000) knlGS:0000000000000000
02:06:54 [ 3199.492391] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
02:06:54 [ 3199.498801] CR2: 0000000000000000 CR3: 000000304b82a000 CR4: 00000000003318b0
02:06:54 [ 3199.506761] Call Trace:
02:06:54 [ 3199.509487] <TASK>
02:06:54 [ 3199.511825] sha256_blocks_simd+0x23/0x50
02:06:54 [ 3199.516303] sha256_update+0x73/0x100
02:06:54 [ 3199.520381] sha256+0x70/0xa0
02:06:54 [ 3199.523690] ? __smp_call_single_queue+0xb0/0x120
02:06:54 [ 3199.528939] ? srso_alias_return_thunk+0x5/0xfbef5
02:06:54 [ 3199.534285] ? bsearch+0x57/0x90
02:06:54 [ 3199.537884] ? __pfx_cmp_id+0x10/0x10
02:06:54 [ 3199.541968] __apply_microcode_amd+0xf1/0x1c0
02:06:54 [ 3199.546827] ? srso_alias_return_thunk+0x5/0xfbef5
02:06:54 [ 3199.552169] ? srso_alias_return_thunk+0x5/0xfbef5
02:06:54 [ 3199.557511] ? cpu_init_exception_handling+0x1fe/0x2c0
02:06:54 [ 3199.563241] ? srso_alias_return_thunk+0x5/0xfbef5
02:06:54 [ 3199.568584] apply_microcode_amd+0xca/0x110
02:06:54 [ 3199.573251] start_secondary+0x24/0x140
02:06:54 [ 3199.577531] ? srso_alias_return_thunk+0x5/0xfbef5
02:06:54 [ 3199.582872] common_startup_64+0x13e/0x141
02:06:54 [ 3199.587446] </TASK>
02:06:54 [ 3199.589877] Modules linked in: iscsi_target_mod target_core_mod binfmt_misc xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nft_compat nf_nat_tftp nf_conntrack_tftp overlay snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set nf_tables nfnetlink sunrpc vfat fat amd_atl intel_rapl_msr intel_rapl_common amd64_edac edac_mce_amd kvm_amd ipmi_ssif kvm mlx5_ib irqbypass cdc_ether ib_uverbs usbnet rapl mii wmi_bmof pcspkr dax_hmem acpi_cpufreq ib_core acpi_ipmi i2c_piix4 k10temp i2c_smbus ipmi_si ipmi_devintf ipmi_msghandler i2c_designware_platform i2c_designware_core sch_fq_codel xfs ast drm_client_lib i2c_algo_bit drm_shmem_helper mlx5_core ahci drm_kms_helper libahci mlxfw tls nvme ghash_clmulni_intel sha512_ssse3 drm tg3 psample pci_hyperv_intf libata ccp nvme_core sp5100_tco wmi dm_mirror dm_region_hash dm_log
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: cryptodev linux-next splat
2025-05-16 11:22 cryptodev linux-next splat Borislav Petkov
@ 2025-05-16 11:34 ` Herbert Xu
2025-05-16 11:48 ` [PATCH] crypto: lib/sha256 - Disable SIMD Herbert Xu
0 siblings, 1 reply; 8+ messages in thread
From: Herbert Xu @ 2025-05-16 11:34 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jain, Ayush, Stephen Rothwell, x86-ml, lkml, linux-crypto,
Eric Biggers
On Fri, May 16, 2025 at 01:22:17PM +0200, Borislav Petkov wrote:
> Hi Herbert,
>
> one of our linux-next tests which hotplugs a CPU fails with the below splat.
>
> Reproducing is very easy:
>
> # echo 0 > /sys/devices/system/cpu/cpu1/online
> # echo 1 > /sys/devices/system/cpu/cpu1/online
>
> Opcode bytes point to:
>
> 02:06:54 [ 3199.416779] Code: 65 c6 05 9a c8 ad 02 01 f7 47 2c 00 40 20 00 74 4f 65 48 c7 05 95 c8 ad 02 00 00 00 00 f6 c3 02 74 0d c7 44 24 04 80 1f 00 00 <0f> ae 54 24 04 83 e3 01 75 47 48 8b 44 24 08 65 48 2b 05 49 97 ad
> All code
> ========
> 0: 65 c6 05 9a c8 ad 02 movb $0x1,%gs:0x2adc89a(%rip) # 0x2adc8a2
> 7: 01
> 8: f7 47 2c 00 40 20 00 testl $0x204000,0x2c(%rdi)
> f: 74 4f je 0x60
> 11: 65 48 c7 05 95 c8 ad movq $0x0,%gs:0x2adc895(%rip) # 0x2adc8b2
> 18: 02 00 00 00 00
> 1d: f6 c3 02 test $0x2,%bl
> 20: 74 0d je 0x2f
> 22: c7 44 24 04 80 1f 00 movl $0x1f80,0x4(%rsp)
> 29: 00
> 2a:* 0f ae 54 24 04 ldmxcsr 0x4(%rsp) <-- trapping instruction
> 2f: 83 e3 01 and $0x1,%ebx
> 32: 75 47 jne 0x7b
> 34: 48 8b 44 24 08 mov 0x8(%rsp),%rax
> 39: 65 gs
> 3a: 48 rex.W
> 3b: 2b .byte 0x2b
> 3c: 05 .byte 0x5
> 3d: 49 97 xchg %rax,%r15
> 3f: ad lods %ds:(%rsi),%eax
>
> Code starting with the faulting instruction
> ===========================================
> 0: 0f ae 54 24 04 ldmxcsr 0x4(%rsp)
> 5: 83 e3 01 and $0x1,%ebx
> 8: 75 47 jne 0x51
> a: 48 8b 44 24 08 mov 0x8(%rsp),%rax
> f: 65 gs
> 10: 48 rex.W
> 11: 2b .byte 0x2b
> 12: 05 .byte 0x5
> 13: 49 97 xchg %rax,%r15
> 15: ad lods %ds:(%rsi),%eax
>
> And LDMXCSR would #UD for a bunch of conditions.
>
> Reverting cryptodev from linux-next next-20250515 this way:
>
> $ git revert -m 1 ed18a632e45785e3392cf96b9683ca033a74b1f8
>
> fixes the issue so I'm thinking it must be one of the patches you guys have
> there.
>
> Ideas?
Yes probably.
So what's happened is that previously if you call sha256_update
from lib/crypto it would only use the generic C code to perform
the operation.
This has now been changed to automatically use SIMD instructions
which obviously blew up in your case.
Eric, can you please take a look at this crash? It looks like
crypto_simd_usable returned true, but kernel_fpu_begin still
crashed for some reason. One of them must be wrong :)
Thanks,
> 02:05:36 [ 3129.220448] systemd[1]: systemd-timedated.service: Deactivated successfully.
> 02:05:36 [ 3129.252398] systemd[1]: systemd-hostnamed.service: Deactivated successfully.
> 02:06:54 [ 3199.347946] smpboot: CPU 1 is now offline
> 02:06:54 [ 3199.375693] smpboot: Booting Node 0 Processor 1 APIC 0x2
> 02:06:54 [ 3199.381723] Oops: invalid opcode: 0000 [#1] SMP NOPTI
> 02:06:54 [ 3199.387364] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Kdump: loaded Not tainted 6.15.0-rc6-next-20250515-484803582c77-1747374910702 #1 PREEMPT(voluntary)
> 02:06:54 [ 3199.402308] Hardware name: AMD Corporation Cinnabar/Cinnabar, BIOS RCB100DB 08/09/2024
> 02:06:54 [ 3199.411140] RIP: 0010:kernel_fpu_begin_mask+0x58/0xc0
> 02:06:54 [ 3199.416779] Code: 65 c6 05 9a c8 ad 02 01 f7 47 2c 00 40 20 00 74 4f 65 48 c7 05 95 c8 ad 02 00 00 00 00 f6 c3 02 74 0d c7 44 24 04 80 1f 00 00 <0f> ae 54 24 04 83 e3 01 75 47 48 8b 44 24 08 65 48 2b 05 49 97 ad
> 02:06:54 [ 3199.437736] RSP: 0000:ff3a2270c019fd98 EFLAGS: 00010002
> 02:06:54 [ 3199.443568] RAX: 0000000000000046 RBX: 0000000000000002 RCX: 0000000000000000
> 02:06:54 [ 3199.451528] RDX: 0000000000000057 RSI: ff380c5800eba000 RDI: ff380c5800eec280
> 02:06:54 [ 3199.459487] RBP: ff3a2270c019fe30 R08: 0000000000000000 R09: ff380c8684245078
> 02:06:54 [ 3199.467446] R10: 0000000000000000 R11: 0000000000000000 R12: ff380c5800eba000
> 02:06:54 [ 3199.475405] R13: 0000000000000057 R14: 00000000000015c0 R15: 00000000000015c0
> 02:06:54 [ 3199.483364] FS: 0000000000000000(0000) GS:ff380c86d4935000(0000) knlGS:0000000000000000
> 02:06:54 [ 3199.492391] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> 02:06:54 [ 3199.498801] CR2: 0000000000000000 CR3: 000000304b82a000 CR4: 00000000003318b0
> 02:06:54 [ 3199.506761] Call Trace:
> 02:06:54 [ 3199.509487] <TASK>
> 02:06:54 [ 3199.511825] sha256_blocks_simd+0x23/0x50
> 02:06:54 [ 3199.516303] sha256_update+0x73/0x100
> 02:06:54 [ 3199.520381] sha256+0x70/0xa0
> 02:06:54 [ 3199.523690] ? __smp_call_single_queue+0xb0/0x120
> 02:06:54 [ 3199.528939] ? srso_alias_return_thunk+0x5/0xfbef5
> 02:06:54 [ 3199.534285] ? bsearch+0x57/0x90
> 02:06:54 [ 3199.537884] ? __pfx_cmp_id+0x10/0x10
> 02:06:54 [ 3199.541968] __apply_microcode_amd+0xf1/0x1c0
> 02:06:54 [ 3199.546827] ? srso_alias_return_thunk+0x5/0xfbef5
> 02:06:54 [ 3199.552169] ? srso_alias_return_thunk+0x5/0xfbef5
> 02:06:54 [ 3199.557511] ? cpu_init_exception_handling+0x1fe/0x2c0
> 02:06:54 [ 3199.563241] ? srso_alias_return_thunk+0x5/0xfbef5
> 02:06:54 [ 3199.568584] apply_microcode_amd+0xca/0x110
> 02:06:54 [ 3199.573251] start_secondary+0x24/0x140
> 02:06:54 [ 3199.577531] ? srso_alias_return_thunk+0x5/0xfbef5
> 02:06:54 [ 3199.582872] common_startup_64+0x13e/0x141
> 02:06:54 [ 3199.587446] </TASK>
> 02:06:54 [ 3199.589877] Modules linked in: iscsi_target_mod target_core_mod binfmt_misc xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nft_compat nf_nat_tftp nf_conntrack_tftp overlay snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set nf_tables nfnetlink sunrpc vfat fat amd_atl intel_rapl_msr intel_rapl_common amd64_edac edac_mce_amd kvm_amd ipmi_ssif kvm mlx5_ib irqbypass cdc_ether ib_uverbs usbnet rapl mii wmi_bmof pcspkr dax_hmem acpi_cpufreq ib_core acpi_ipmi i2c_piix4 k10temp i2c_smbus ipmi_si ipmi_devintf ipmi_msghandler i2c_designware_platform i2c_designware_core sch_fq_codel xfs ast drm_client_lib i2c_algo_bit drm_shmem_helper mlx5_core ahci drm_kms_helper libahci mlxfw tls nvme ghash_clmulni_intel sha512_ssse3 drm tg3 psample pci_hyperv_intf libata ccp nvme_core sp5100_tco wmi dm_mirror dm_region_hash dm_log
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] crypto: lib/sha256 - Disable SIMD
2025-05-16 11:34 ` Herbert Xu
@ 2025-05-16 11:48 ` Herbert Xu
2025-05-16 12:27 ` Borislav Petkov
2025-05-16 17:03 ` Eric Biggers
0 siblings, 2 replies; 8+ messages in thread
From: Herbert Xu @ 2025-05-16 11:48 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jain, Ayush, Stephen Rothwell, x86-ml, lkml, linux-crypto,
Eric Biggers
On Fri, May 16, 2025 at 07:34:06PM +0800, Herbert Xu wrote:
>
> So what's happened is that previously if you call sha256_update
> from lib/crypto it would only use the generic C code to perform
> the operation.
>
> This has now been changed to automatically use SIMD instructions
> which obviously blew up in your case.
In the interim you can go back to the old ways and disable SIMD
for lib/crypto sha256 with this patch:
---8<---
Disable SIMD usage in lib/crypto sha256 as it is causing crashes.
Reported-by: Borislav Petkov <bp@alien8.de>
Fixes: 950e5c84118c ("crypto: sha256 - support arch-optimized lib and expose through shash")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/include/crypto/internal/sha2.h b/include/crypto/internal/sha2.h
index b9bccd3ff57f..e1b0308c0539 100644
--- a/include/crypto/internal/sha2.h
+++ b/include/crypto/internal/sha2.h
@@ -32,7 +32,7 @@ static inline void sha256_choose_blocks(
if (!IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_SHA256) || force_generic)
sha256_blocks_generic(state, data, nblocks);
else if (IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_SHA256_SIMD) &&
- (force_simd || crypto_simd_usable()))
+ force_simd)
sha256_blocks_simd(state, data, nblocks);
else
sha256_blocks_arch(state, data, nblocks);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto: lib/sha256 - Disable SIMD
2025-05-16 11:48 ` [PATCH] crypto: lib/sha256 - Disable SIMD Herbert Xu
@ 2025-05-16 12:27 ` Borislav Petkov
2025-05-16 17:03 ` Eric Biggers
1 sibling, 0 replies; 8+ messages in thread
From: Borislav Petkov @ 2025-05-16 12:27 UTC (permalink / raw)
To: Herbert Xu
Cc: Ayush Jain, Stephen Rothwell, x86-ml, lkml, linux-crypto,
Eric Biggers
On Fri, May 16, 2025 at 07:48:52PM +0800, Herbert Xu wrote:
> On Fri, May 16, 2025 at 07:34:06PM +0800, Herbert Xu wrote:
> >
> > So what's happened is that previously if you call sha256_update
> > from lib/crypto it would only use the generic C code to perform
> > the operation.
> >
> > This has now been changed to automatically use SIMD instructions
> > which obviously blew up in your case.
>
> In the interim you can go back to the old ways and disable SIMD
> for lib/crypto sha256 with this patch:
>
> ---8<---
> Disable SIMD usage in lib/crypto sha256 as it is causing crashes.
>
> Reported-by: Borislav Petkov <bp@alien8.de>
Please make that
Reported-by: Ayush Jain <Ayush.Jain3@amd.com>
I'm just the messenger.
> Fixes: 950e5c84118c ("crypto: sha256 - support arch-optimized lib and expose through shash")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/include/crypto/internal/sha2.h b/include/crypto/internal/sha2.h
> index b9bccd3ff57f..e1b0308c0539 100644
> --- a/include/crypto/internal/sha2.h
> +++ b/include/crypto/internal/sha2.h
> @@ -32,7 +32,7 @@ static inline void sha256_choose_blocks(
> if (!IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_SHA256) || force_generic)
> sha256_blocks_generic(state, data, nblocks);
> else if (IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_SHA256_SIMD) &&
> - (force_simd || crypto_simd_usable()))
> + force_simd)
> sha256_blocks_simd(state, data, nblocks);
> else
> sha256_blocks_arch(state, data, nblocks);
> --
If you end up doing this, that fixes it, obviously:
Tested-by: Ayush Jain <Ayush.Jain3@amd.com>
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto: lib/sha256 - Disable SIMD
2025-05-16 11:48 ` [PATCH] crypto: lib/sha256 - Disable SIMD Herbert Xu
2025-05-16 12:27 ` Borislav Petkov
@ 2025-05-16 17:03 ` Eric Biggers
2025-05-16 18:13 ` Borislav Petkov
1 sibling, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2025-05-16 17:03 UTC (permalink / raw)
To: Herbert Xu
Cc: Borislav Petkov, Jain, Ayush, Stephen Rothwell, x86-ml, lkml,
linux-crypto
On Fri, May 16, 2025 at 07:48:52PM +0800, Herbert Xu wrote:
> On Fri, May 16, 2025 at 07:34:06PM +0800, Herbert Xu wrote:
> >
> > So what's happened is that previously if you call sha256_update
> > from lib/crypto it would only use the generic C code to perform
> > the operation.
> >
> > This has now been changed to automatically use SIMD instructions
> > which obviously blew up in your case.
>
> In the interim you can go back to the old ways and disable SIMD
> for lib/crypto sha256 with this patch:
>
> ---8<---
> Disable SIMD usage in lib/crypto sha256 as it is causing crashes.
>
> Reported-by: Borislav Petkov <bp@alien8.de>
> Fixes: 950e5c84118c ("crypto: sha256 - support arch-optimized lib and expose through shash")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
That's silly. We should just fix x86's irq_fpu_usable() to return false before
the CPU is properly initialized. It already checks a per-cpu bool, so it
shouldn't be too hard to fit that in.
Using the generic SHA-256 code explicitly is also an option, but ideally the
regular functions would just work.
- Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto: lib/sha256 - Disable SIMD
2025-05-16 17:03 ` Eric Biggers
@ 2025-05-16 18:13 ` Borislav Petkov
2025-05-16 19:06 ` Eric Biggers
0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2025-05-16 18:13 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, Jain, Ayush, Stephen Rothwell, x86-ml, lkml,
linux-crypto
On Fri, May 16, 2025 at 10:03:16AM -0700, Eric Biggers wrote:
> That's silly. We should just fix x86's irq_fpu_usable() to return false
> before the CPU is properly initialized. It already checks a per-cpu bool, so
> it shouldn't be too hard to fit that in.
Probably.
There's a fpu__init_cpu() call almost right after load_ucode_ap() which causes
this thing.
I'm not sure how much initialized stuff you need for SHA256 SIMD... perhaps
swap fpu__init_cpu() and load_ucode_ap() calls after proper code audit whether
that's ok.
Or add a "is the FPU initialized" check, as you propose, which is probably
easier.
As always, the x86 CPU init path is nasty and needs careful auditing.
> Using the generic SHA-256 code explicitly is also an option,
Or that.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto: lib/sha256 - Disable SIMD
2025-05-16 18:13 ` Borislav Petkov
@ 2025-05-16 19:06 ` Eric Biggers
2025-05-16 20:34 ` Thomas Gleixner
0 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2025-05-16 19:06 UTC (permalink / raw)
To: Borislav Petkov
Cc: Herbert Xu, Jain, Ayush, Stephen Rothwell, x86-ml, lkml,
linux-crypto
On Fri, May 16, 2025 at 08:13:22PM +0200, Borislav Petkov wrote:
> On Fri, May 16, 2025 at 10:03:16AM -0700, Eric Biggers wrote:
> > That's silly. We should just fix x86's irq_fpu_usable() to return false
> > before the CPU is properly initialized. It already checks a per-cpu bool, so
> > it shouldn't be too hard to fit that in.
>
> Probably.
>
> There's a fpu__init_cpu() call almost right after load_ucode_ap() which causes
> this thing.
>
> I'm not sure how much initialized stuff you need for SHA256 SIMD... perhaps
> swap fpu__init_cpu() and load_ucode_ap() calls after proper code audit whether
> that's ok.
>
> Or add a "is the FPU initialized" check, as you propose, which is probably
> easier.
>
> As always, the x86 CPU init path is nasty and needs careful auditing.
There are a few different ways in which __apply_microcode_amd() can get called:
__apply_microcode_amd
load_ucode_amd_bsp
load_ucode_bsp
x86_64_start_kernel
apply_microcode_amd
load_ucode_amd_ap
load_ucode_ap
start_secondary
microcode_ops::apply_microcode
load_secondary
__load_primary
reload_ucode_amd
reload_early_microcode
microcode_bsp_resume
mc_syscore_ops::resume
syscore_resume
__restore_processor_state
restore_processor_state
What would you say about going back to my earlier plan to make irq_fpu_usable()
return false when irqs_disabled(), like what arm64 does? (As I had in
https://lore.kernel.org/lkml/20250220051325.340691-2-ebiggers@kernel.org/).
I think that would handle all these cases, as well as others. We'd need to fix
__save_processor_state() to save the FPU state directly without pretending that
it's using kernel-mode FPU, but I don't know of any issues besides that. Then
we could also delete the irqs_disabled() checks that I added to
kernel_fpu_begin() and kernel_fpu_end().
- Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] crypto: lib/sha256 - Disable SIMD
2025-05-16 19:06 ` Eric Biggers
@ 2025-05-16 20:34 ` Thomas Gleixner
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Gleixner @ 2025-05-16 20:34 UTC (permalink / raw)
To: Eric Biggers, Borislav Petkov
Cc: Herbert Xu, Jain, Ayush, Stephen Rothwell, x86-ml, lkml,
linux-crypto
On Fri, May 16 2025 at 12:06, Eric Biggers wrote:
> What would you say about going back to my earlier plan to make irq_fpu_usable()
> return false when irqs_disabled(), like what arm64 does? (As I had in
> https://lore.kernel.org/lkml/20250220051325.340691-2-ebiggers@kernel.org/).
> + return !this_cpu_read(in_kernel_fpu) &&
> + !in_hardirq() && !irqs_disabled() && !in_nmi();
The !in_hardirq() is redundant because hard interrupt context runs with
interrupts disabled.
> I think that would handle all these cases, as well as others. We'd need to fix
> __save_processor_state() to save the FPU state directly without pretending that
> it's using kernel-mode FPU, but I don't know of any issues besides
> that.
Looks about right.
> Then we could also delete the irqs_disabled() checks that I added to
> kernel_fpu_begin() and kernel_fpu_end().
Yes. That conditional locking is horrible.
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-05-16 20:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-16 11:22 cryptodev linux-next splat Borislav Petkov
2025-05-16 11:34 ` Herbert Xu
2025-05-16 11:48 ` [PATCH] crypto: lib/sha256 - Disable SIMD Herbert Xu
2025-05-16 12:27 ` Borislav Petkov
2025-05-16 17:03 ` Eric Biggers
2025-05-16 18:13 ` Borislav Petkov
2025-05-16 19:06 ` Eric Biggers
2025-05-16 20:34 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox