On Thu, 27 Aug 2020 at 11:20, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Thu, Aug 27, 2020 at 10:42 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >
> > In that case, I suppose we should simply disable instrumentation for
> > chacha_permute()? It is a straight-forward arithmetic transformation
> > on a u32[16] array, where ubsan has limited value afaict.
>
> I guess that always works as a last resort, but shouldn't we first try
> to figure out why ubsan even makes a difference and whether the
> object code without ubsan looks like a reasonable representation
> of the source form?
>
> Since it really is a fairly simple transformation, I would have
> expected the compiler to not emit any ubsan checks. If gcc
> only gets confused about the fixed offsets possibly overflowing
> the fixed-length array, maybe it helps to give it a little extra
> information like (untested):
>
> --- a/lib/crypto/chacha.c
> +++ b/lib/crypto/chacha.c
> @@ -13,7 +13,7 @@
> #include <asm/unaligned.h>
> #include <crypto/chacha.h>
>
> -static void chacha_permute(u32 *x, int nrounds)
> +static void chacha_permute(u32 x[16], int nrounds)
> {
> int i;
>
That does not help, unfortunately.
What does seem to work is
struct chacha_state { u32 x[16]; };
struct chacha_state chacha_permute(struct chacha_state st, int nrounds)
{
struct chacha_state ret = st;
u32 *x = ret.x;
...
return st;
}
(and updating the caller accordingly, obviously)
