Re: [PATCH] crypto: qce - revert "use __free() for a buffer that's always freed"

[Herbert Xu]

On Wed, Dec 18, 2024 at 01:11:17PM -0700, Nathan Chancellor wrote:
> Commit ce8fd0500b74 ("crypto: qce - use __free() for a buffer that's
> always freed") introduced a buggy use of __free(), which clang
> rightfully points out:
> 
>   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,
>         |             ^
> 
> Jumping over a variable declared with the cleanup attribute does not
> prevent the cleanup function from running; instead, the cleanup function
> is called with an uninitialized value.
> 
> Moving the declaration back to the top function with __free() and a NULL
> initialization would resolve the bug but that is really not much
> different from the original code. Since the function is so simple and
> there is no functional reason to use __free() here, just revert the
> original change to resolve the issue.
> 
> Fixes: ce8fd0500b74 ("crypto: qce - use __free() for a buffer that's always freed")
> Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
> Closes: https://lore.kernel.org/CA+G9fYtpAwXa5mUQ5O7vDLK2xN4t-kJoxgUe1ZFRT=AGqmLSRA@xxxxxxxxxxxxxx/
> Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> ---
>  drivers/crypto/qce/sha.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt