2008-11-14 20:57:43

by Neil Horman

Subject: [PATCH] ansi_cprng: zero out key data on exit

Its been mentioned to me a few times that the ansi_cprng doesn't zero out its
data when a context is freed. Given that we store key, and other seed data in
that structure, I think its probably a good idea that we zero it out. This
patch does that.

Regards
Neil

Signed-off-by: Neil Horman <[email protected]>


ansi_cprng.c | 1 +
1 file changed, 1 insertion(+)


diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 72db0fd..e6ad373 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -266,6 +266,7 @@ done:
static void free_prng_context(struct prng_context *ctx)
{
crypto_free_cipher(ctx->tfm);
+ memset(ctx, 0, sizeof(struct prng_context));
}

static int reset_prng_context(struct prng_context *ctx,
--
/****************************************************
* Neil Horman <[email protected]>
* Software Engineer, Red Hat
****************************************************/

2008-11-24 13:22:13

by Herbert Xu

Subject: Re: [PATCH] ansi_cprng: zero out key data on exit

On Fri, Nov 14, 2008 at 03:55:36PM -0500, Neil Horman wrote:
>
> diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
> index 72db0fd..e6ad373 100644
> --- a/crypto/ansi_cprng.c
> +++ b/crypto/ansi_cprng.c
> @@ -266,6 +266,7 @@ done:
> static void free_prng_context(struct prng_context *ctx)
> {
> crypto_free_cipher(ctx->tfm);
> + memset(ctx, 0, sizeof(struct prng_context));
> }

crypto_free_tfm should already handle this, no?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-24 16:26:58

by Neil Horman

Subject: Re: [PATCH] ansi_cprng: zero out key data on exit

On Mon, Nov 24, 2008 at 09:22:07PM +0800, Herbert Xu wrote:
> On Fri, Nov 14, 2008 at 03:55:36PM -0500, Neil Horman wrote:
> >
> > diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
> > index 72db0fd..e6ad373 100644
> > --- a/crypto/ansi_cprng.c
> > +++ b/crypto/ansi_cprng.c
> > @@ -266,6 +266,7 @@ done:
> > static void free_prng_context(struct prng_context *ctx)
> > {
> > crypto_free_cipher(ctx->tfm);
> > + memset(ctx, 0, sizeof(struct prng_context));
> > }
>
> crypto_free_tfm should already handle this, no?
>
Yep, my bad, you're right. I was thinking since it was a pointer, we'd just
null the pointer rather than the data, but it didn't occur to me that both were
allocated contiguously, so we're good. Patch rescinded, sorry for the noise.
Neil

> Cheers,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

--
/****************************************************
* Neil Horman <[email protected]>
* Software Engineer, Red Hat
****************************************************/