public inbox for linux-crypto@vger.kernel.org
 help / color / mirror / Atom feed
* 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