linux-kernel - Re: [PATCH] crypto: af_alg/hash: Fix uninit-value access in af_alg_free

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]

Date: Thu, 4 Jan 2024 10:03:06 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: David Howells <dhowells@...hat.com>
Cc: Shigeru Yoshida <syoshida@...hat.com>, davem@...emloft.net,
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: af_alg/hash: Fix uninit-value access in
 af_alg_free_sg()

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@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt