Accidental config renaming in commit 3f342a23257d ("crypto: Kconfig - simplify hash entries")

Accidental config renaming in commit 3f342a23257d ("crypto: Kconfig - simplify hash entries")

[Lukas Bulwahn]

Dear Robert,

I noticed that:

Commit 3f342a23257d ("crypto: Kconfig - simplify hash entries") makes
a lot of changes to the config descriptions, but among all those
changes, it also renames CRYPTO_SHA1_ARM64_CE to CRYPTO_SHA1_ARM64.

Given that you did not touch the corresponding Makefile, it seems that
this config renaming was unintended and accidentally.

Could you please confirm that?

Best regards,

Lukas

RE: Accidental config renaming in commit 3f342a23257d ("crypto: Kconfig - simplify hash entries")

[Elliott, Robert (Servers)]

Sorry for the delay - I'm out of office this month.

1. I agree this is wrong:

-config CRYPTO_SHA1_ARM64_CE
-	tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
+config CRYPTO_SHA1_ARM64
+	tristate "Hash functions: SHA-1 (ARMv8 Crypto Extensions)"


I think my grep comparisons to make sure the config lines didn't
change were foiled by - being a special character.

Herbert, can you fix that in a way that won't
cause bisection anomalies?

2. There's one more problem in that commit - the
CRYPTO_BLAKE2S symbol was dropped by 6.0 and should no
longer exist. It's a remnant from the original series
being based on 5.19.

The extra space before "config" led me to overlook it.
(I'll investigate if checkpatch.pl can check for leading
spaces before keywords like this next month)

+ config CRYPTO_BLAKE2S
+	tristate "BLAKE2s"
+        select CRYPTO_LIB_BLAKE2S_GENERIC
+        select CRYPTO_HASH
+        help
+	  BLAKE2s cryptographic hash function (RFC 7693)
+
+	  BLAKE2s is optimized for 8 to 32-bit platforms and can produce
+	  digests of any size between 1 and 32 bytes. The keyed hash is
+	  also implemented.
+
+	  This module provides the following algorithms:
+          - blake2s-128
+          - blake2s-160
+          - blake2s-224
+          - blake2s-256
+
+	  Used by Wireguard.
+
+          See https://blake2.net for further information.



> -----Original Message-----
> From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Sent: Tuesday, September 6, 2022 9:28 PM
> To: Elliott, Robert (Servers) <elliott@hpe.com>; Herbert Xu
> <herbert@gondor.apana.org.au>; linux-crypto@vger.kernel.org; Linux Kernel
> Mailing List <linux-kernel@vger.kernel.org>
> Subject: Accidental config renaming in commit 3f342a23257d ("crypto:
> Kconfig - simplify hash entries")
> 
> Dear Robert,
> 
> I noticed that:
> 
> Commit 3f342a23257d ("crypto: Kconfig - simplify hash entries") makes
> a lot of changes to the config descriptions, but among all those
> changes, it also renames CRYPTO_SHA1_ARM64_CE to CRYPTO_SHA1_ARM64.
> 
> Given that you did not touch the corresponding Makefile, it seems that
> this config renaming was unintended and accidentally.
> 
> Could you please confirm that?
> 
> Best regards,
> 
> Lukas

Re: Accidental config renaming in commit 3f342a23257d ("crypto: Kconfig - simplify hash entries")

[Herbert Xu]

On Tue, Sep 13, 2022 at 08:30:45AM +0000, Elliott, Robert (Servers) wrote:
>
> Herbert, can you fix that in a way that won't
> cause bisection anomalies?

Sorry, it's a bit late for that.  Please send an incremental patch.

Thanks,
-- 
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

Re: Accidental config renaming in commit 3f342a23257d ("crypto: Kconfig - simplify hash entries")

[Lukas Bulwahn]

On Tue, Sep 13, 2022 at 10:30 AM Elliott, Robert (Servers)
<elliott@hpe.com> wrote:
>
> Sorry for the delay - I'm out of office this month.
>

Robert, as you are out of office, I have sent two fixup patches for
Herbert Xu to pick up:

https://lore.kernel.org/linux-crypto/20220914083626.17713-1-lukas.bulwahn@gmail.com/
https://lore.kernel.org/linux-crypto/20220914083827.18939-1-lukas.bulwahn@gmail.com/

I hope that helps.

Lukas


Lukas

> 1. I agree this is wrong:
>
> -config CRYPTO_SHA1_ARM64_CE
> -       tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
> +config CRYPTO_SHA1_ARM64
> +       tristate "Hash functions: SHA-1 (ARMv8 Crypto Extensions)"
>
>
> I think my grep comparisons to make sure the config lines didn't
> change were foiled by - being a special character.
>
> Herbert, can you fix that in a way that won't
> cause bisection anomalies?
>
> 2. There's one more problem in that commit - the
> CRYPTO_BLAKE2S symbol was dropped by 6.0 and should no
> longer exist. It's a remnant from the original series
> being based on 5.19.
>
> The extra space before "config" led me to overlook it.
> (I'll investigate if checkpatch.pl can check for leading
> spaces before keywords like this next month)
>
> + config CRYPTO_BLAKE2S
> +       tristate "BLAKE2s"
> +        select CRYPTO_LIB_BLAKE2S_GENERIC
> +        select CRYPTO_HASH
> +        help
> +         BLAKE2s cryptographic hash function (RFC 7693)
> +
> +         BLAKE2s is optimized for 8 to 32-bit platforms and can produce
> +         digests of any size between 1 and 32 bytes. The keyed hash is
> +         also implemented.
> +
> +         This module provides the following algorithms:
> +          - blake2s-128
> +          - blake2s-160
> +          - blake2s-224
> +          - blake2s-256
> +
> +         Used by Wireguard.
> +
> +          See https://blake2.net for further information.
>
>
>
> > -----Original Message-----
> > From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> > Sent: Tuesday, September 6, 2022 9:28 PM
> > To: Elliott, Robert (Servers) <elliott@hpe.com>; Herbert Xu
> > <herbert@gondor.apana.org.au>; linux-crypto@vger.kernel.org; Linux Kernel
> > Mailing List <linux-kernel@vger.kernel.org>
> > Subject: Accidental config renaming in commit 3f342a23257d ("crypto:
> > Kconfig - simplify hash entries")
> >
> > Dear Robert,
> >
> > I noticed that:
> >
> > Commit 3f342a23257d ("crypto: Kconfig - simplify hash entries") makes
> > a lot of changes to the config descriptions, but among all those
> > changes, it also renames CRYPTO_SHA1_ARM64_CE to CRYPTO_SHA1_ARM64.
> >
> > Given that you did not touch the corresponding Makefile, it seems that
> > this config renaming was unintended and accidentally.
> >
> > Could you please confirm that?
> >
> > Best regards,
> >
> > Lukas

RE: Accidental config renaming in commit 3f342a23257d ("crypto: Kconfig - simplify hash entries")

[Elliott, Robert (Servers)]

> -----Original Message-----
> From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Sent: Tuesday, September 13, 2022 10:44 PM
> To: Elliott, Robert (Servers) <elliott@hpe.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>; linux-
> crypto@vger.kernel.org; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: Re: Accidental config renaming in commit 3f342a23257d ("crypto:
> Kconfig - simplify hash entries")
> 
> On Tue, Sep 13, 2022 at 10:30 AM Elliott, Robert (Servers)
> <elliott@hpe.com> wrote:
> >
> > Sorry for the delay - I'm out of office this month.
> >
> 
> Robert, as you are out of office, I have sent two fixup patches for
> Herbert Xu to pick up:
> 
> https://lore.kernel.org/linux-crypto/20220914083626.17713-1-
> lukas.bulwahn@gmail.com/
> https://lore.kernel.org/linux-crypto/20220914083827.18939-1-
> lukas.bulwahn@gmail.com/
> 
> I hope that helps.
> 
> Lukas
> 
> 
> Lukas
 
Thanks, let's use your series. 

I was able to get online tonight and tested "make menuconfig"
under arm64 to confirm the corrections, and did some more grep
runs to ensure there are no other unintended changes lurking.