2014-11-30 17:05:28

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 0/8] replace memset by memzero_explicit

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
local idexpression e;
type T,T1;
@@

{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)&x[0]
)
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when != e
when strict
}

@@
identifier i,x;
local idexpression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
e = (T)&x
... when any
when exists
- memset
+ memzero_explicit
(&x,
-0,
...)
... when != x
when != e
when strict
}

// ------------------------------------------------------------------------

@@
identifier x;
type T,T1;
expression e;
@@

{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)&x[0]
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}

@@
identifier i,x;
expression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
when != e = (T)&x
- memset
+ memzero_explicit
(&x,
-0,
...)
... when != x
when strict
}
// </smpl>


2014-11-30 17:05:38

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 4/8] cifs: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

fs/cifs/smbencrypt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/smbencrypt.c b/fs/cifs/smbencrypt.c
index 6c15663..6512c19 100644
--- a/fs/cifs/smbencrypt.c
+++ b/fs/cifs/smbencrypt.c
@@ -221,7 +221,7 @@ E_md4hash(const unsigned char *passwd, unsigned char *p16,
}

rc = mdfour(p16, (unsigned char *) wpwd, len * sizeof(__le16));
- memset(wpwd, 0, 129 * sizeof(__le16));
+ memzero_explicit(wpwd, 129 * sizeof(__le16));

return rc;
}

2014-11-30 17:05:33

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 1/8] purgatory/sha256: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

arch/x86/purgatory/sha256.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/purgatory/sha256.c b/arch/x86/purgatory/sha256.c
index 548ca67..8271ca5 100644
--- a/arch/x86/purgatory/sha256.c
+++ b/arch/x86/purgatory/sha256.c
@@ -205,7 +205,7 @@ static void sha256_transform(u32 *state, const u8 *input)

/* clear any sensitive info... */
a = b = c = d = e = f = g = h = t1 = t2 = 0;
- memset(W, 0, 64 * sizeof(u32));
+ memzero_explicit(W, 64 * sizeof(u32));
}

int sha256_init(struct sha256_state *sctx)

2014-11-30 17:05:32

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 7/8] crypto: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

I was not able to compile test this one.

arch/arm/crypto/sha512_neon_glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/crypto/sha512_neon_glue.c b/arch/arm/crypto/sha512_neon_glue.c
index f3452c6..b124dce 100644
--- a/arch/arm/crypto/sha512_neon_glue.c
+++ b/arch/arm/crypto/sha512_neon_glue.c
@@ -241,7 +241,7 @@ static int sha384_neon_final(struct shash_desc *desc, u8 *hash)
sha512_neon_final(desc, D);

memcpy(hash, D, SHA384_DIGEST_SIZE);
- memset(D, 0, SHA512_DIGEST_SIZE);
+ memzero_explicit(D, SHA512_DIGEST_SIZE);

return 0;
}

2014-11-30 17:05:31

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 2/8] sparc64: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

I was not able to compile test these.

arch/sparc/crypto/sha256_glue.c | 2 +-
arch/sparc/crypto/sha512_glue.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/crypto/sha512_glue.c b/arch/sparc/crypto/sha512_glue.c
index 9fff885..11eb36c 100644
--- a/arch/sparc/crypto/sha512_glue.c
+++ b/arch/sparc/crypto/sha512_glue.c
@@ -139,7 +139,7 @@ static int sha384_sparc64_final(struct shash_desc *desc, u8 *hash)
sha512_sparc64_final(desc, D);

memcpy(hash, D, 48);
- memset(D, 0, 64);
+ memzero_explicit(D, 64);

return 0;
}
diff --git a/arch/sparc/crypto/sha256_glue.c b/arch/sparc/crypto/sha256_glue.c
index 41f27cc..285268c 100644
--- a/arch/sparc/crypto/sha256_glue.c
+++ b/arch/sparc/crypto/sha256_glue.c
@@ -135,7 +135,7 @@ static int sha224_sparc64_final(struct shash_desc *desc, u8 *hash)
sha256_sparc64_final(desc, D);

memcpy(hash, D, SHA224_DIGEST_SIZE);
- memset(D, 0, SHA256_DIGEST_SIZE);
+ memzero_explicit(D, SHA256_DIGEST_SIZE);

return 0;
}

2014-11-30 17:06:38

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 6/8] crypto: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

arch/powerpc/crypto/sha1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index 0f88c7b..d3feba5 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -66,7 +66,7 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
src = data + done;
} while (done + 63 < len);

- memset(temp, 0, sizeof(temp));
+ memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx->buffer + partial, src, len - done);

2014-11-30 17:06:56

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 5/8] dm: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

drivers/md/dm-crypt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index fc93b93..08981be 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -705,7 +705,7 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
for (i = 0; i < ((1 << SECTOR_SHIFT) / 8); i++)
crypto_xor(data + i * 8, buf, 8);
out:
- memset(buf, 0, sizeof(buf));
+ memzero_explicit(buf, sizeof(buf));
return r;
}

2014-11-30 17:06:55

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 8/8] wusb: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

drivers/usb/wusbcore/dev-sysfs.c | 2 +-
drivers/usb/wusbcore/security.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c
index 1018345..415b140 100644
--- a/drivers/usb/wusbcore/dev-sysfs.c
+++ b/drivers/usb/wusbcore/dev-sysfs.c
@@ -101,7 +101,7 @@ static ssize_t wusb_ck_store(struct device *dev,
if (wusbhc == NULL)
return -ENODEV;
result = wusb_dev_4way_handshake(wusbhc, usb_dev->wusb_dev, &ck);
- memset(&ck, 0, sizeof(ck));
+ memzero_explicit(&ck, sizeof(ck));
wusbhc_put(wusbhc);
return result < 0 ? result : size;
}
diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index cc74d66..b66faaf 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -522,10 +522,10 @@ error_hs3:
error_hs2:
error_hs1:
memset(hs, 0, 3*sizeof(hs[0]));
- memset(&keydvt_out, 0, sizeof(keydvt_out));
- memset(&keydvt_in, 0, sizeof(keydvt_in));
- memset(&ccm_n, 0, sizeof(ccm_n));
- memset(mic, 0, sizeof(mic));
+ memzero_explicit(&keydvt_out, sizeof(keydvt_out));
+ memzero_explicit(&keydvt_in, sizeof(keydvt_in));
+ memzero_explicit(&ccm_n, sizeof(ccm_n));
+ memzero_explicit(mic, sizeof(mic));
if (result < 0)
wusb_dev_set_encryption(usb_dev, 0);
error_dev_set_encryption:

2014-11-30 17:07:52

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 3/8] crypto: replace memset by memzero_explicit

From: Julia Lawall <[email protected]>

Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
// </smpl>

This change was suggested by Daniel Borkmann <[email protected]>

Signed-off-by: Julia Lawall <[email protected]>

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

arch/x86/crypto/sha256_ssse3_glue.c | 2 +-
arch/x86/crypto/sha512_ssse3_glue.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/crypto/sha512_ssse3_glue.c b/arch/x86/crypto/sha512_ssse3_glue.c
index 26a5898..0b6af26 100644
--- a/arch/x86/crypto/sha512_ssse3_glue.c
+++ b/arch/x86/crypto/sha512_ssse3_glue.c
@@ -219,7 +219,7 @@ static int sha384_ssse3_final(struct shash_desc *desc, u8 *hash)
sha512_ssse3_final(desc, D);

memcpy(hash, D, SHA384_DIGEST_SIZE);
- memset(D, 0, SHA512_DIGEST_SIZE);
+ memzero_explicit(D, SHA512_DIGEST_SIZE);

return 0;
}
diff --git a/arch/x86/crypto/sha256_ssse3_glue.c b/arch/x86/crypto/sha256_ssse3_glue.c
index 4dc100d..8fad72f 100644
--- a/arch/x86/crypto/sha256_ssse3_glue.c
+++ b/arch/x86/crypto/sha256_ssse3_glue.c
@@ -211,7 +211,7 @@ static int sha224_ssse3_final(struct shash_desc *desc, u8 *hash)
sha256_ssse3_final(desc, D);

memcpy(hash, D, SHA224_DIGEST_SIZE);
- memset(D, 0, SHA256_DIGEST_SIZE);
+ memzero_explicit(D, SHA256_DIGEST_SIZE);

return 0;
}

2014-12-01 15:20:51

by David Sterba

[permalink] [raw]
Subject: Re: [PATCH 1/8] purgatory/sha256: replace memset by memzero_explicit

On Sun, Nov 30, 2014 at 05:59:27PM +0100, Julia Lawall wrote:
> @@ -205,7 +205,7 @@ static void sha256_transform(u32 *state, const u8 *input)
>
> /* clear any sensitive info... */
> a = b = c = d = e = f = g = h = t1 = t2 = 0;

All of these get optimized away.

How to clear them effectively is another question. Setting them one by
one casting to volatile would generate 10 instructions. Although this
might be quick as there are no dependencies, the function
sha256_transform is called repeatedly and the small overhead may add up.

Other attempts to fix it require restructuring the code so all the
variables are eg. stored in an array that gets cleared in one go. Here
the indirect access could be hidden into the macros (e0, e1, s0, s1).

> - memset(W, 0, 64 * sizeof(u32));
> + memzero_explicit(W, 64 * sizeof(u32));

2014-12-03 00:14:32

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 8/8] wusb: replace memset by memzero_explicit

On Sun, Nov 30, 2014 at 05:59:34PM +0100, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Memset on a local variable may be removed when it is called just before the
> variable goes out of scope. Using memzero_explicit defeats this
> optimization. A simplified version of the semantic patch that makes this
> change is as follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier x;
> type T;
> @@
>
> {
> ... when any
> T x[...];
> ... when any
> when exists
> - memset
> + memzero_explicit
> (x,
> -0,
> ...)
> ... when != x
> when strict
> }
> // </smpl>
>
> This change was suggested by Daniel Borkmann <[email protected]>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> Daniel Borkmann suggested that these patches could go through Herbert Xu's
> cryptodev tree.

Why? There's no dependancy on anything in the cryptodev tree,
memzero_explicit is in Linus's tree now.

thanks,

greg k-h

2014-12-03 00:19:46

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH 8/8] wusb: replace memset by memzero_explicit

On 12/03/2014 01:14 AM, Greg Kroah-Hartman wrote:
> On Sun, Nov 30, 2014 at 05:59:34PM +0100, Julia Lawall wrote:
>> From: Julia Lawall <[email protected]>
>>
>> Memset on a local variable may be removed when it is called just before the
>> variable goes out of scope. Using memzero_explicit defeats this
>> optimization. A simplified version of the semantic patch that makes this
>> change is as follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @@
>> identifier x;
>> type T;
>> @@
>>
>> {
>> ... when any
>> T x[...];
>> ... when any
>> when exists
>> - memset
>> + memzero_explicit
>> (x,
>> -0,
>> ...)
>> ... when != x
>> when strict
>> }
>> // </smpl>
>>
>> This change was suggested by Daniel Borkmann <[email protected]>
>>
>> Signed-off-by: Julia Lawall <[email protected]>
>>
>> ---
>> Daniel Borkmann suggested that these patches could go through Herbert Xu's
>> cryptodev tree.
>
> Why? There's no dependancy on anything in the cryptodev tree,
> memzero_explicit is in Linus's tree now.

Sorry, I guess this was not really clear, that comment actually only
refers to the arch/*/crypto/ patches.

Anyway, thanks for queueing this up, Greg.

Cheers,
Daniel