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

Ard Biesheuvel
2022-01-27T19:45:25Z

On Thu, 27 Jan 2022 at 20:26, Eric Biggers <ebiggers@kernel.org> wrote:
>
> 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
>

Good point :-)