RE: [PATCH 4/6] crypto/openssl: fix 3DES-CTR with big endian CPUs

["Morten Brørup" <mb@smartsharesystems.com>]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Thursday, 24 October 2024 14.06
> 
> Caught by code review.
> 
> Don't byte swap unconditionally (assuming that CPU is little endian is
> wrong). Instead, convert from big endian to cpu and vice versa.

Yes looks like a bug.
I wonder if this PMD has more similar bugs...
grep bswap drivers/crypto/openssl/* says no.

> @@ -110,9 +111,9 @@ ctr_inc(uint8_t *ctr)
>  {
>  	uint64_t *ctr64 = (uint64_t *)ctr;
> 
> -	*ctr64 = __builtin_bswap64(*ctr64);
> +	*ctr64 = rte_be_to_cpu_64(*ctr64);
>  	(*ctr64)++;
> -	*ctr64 = __builtin_bswap64(*ctr64);
> +	*ctr64 = rte_cpu_to_be_64(*ctr64);
>  }

But that's not all.

There may be an alignment bug too; the way it is used in process_openssl_cipher_des3ctr(), "ctr" is not guaranteed to be uint64_t aligned.

How about this instead:

ctr_inc(void *ctr)
{
	uint64_t ctr64 = rte_be_to_cpu_64(*(unaligned_uint64_t *)ctr);
	ctr64++;
	*(unaligned_uint64_t *)ctr = rte_cpu_to_be_64(ctr64);
}

Or this:

ctr_inc(void *ctr)
{
	uint64_t ctr64;

	memcpy(&ctr64, ctr, sizeof(uint64_t));
	ctr64 = rte_be_to_cpu_64(ctr64);
	ctr64++;
	ctr64 = rte_cpu_to_be_64(ctr64);
	memcpy(ctr, &ctr64, sizeof(uint64_t));
}

Or use a union in process_openssl_cipher_des3ctr() to ensure it's uint64_t aligned.