Skip to contentSkip to conversation

s390 crypto: explicitly memzero stack key material in aes_s390.c

PATCH Torsten Duwe 2 participants 2 messages Feb 21, 2020 – Feb 25, 2020

Subsystems (3)
Crypto API s390 s390 crypto modules, prng driver, arch random
lore
Share & tools
lore

Torsten Duwe

author
#0 lore raw

From: Torsten Duwe <[email protected]>

s390 crypto: explicitly memzero stack key material in aes_s390.c

aes_s390.c has several functions which allocate space for key material on the stack and leave the used keys there. It is considered good practice to clean these locations before the function returns.

Signed-off-by Torsten Duwe <[email protected]>

---
This popped up during our FIPS certification.
It's obviously a good idea not to leave key material on the stack.

arch/s390/crypto/aes_s390.c +3 -0
@@ -392,6 +392,7 @@ static int cbc_aes_crypt(struct blkciphe
 		ret = blkcipher_walk_done(desc, walk, nbytes - n);
 	}
 	memcpy(walk->iv, param.iv, AES_BLOCK_SIZE);
+	memzero_explicit(&param, sizeof(param));
 	return ret;
 }
 

@@ -576,6 +577,8 @@ static int xts_aes_crypt(struct blkciphe
 			 walk->dst.virt.addr, walk->src.virt.addr, n);
 		ret = blkcipher_walk_done(desc, walk, nbytes - n);
 	}
+	memzero_explicit(&pcc_param, sizeof(pcc_param));
+	memzero_explicit(&xts_param, sizeof(xts_param));
 	return ret;
 }
maintainer
#1 lore raw
Quoted 25 lines · Torsten Duwe

From: Torsten Duwe <[email protected]>

s390 crypto: explicitly memzero stack key material in aes_s390.c

aes_s390.c has several functions which allocate space for key material on the stack and leave the used keys there. It is considered good practice to clean these locations before the function returns.

Signed-off-by Torsten Duwe <[email protected]>

---
This popped up during our FIPS certification.
It's obviously a good idea not to leave key material on the stack.

arch/s390/crypto/aes_s390.c +1 -0
@@ -392,6 +392,7 @@ static int cbc_aes_crypt(struct blkciphe
 		ret = blkcipher_walk_done(desc, walk, nbytes - n);
 	}
 	memcpy(walk->iv, param.iv, AES_BLOCK_SIZE);
+	memzero_explicit(&param, sizeof(param));
 	return ret;
 }

arch/s390/crypto/aes_s390.c static int xts_aes_crypt(struct blkciphe · line 584

Quoted 8 lines
arch/s390/crypto/aes_s390.c +2 -0
@@ -576,6 +577,8 @@ static int xts_aes_crypt(struct blkciphe
 			 walk->dst.virt.addr, walk->src.virt.addr, n);
 		ret = blkcipher_walk_done(desc, walk, nbytes - n);
 	}
+	memzero_explicit(&pcc_param, sizeof(pcc_param));
+	memzero_explicit(&xts_param, sizeof(xts_param));
 	return ret;
 }

Thanks Torsten, I've picked this patch. It will go upstream via the s390 subsystem with the next kernel merge window. regards Harald Freudenberger