Re: [PATCH] crypto: af_alg/hash: Fix uninit-value access in af_alg_free_sg()

From: Herbert Xu
Date: Wed Jan 03 2024 - 21:03:23 EST


On Wed, Jan 03, 2024 at 03:36:51PM +0000, David Howells wrote:
> Hmmm... Is that going to get you a potential memory leak?
>
> ctx->sgl.sgt.sgl could (in theory) point to an allocated table. I guess that
> would be cleaned up by af_alg_free_areq_sgls(), so there's probably no leak
> there.

The SG list is only setup in this function, and gets freed before
we return. There should be no SG list on entry. It's only because
you added the special case for a zero-length hash that we hit the
bogus free. So we should fix this by not freeing the SG list in
the zero-length case, as it was never allocated.

> OTOH, af_alg_free_areq_sgls() is going to call af_alg_free_sg(), so maybe we
> want to initialise sgl->sgt.sgl to NULL as well.

That has nothing to do with this. This SG list is specific to
algif_hash and has nothing to do with the shared SG list used
by aead and skcipher.

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