Received-SPF: pass (google.com: domain of linux-crypto+bounces-1225-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 4 Jan 2024 10:03:06 +0800
From: Herbert Xu <herbert@gondor.apana.org.au>
To: David Howells <dhowells@redhat.com>
Cc: Shigeru Yoshida <syoshida@redhat.com>, davem@davemloft.net,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] crypto: af_alg/hash: Fix uninit-value access in
 af_alg_free_sg()
Message-ID: <ZZYR2qcc2Fmaxqq0@gondor.apana.org.au>
References: <ZYUFs1MumRFf3mnv@gondor.apana.org.au>
 <20231211135949.689204-1-syoshida@redhat.com>
 <386306.1704296211@warthog.procyon.org.uk>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <386306.1704296211@warthog.procyon.org.uk>

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