Re: pkgtools/digest sha256 aliasing bug with gcc 11

["David H. Gutteridge" <david%gutteridge.ca@localhost>]

On Tue, 8 Feb 2022 at 00:51:39 -0800, Michael Forney wrote:
> Hi all,
>
> I recently rebuilt my pkgtools/digest package for the blake2s update,
> and noticed that sha256 sums are now incorrect. It turns out that
> this is because I had updated gcc to 11.2 in the mean time, which
> appears to be stricter about aliasing violations and is completely
> eliminating the code that stores the bit length into the buffer
> (see https://godbolt.org/z/3doTrbse9 for a minimal reproducer).
>
> I'm not sure if this is a regression in gcc or an intended code
> transformation, but regardless, I think we should make it use memcpy
> instead to avoid this issue. There are still a number of questionable
> casts in this file, but the others don't seem to cause a problem
> in practice.
>
> Does this patch look good to apply?

I can't comment on the compiler internals aspect of this, but it seems
reasonable to me, and I can confirm it fixes the issue for me, which I
reproduced and tested the patch with GCC 11.2.1 on Fedora 35.

> -- >8 --
> Subject: [PATCH] digest: fix aliasing bug with gcc 11
>
> gcc 11 with -O2 optimizes away the store of the bit length into the
> last 8 bytes of the context buffer due to an aliasing violation
> (stored through uint64_t, retrieved through uint32_t).
>
> To fix this, import the NetBSD patch from christos[0] which makes
> it use memcpy instead.
>
> [0] 
> http://cvsweb.netbsd.org/bsdweb.cgi/src/common/lib/libc/hash/sha2/sha2.c.diff?r1=1.12&r2=1.13
> ---
> pkgtools/digest/files/configure | 4 ++--
> pkgtools/digest/files/sha2.c    | 3 ++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/pkgtools/digest/files/configure 
> b/pkgtools/digest/files/configure
> index f833c50c7067..a10ad8606bd3 100755
> --- a/pkgtools/digest/files/configure
> +++ b/pkgtools/digest/files/configure
> @@ -580,8 +580,8 @@ MAKEFLAGS=
> # Identity of this package.
> PACKAGE_NAME='nbsd-digest'
> PACKAGE_TARNAME='nbsd-digest'
> -PACKAGE_VERSION='20211023'
> -PACKAGE_STRING='nbsd-digest 20190127'
> +PACKAGE_VERSION='20220208'
> +PACKAGE_STRING='nbsd-digest 20220208'
> PACKAGE_BUGREPORT='agc%netbsd.org@localhost'
> PACKAGE_URL=''
>
> diff --git a/pkgtools/digest/files/sha2.c 
> b/pkgtools/digest/files/sha2.c
> index 972a24fbbd36..f2cc98487191 100644
> --- a/pkgtools/digest/files/sha2.c
> +++ b/pkgtools/digest/files/sha2.c
> @@ -548,7 +548,8 @@ void SHA256_Final(sha2_byte digest[], SHA256_CTX* 
> context) {
> 			*context->buffer = 0x80;
> 		}
> 		/* Set the bit count: */
> -		*(sha2_word64*)&context->buffer[SHA256_SHORT_BLOCK_LENGTH] = 
> context->bitcount;
> +		MEMCPY_BCOPY(&context->buffer[SHA256_SHORT_BLOCK_LENGTH],
> +		    &context->bitcount, sizeof(context->bitcount));
>
> 		/* Final transform: */
> 		SHA256_Transform(context, (sha2_word32*)context->buffer);
> --
> 2.34.1