diff mbox series

crypto: powerpc/poly1305 - add depends on BROKEN for now

Message ID 20250514051847.193996-1-ebiggers@kernel.org (mailing list archive)
State Handled Elsewhere
Headers show
Series crypto: powerpc/poly1305 - add depends on BROKEN for now | expand

Commit Message

Eric Biggers May 14, 2025, 5:18 a.m. UTC
From: Eric Biggers <ebiggers@google.com>

As discussed in the thread containing
https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
Power10-optimized Poly1305 code is currently not safe to call in softirq
context.  Disable it for now.  It can be re-enabled once it is fixed.

Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 arch/powerpc/lib/crypto/Kconfig | 1 +
 1 file changed, 1 insertion(+)


base-commit: 57999ed153ed7e651afecbabe0e998e75cf2d798

Comments

Herbert Xu May 14, 2025, 9:41 a.m. UTC | #1
Eric Biggers <ebiggers@kernel.org> wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> As discussed in the thread containing
> https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
> Power10-optimized Poly1305 code is currently not safe to call in softirq
> context.  Disable it for now.  It can be re-enabled once it is fixed.
> 
> Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> arch/powerpc/lib/crypto/Kconfig | 1 +
> 1 file changed, 1 insertion(+)

I thought this fix should be enough, no?

https://patchwork.kernel.org/project/linux-crypto/patch/aB8Yy0JGvoErc0ns@gondor.apana.org.au/

Cheers,
Eric Biggers May 14, 2025, 4:29 p.m. UTC | #2
On Wed, May 14, 2025 at 05:41:39PM +0800, Herbert Xu wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > As discussed in the thread containing
> > https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
> > Power10-optimized Poly1305 code is currently not safe to call in softirq
> > context.  Disable it for now.  It can be re-enabled once it is fixed.
> > 
> > Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > arch/powerpc/lib/crypto/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> 
> I thought this fix should be enough, no?
> 
> https://patchwork.kernel.org/project/linux-crypto/patch/aB8Yy0JGvoErc0ns@gondor.apana.org.au/

I didn't notice that.  Probably, though I don't have time to review this subtle
Poly1305 code.  Especially with all the weird unions in the code.  Would be
great if the PowerPC folks would take a look.

- Eric
Herbert Xu May 15, 2025, 2:10 a.m. UTC | #3
On Wed, May 14, 2025 at 09:29:33AM -0700, Eric Biggers wrote:
>
> I didn't notice that.  Probably, though I don't have time to review this subtle
> Poly1305 code.  Especially with all the weird unions in the code.  Would be
> great if the PowerPC folks would take a look.

Of course more reviews would be great and I think they're all on
the cc list.

I did test this by manually forcing the conversion, which is how
I discovered that powerpc wasn't even using donna64.

Cheers,
Christophe Leroy May 19, 2025, 1:55 p.m. UTC | #4
Le 15/05/2025 à 04:10, Herbert Xu a écrit :
> On Wed, May 14, 2025 at 09:29:33AM -0700, Eric Biggers wrote:
>>
>> I didn't notice that.  Probably, though I don't have time to review this subtle
>> Poly1305 code.  Especially with all the weird unions in the code.  Would be
>> great if the PowerPC folks would take a look.
> 
> Of course more reviews would be great and I think they're all on
> the cc list.
> 
> I did test this by manually forcing the conversion, which is how
> I discovered that powerpc wasn't even using donna64.
> 

As far as I can see related patches found in linux-next tree were not 
sent to linuxppc-dev@lists.ozlabs.org.

Could you resend them, and split out the introduction of 
CONFIG_ARCH_SUPPORTS_INT128 from other parts of patch "crypto: 
powerpc/poly1305 - Add SIMD fallback" and add the lib/tishift.S in the 
patch which adds CONFIG_ARCH_SUPPORTS_INT128 ?

Thanks
Christophe
Herbert Xu May 20, 2025, 2:32 a.m. UTC | #5
On Mon, May 19, 2025 at 03:55:16PM +0200, Christophe Leroy wrote:
> 
> As far as I can see related patches found in linux-next tree were not sent
> to linuxppc-dev@lists.ozlabs.org.

I just checked and it was definitely sent to linuxppc-dev:

Cc: oe-kbuild-all@lists.linux.dev, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, Venkat Rao Bagalkote <venkat88@linux.ibm.com>, Madhavan Srinivasan <maddy@linux.ibm.com>, Stephen Rothwell <sfr@canb.auug.org.au>, Danny Tsen <dtsen@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org, Michael Ellerman <mpe@ellerman.id.au>

> Could you resend them, and split out the introduction of
> CONFIG_ARCH_SUPPORTS_INT128 from other parts of patch "crypto:
> powerpc/poly1305 - Add SIMD fallback" and add the lib/tishift.S in the patch
> which adds CONFIG_ARCH_SUPPORTS_INT128 ?

I'll just revert them and mark powerpc/poly1305 as broken.  You
guys can sort it out later.

Thanks,
Herbert Xu May 20, 2025, 2:41 a.m. UTC | #6
On Wed, May 14, 2025 at 05:18:47AM +0000, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> As discussed in the thread containing
> https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
> Power10-optimized Poly1305 code is currently not safe to call in softirq
> context.  Disable it for now.  It can be re-enabled once it is fixed.
> 
> Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  arch/powerpc/lib/crypto/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
Christophe Leroy May 20, 2025, 5:17 a.m. UTC | #7
Le 20/05/2025 à 04:32, Herbert Xu a écrit :
> On Mon, May 19, 2025 at 03:55:16PM +0200, Christophe Leroy wrote:
>>
>> As far as I can see related patches found in linux-next tree were not sent
>> to linuxppc-dev@lists.ozlabs.org.
> 
> I just checked and it was definitely sent to linuxppc-dev:

Oops sorry, my mistake.

I see them in patchwork, they still have status 'New'.

> 
> Cc: oe-kbuild-all@lists.linux.dev, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, Venkat Rao Bagalkote <venkat88@linux.ibm.com>, Madhavan Srinivasan <maddy@linux.ibm.com>, Stephen Rothwell <sfr@canb.auug.org.au>, Danny Tsen <dtsen@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org, Michael Ellerman <mpe@ellerman.id.au>
> 
>> Could you resend them, and split out the introduction of
>> CONFIG_ARCH_SUPPORTS_INT128 from other parts of patch "crypto:
>> powerpc/poly1305 - Add SIMD fallback" and add the lib/tishift.S in the patch
>> which adds CONFIG_ARCH_SUPPORTS_INT128 ?
> 
> I'll just revert them and mark powerpc/poly1305 as broken.  You
> guys can sort it out later.

Fine, lets do that and make sure all necessary bits are there when 
enabling CONFIG_ARCH_SUPPORTS_INT128

Thanks
Christophe
diff mbox series

Patch

diff --git a/arch/powerpc/lib/crypto/Kconfig b/arch/powerpc/lib/crypto/Kconfig
index ffa541ad6d5da..3f9e1bbd9905b 100644
--- a/arch/powerpc/lib/crypto/Kconfig
+++ b/arch/powerpc/lib/crypto/Kconfig
@@ -8,10 +8,11 @@  config CRYPTO_CHACHA20_P10
 	select CRYPTO_ARCH_HAVE_LIB_CHACHA
 
 config CRYPTO_POLY1305_P10
 	tristate
 	depends on PPC64 && CPU_LITTLE_ENDIAN && VSX
+	depends on BROKEN # Needs to be fixed to work in softirq context
 	default CRYPTO_LIB_POLY1305
 	select CRYPTO_ARCH_HAVE_LIB_POLY1305
 	select CRYPTO_LIB_POLY1305_GENERIC
 
 config CRYPTO_SHA256_PPC_SPE