linux-kernel - [PATCH] crypto: sha512: remove imaginary and mystifying clearing of variables

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210822103107.28974-1-lukas.bulwahn@gmail.com>
Date:   Sun, 22 Aug 2021 12:31:07 +0200
From:   Lukas Bulwahn <lukas.bulwahn@...il.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        linux-crypto@...r.kernel.org
Cc:     Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        clang-built-linux@...glegroups.com,
        kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: [PATCH] crypto: sha512: remove imaginary and mystifying clearing of variables

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@...il.com>
---
 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