Re: [RFC PATCH 1/7] crypto: xctr - Add XCTR support

Eric Biggers
2022-01-27T19:27:53Z

On Thu, Jan 27, 2022 at 10:42:49AM +0100, Ard Biesheuvel wrote:
> > diff --git a/include/crypto/xctr.h b/include/crypto/xctr.h
> > new file mode 100644
> > index 000000000000..0d025e08ca26
> > --- /dev/null
> > +++ b/include/crypto/xctr.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * XCTR: XOR Counter mode
> > + *
> > + * Copyright 2021 Google LLC
> > + */
> > +
> > +#include <asm/unaligned.h>
> > +
> > +#ifndef _CRYPTO_XCTR_H
> > +#define _CRYPTO_XCTR_H
> > +
> > +static inline void u32_to_le_block(u8 *a, u32 x, unsigned int size)
> > +{
> > + memset(a, 0, size);
> > + put_unaligned(cpu_to_le32(x), (u32 *)a);
>
> Please use put_unaligned_le32() here.
>
> And casting 'a' to (u32 *) is invalid C, so just pass 'a' directly.
> Otherwise, the compiler might infer that 'a' is guaranteed to be
> aligned after all, and use an aligned access instead.

I agree that put_unaligned_le32() is more suitable here, but I don't think
casting 'a' to 'u32 *' is undefined; it's only dereferencing it that would be
undefined. If such casts were undefined, then get_unaligned() and
put_unaligned() would be unusable under any circumstance. Here's an example of
code that would be incorrect in that case:
https://lore.kernel.org/linux-crypto/20220119093109.1567314-1-ardb@kernel.org

- Eric