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

[Morten Brørup]

> From: David Marchand [mailto:david.march...@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.