linux-kernel - Re: next-20241216: drivers/crypto/qce/sha.c:365:3: error: cannot jump from this goto statement to its label

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241216180231.GA1069997@ax162>
Date: Mon, 16 Dec 2024 11:02:31 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Naresh Kamboju <naresh.kamboju@...aro.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Cc: open list <linux-kernel@...r.kernel.org>,
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
	lkft-triage@...ts.linaro.org,
	Linux Regressions <regressions@...ts.linux.dev>,
	clang-built-linux <llvm@...ts.linux.dev>,
	thara gopinath <thara.gopinath@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Anders Roxell <anders.roxell@...aro.org>,
	Dan Carpenter <dan.carpenter@...aro.org>,
	Arnd Bergmann <arnd@...db.de>
Subject: Re: next-20241216: drivers/crypto/qce/sha.c:365:3: error: cannot
 jump from this goto statement to its label

Hi Naresh,

Thanks for the report.

+ Bartosz as author of ce8fd0500b74

On Mon, Dec 16, 2024 at 10:04:05PM +0530, Naresh Kamboju wrote:
> The arm and arm64 builds failed on Linux next-20241216 due to following
> build warnings / errors with clang-19 and clang-nightly toolchain.
> Whereas the gcc-13 builds pass.
> 
> arm, arm64:
>   * build/clang-19-defconfig
>   * build/clang-nightly-defconfig
> 
> First seen on Linux next-20241216.
>   Good: next-20241216
>   Bad:  next-20241213
> 
> Build log:
> -----------
<trimmed irrelevant warning>
> drivers/crypto/qce/sha.c:365:3: error: cannot jump from this goto
> statement to its label
>   365 |                 goto err_free_ahash;
>       |                 ^
> drivers/crypto/qce/sha.c:373:6: note: jump bypasses initialization of
> variable with __attribute__((cleanup))
>   373 |         u8 *buf __free(kfree) = kzalloc(keylen + QCE_MAX_ALIGN_SIZE,
>       |             ^
> 1 error generated.

It is a bug to jump over the initialization of a cleanup variable
because the cleanup function will be called on an uninitialized pointer
in those cases. GCC does not catch this at compile time like clang does
(it would be nice if we could document this somewhere and really
encourage people doing cleanup annotations to ensure their patches pass
a clang build except in architecture code where clang does not support
that target):

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951

It may be worth just reverting commit ce8fd0500b74 ("crypto: qce - use
__free() for a buffer that's always freed") since it seems like little
value in this case but if we want to forward fix it, I think we could
just mirror what the rest of the kernel does and keep the declaration at
the top of the function and initialize the pointer to NULL. The diff
below resolves the issue for me, which I don't mind sending as a formal
patch.

Cheers,
Nathan

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index c4ddc3b265ee..e251f0f9a4fd 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -337,6 +337,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 	struct scatterlist sg;
 	unsigned int blocksize;
 	struct crypto_ahash *ahash_tfm;
+	u8 *buf __free(kfree) = NULL;
 	int ret;
 	const char *alg_name;
 
@@ -370,8 +371,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 				   crypto_req_done, &wait);
 	crypto_ahash_clear_flags(ahash_tfm, ~0);
 
-	u8 *buf __free(kfree) = kzalloc(keylen + QCE_MAX_ALIGN_SIZE,
-					GFP_KERNEL);
+	buf = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, GFP_KERNEL);
 	if (!buf) {
 		ret = -ENOMEM;
 		goto err_free_req;