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

[Nathan Chancellor <nathan@kernel.org>]

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;