Messages in this thread | | | Subject | Re: [PATCH] Trusted and Encrypted Keys: fix up TSS_rawhmac() so we always kfree() and remember to call va_end() | From | David Safford <> | Date | Fri, 14 Jan 2011 08:28:02 -0500 |
| |
On Thu, 2011-01-13 at 21:07 +0100, Jesper Juhl wrote: > In security/keys/trusted_defined.c::TSS_rawhmac() we may leak the storage > allocated to 'sdesc' if > data = va_arg(argp, unsigned char *); > results in a NULL 'data' and we then leave the function by returning > -EINVAL. We also neglect calling va_end(argp) in that case and furthermore > we neglect va_end(argp) if > ret = crypto_shash_update(&sdesc->shash, data, dlen); > results in ret being negative and we then jump to the 'out' label. > > I believe this patch takes care of these issues. Please review and > consider for inclusion. > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
thanks for catching this.
Acked-by: David Safford <safford@watson.ibm.com>
> --- > trusted_defined.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > compile tested only. > > diff --git a/security/keys/trusted_defined.c b/security/keys/trusted_defined.c > index 975e9f2..0ec7ab8 100644 > --- a/security/keys/trusted_defined.c > +++ b/security/keys/trusted_defined.c > @@ -101,16 +101,18 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key, > if (dlen == 0) > break; > data = va_arg(argp, unsigned char *); > - if (data == NULL) > - return -EINVAL; > + if (data == NULL) { > + ret = -EINVAL; > + goto out; > + } > ret = crypto_shash_update(&sdesc->shash, data, dlen); > if (ret < 0) > goto out; > } > - va_end(argp); > if (!ret) > ret = crypto_shash_final(&sdesc->shash, digest); > out: > + va_end(argp); > kfree(sdesc); > return ret; > } > > >
| |