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

Message-ID: <CACMJSevssgFn6omKpV0rAMQKXpH__mmmkpdjUSYB7s2U=dsAEw@mail.gmail.com>
Date: Mon, 16 Dec 2024 19:45:03 +0100
From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
To: Nathan Chancellor <nathan@...nel.org>, Herbert Xu <herbert@...dor.apana.org.au>
Cc: Naresh Kamboju <naresh.kamboju@...aro.org>, 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

On Mon, 16 Dec 2024 at 19:02, Nathan Chancellor <nathan@...nel.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;