On Sun, Aug 22, 2021 at 3:31 AM Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx> wrote:
>
> The function sha512_transform() assigns all local variables to 0 before
> returning to its caller with the intent to erase sensitive data.
>
> However, make clang-analyzer warns that all these assignments are dead
> stores, and as commit 7a4295f6c9d5 ("crypto: lib/sha256 - Don't clear
> temporary variables") already points out for sha256_transform():
>
> The assignments to clear a through h and t1/t2 are optimized out by the
> compiler because they are unused after the assignments.
>
> Clearing individual scalar variables is unlikely to be useful, as they
> may have been assigned to registers, and even if stack spilling was
> required, there may be compiler-generated temporaries that are
> impossible to clear in any case.
>
> This applies here again as well. Drop meaningless clearing of local
> variables and avoid this way that the code suggests that data is erased,
> which simply does not happen.
>
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> ---
> crypto/sha512_generic.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/crypto/sha512_generic.c b/crypto/sha512_generic.c
> index c72d72ad828e..be70e76d6d86 100644
> --- a/crypto/sha512_generic.c
> +++ b/crypto/sha512_generic.c
> @@ -143,9 +143,6 @@ sha512_transform(u64 *state, const u8 *input)
>
> state[0] += a; state[1] += b; state[2] += c; state[3] += d;
> state[4] += e; state[5] += f; state[6] += g; state[7] += h;
> -
> - /* erase our data */
> - a = b = c = d = e = f = g = h = t1 = t2 = 0;
> }
>
> static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src,
> --
> 2.26.2
>
--
Thanks,
~Nick Desaulniers
