Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material

[David Howells]

Eric Biggers  wrote:

> It's not actually needed because it's impossible for the compiler to optimize
> away the memset().  memzero_explicit() is only needed on stack data.

Okay, also reasonable.

David

Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material

[David Howells]

Eric Biggers  wrote:

> It's not actually needed because it's impossible for the compiler to optimize
> away the memset().  memzero_explicit() is only needed on stack data.

Okay, also reasonable.

David

Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material

[Eric Biggers]

On Fri, Apr 21, 2017 at 03:31:08PM +0100, David Howells wrote:
> Eric Biggers  wrote:
> 
> > -   memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
> > -   kfree(key->payload.data[0]);
> > +   kzfree(key->payload.data[0]);
> 
> Should kzfree() be using memzero_explicit() rather than memset()?
> 
> David

It's not actually needed because it's impossible for the compiler to optimize
away the memset().  memzero_explicit() is only needed on stack data.

The reason I still used memzero_explicit() for heap data in a couple of my
patches, even though it's unnecessary, is just that it makes it clearer that
it's being done for sanitization purposes, as opposed to some random memset.

That's not as much of an issue for kzfree(), since it's explicitly for
sanitization purposes already.

As a separate note, something that might make sense at some point would be to
skip the memset in kzfree() if slab poisoning is enabled.

Eric

Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material

[Eric Biggers]

On Fri, Apr 21, 2017 at 03:31:08PM +0100, David Howells wrote:
> Eric Biggers  wrote:
> 
> > -   memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
> > -   kfree(key->payload.data[0]);
> > +   kzfree(key->payload.data[0]);
> 
> Should kzfree() be using memzero_explicit() rather than memset()?
> 
> David

It's not actually needed because it's impossible for the compiler to optimize
away the memset().  memzero_explicit() is only needed on stack data.

The reason I still used memzero_explicit() for heap data in a couple of my
patches, even though it's unnecessary, is just that it makes it clearer that
it's being done for sanitization purposes, as opposed to some random memset.

That's not as much of an issue for kzfree(), since it's explicitly for
sanitization purposes already.

As a separate note, something that might make sense at some point would be to
skip the memset in kzfree() if slab poisoning is enabled.

Eric

Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material

[David Howells]

Eric Biggers  wrote:

> - memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
> - kfree(key->payload.data[0]);
> + kzfree(key->payload.data[0]);

Should kzfree() be using memzero_explicit() rather than memset()?

David

Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material

[David Howells]

Eric Biggers  wrote:

> - memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
> - kfree(key->payload.data[0]);
> + kzfree(key->payload.data[0]);

Should kzfree() be using memzero_explicit() rather than memset()?

David