next-20241216: drivers/crypto/qce/sha.c:365:3: error: cannot jump from this goto statement to its label
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: ----------- fs/netfs/read_retry.c:235:20: warning: variable 'subreq' is uninitialized when used here [-Wuninitialized] 235 | if (list_is_last(&subreq->rreq_link, &stream->subrequests)) | ^~~~~~ fs/netfs/read_retry.c:28:36: note: initialize the variable 'subreq' to silence this warning 28 | struct netfs_io_subrequest *subreq; | ^ | = NULL 1 warning generated. 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.
Links: ------- - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20241216/tes... - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20241216/tes... - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20241216/tes... - https://storage.tuxsuite.com/public/linaro/lkft/builds/2qHsa6j5c1HY3GRZFGrxu...
Steps to reproduce: ------------ # tuxmake --runtime podman --target-arch arm64 --toolchain clang-19 --kconfig defconfig LLVM=1 LLVM_IAS=1
metadata: ---- git describe: next-20241216 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git git sha: e25c8d66f6786300b680866c0e0139981273feba kernel config: https://storage.tuxsuite.com/public/linaro/lkft/builds/2qHsa8wFmfrlj0VdDqAG4... build url: https://storage.tuxsuite.com/public/linaro/lkft/builds/2qHsa8wFmfrlj0VdDqAG4... toolchain: clang-19 config: clang-19-defconfig arch: arm64, arm
-- Linaro LKFT https://lkft.linaro.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;
On Mon, 16 Dec 2024 at 19:02, Nathan Chancellor nathan@kernel.org wrote:
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
I'm fine with dropping that commit from next.
Bartosz
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;
-
Bartosz Golaszewski -
Naresh Kamboju -
Nathan Chancellor