Skip to contentSkip to conversation

crypto: catch base cipher self-test failures in fips mode

PATCH Superseded Jarod Wilson 3 participants 9 messages Apr 29, 2009 – May 4, 2009

Subsystems (2)
Crypto API Arm STM32
lore
Share & tools
lore

Jarod Wilson

author
#0 lore raw

PATCH 1/1 crypto: catch base cipher self-test failures in fips mode

I think this might have already been posted by Neil Horman, and we already have it in the Red Hat Enterprise Linux 5.x kernels, but in fips mode, we need to panic on the base cipher self-tests failing as well as the later tests.

Signed-off-by Jarod Wilson <[email protected]>
---
 crypto/testmgr.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
crypto/testmgr.c +7 -3
index 40c1078..5a50416 100644
@@ -2109,7 +2109,7 @@ static int alg_find_test(const char *alg)
 int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 {
 	int i;
-	int rc;
+	int rc = 0;
 
 	if ((type & CRYPTO_ALG_TYPE_MASK) == CRYPTO_ALG_TYPE_CIPHER) {
 		char nalg[CRYPTO_MAX_ALG_NAME];

@@ -2122,7 +2122,8 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		if (i < 0)
 			goto notest;
 
-		return alg_test_cipher(alg_test_descs + i, driver, type, mask);
+		rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
+		goto test_done;
 	}
 
 	i = alg_find_test(alg);

@@ -2131,14 +2132,13 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 
 	rc = alg_test_descs[i].test(alg_test_descs + i, driver,
 				      type, mask);
+	goto test_done;
+notest:
+	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);
+test_done:
 	if (fips_enabled && rc)
 		panic("%s: %s alg self test failed in fips mode!\n", driver, alg);
-
 	return rc;
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines · Jarod Wilson

I think this might have already been posted by Neil Horman, and we already have it in the Red Hat Enterprise Linux 5.x kernels, but in fips mode, we need to panic on the base cipher self-tests failing as well as the later tests.

Signed-off-by Jarod Wilson <[email protected]>

I did post it:
http://www.mail-archive.com/[email protected]/msg02307.html
looks like it somehow just never made it to Linus. Thanks for noticing, Jarod.

Acked-by Neil Horman <[email protected]>
Quoted 1 line
author
#2 lore raw
Quoted 11 lines · On Wednesday 29 April 2009 06:36:23 Neil Horman wrote:
Quoted 7 lines · Jarod Wilson

I think this might have already been posted by Neil Horman, and we already have it in the Red Hat Enterprise Linux 5.x kernels, but in fips mode, we need to panic on the base cipher self-tests failing as well as the later tests.

Signed-off-by Jarod Wilson <[email protected]>

I did post it:
http://www.mail-archive.com/[email protected]/msg02307.html
looks like it somehow just never made it to Linus. Thanks for noticing, Jarod.

That part got committed, this is an additional piece, as I believe that wasn't quite complete. This patch adds another check for the rc of alg_test_cipher() (vs. only the check for alg_test_descs[i].test()).

Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#3 lore raw
In reply to Jarod Wilson

crypto/testmgr.c int alg_test(const char *driver, const char *alg, u32 type, u32 mask) · line 2137

Quoted 3 lines · Jarod Wilson
crypto/testmgr.c +2 -0
+notest:
+	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);

Can notest ever get here with rc != 0?

Cheers,

Signature · 4 lines
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
author
#4 lore raw
Quoted 6 lines · On Wednesday 29 April 2009 09:15:07 Herbert Xu wrote:

crypto/testmgr.c int alg_test(const char *driver, const char *alg, u32 type, u32 mask) · line 2137

Quoted 3 lines · Jarod Wilson
crypto/testmgr.c +2 -0
+notest:
+	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);

Can notest ever get here with rc != 0?

Nope.

Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#5 lore raw
Quoted 9 lines · Jarod Wilson
Quoted 6 lines · On Wednesday 29 April 2009 09:15:07 Herbert Xu wrote:

crypto/testmgr.c int alg_test(const char *driver, const char *alg, u32 type, u32 mask) · line 2137

Quoted 3 lines · Jarod Wilson
crypto/testmgr.c +2 -0
+notest:
+	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);

Can notest ever get here with rc != 0?

Nope.

So why do we need to move it?

Cheers,

Signature · 4 lines
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
author
#6 lore raw
Quoted 4 lines · On Wednesday 29 April 2009 09:26:46 Herbert Xu wrote:
Quoted 3 lines · Jarod Wilson
Quoted 2 lines · On Wednesday 29 April 2009 09:15:07 Herbert Xu wrote:
Quoted 1 line · Jarod Wilson
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

crypto/testmgr.c int alg_test(const char *driver, const char *alg, u32 type, u32 mask) · line 2137

Quoted 2 lines
crypto/testmgr.c +2 -0
+notest:
+	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);

Can notest ever get here with rc != 0?

Nope.

So why do we need to move it?

Oh. Hrm. Upon looking a bit harder at it, I think the only reason would be if we wanted to print out a message claiming success in testing ctr(aes).

There's also a devious ulterior motive for this patch, which is to make the cryptodev tree look identical(er) to the Red Hat Enterprise Linux 5.x kernel tree w/in alg_test()...

Signature · 2 lines
Jarod Wilson
[email protected]
author
#7 lore raw

PATCH v2 1/1 crypto: catch base cipher self-test failures in fips mode

Quoted 14 lines · On Wednesday 29 April 2009 09:34:31 Jarod Wilson wrote:
Quoted 4 lines · On Wednesday 29 April 2009 09:26:46 Herbert Xu wrote:
Quoted 3 lines · Jarod Wilson
Quoted 2 lines · On Wednesday 29 April 2009 09:15:07 Herbert Xu wrote:
Quoted 1 line · Jarod Wilson
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+notest:
+	printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);

Can notest ever get here with rc != 0?

Nope.

So why do we need to move it?

Quoted 7 lines

Oh. Hrm. Upon looking a bit harder at it, I think the only reason would be if we wanted to print out a message claiming success in testing ctr(aes).

There's also a devious ulterior motive for this patch, which is to make the cryptodev tree look identical(er) to the Red Hat Enterprise Linux 5.x kernel tree w/in alg_test()...

Version 2, streamlined and simplified, RHEL5 be damned.

Signed-off-by Jarod Wilson <[email protected]>
---
 crypto/testmgr.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
crypto/testmgr.c +3 -1
index 0161cc2..f39c148 100644
@@ -2220,7 +2220,8 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		if (i < 0)
 			goto notest;
 
-		return alg_test_cipher(alg_test_descs + i, driver, type, mask);
+		rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
+		goto test_done;
 	}
 
 	i = alg_find_test(alg);

@@ -2229,6 +2230,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 
 	rc = alg_test_descs[i].test(alg_test_descs + i, driver,
 				      type, mask);
+test_done:
 	if (fips_enabled && rc)
 		panic("%s: %s alg self test failed in fips mode!\n", driver, alg);
Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#8 lore raw
Quoted 4 lines · Jarod Wilson

Version 2, streamlined and simplified, RHEL5 be damned.

Signed-off-by Jarod Wilson <[email protected]>

Patch applied.

Signature · 4 lines
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