From: Herbert Xu on
On Fri, Aug 06, 2010 at 02:01:03AM +0100, David Howells wrote:
>
> but it's not being found by the crypto routines. By GIT bisection, I note
> that this problem is introduced in the following commit:
>
> commit 0b767f96164b2b27488e3daa722ff16e89d49314
> Author: Alexander Shishkin <virtuoso(a)slind.org>
> Date: Thu Jun 3 20:53:43 2010 +1000
>
> crypto: testmgr - add an option to disable cryptoalgos' self-tests

Indeed this changeset is buggy. It makes cryptomgr return a value
as if it was not loaded. This triggers the module load.

I'll send you a patch.

Thanks,
--
Email: Herbert Xu <herbert(a)gondor.apana.org.au>
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 majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Herbert Xu on
On Fri, Aug 06, 2010 at 09:17:06AM +0800, Herbert Xu wrote:
> On Fri, Aug 06, 2010 at 02:01:03AM +0100, David Howells wrote:
> >
> > but it's not being found by the crypto routines. By GIT bisection, I note
> > that this problem is introduced in the following commit:
> >
> > commit 0b767f96164b2b27488e3daa722ff16e89d49314
> > Author: Alexander Shishkin <virtuoso(a)slind.org>
> > Date: Thu Jun 3 20:53:43 2010 +1000
> >
> > crypto: testmgr - add an option to disable cryptoalgos' self-tests
>
> Indeed this changeset is buggy. It makes cryptomgr return a value
> as if it was not loaded. This triggers the module load.
>
> I'll send you a patch.

This patch should do the trick:

commit 326a6346ffb5b19eb593530d9d3096d409e46f62
Author: Herbert Xu <herbert(a)gondor.apana.org.au>
Date: Fri Aug 6 09:40:28 2010 +0800

crypto: testmgr - Fix test disabling option

This patch fixes a serious bug in the test disabling patch where
it can cause an spurious load of the cryptomgr module even when
it's compiled in.

It also negates the test disabling option so that its absence
causes tests to be enabled.

The Kconfig option is also now behind EMBEDDED.

Signed-off-by: Herbert Xu <herbert(a)gondor.apana.org.au>

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1cd497d..6f5c50f 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -101,13 +101,12 @@ config CRYPTO_MANAGER2
select CRYPTO_BLKCIPHER2
select CRYPTO_PCOMP2

-config CRYPTO_MANAGER_TESTS
- bool "Run algolithms' self-tests"
- default y
- depends on CRYPTO_MANAGER2
+config CRYPTO_MANAGER_DISABLE_TESTS
+ bool "Disable run-time self tests"
+ depends on CRYPTO_MANAGER2 && EMBEDDED
help
- Run cryptomanager's tests for the new crypto algorithms being
- registered.
+ Disable run-time self tests that normally take place at
+ algorithm registration.

config CRYPTO_GF128MUL
tristate "GF(2^128) multiplication functions (EXPERIMENTAL)"
diff --git a/crypto/algboss.c b/crypto/algboss.c
index 40bd391..791d194 100644
--- a/crypto/algboss.c
+++ b/crypto/algboss.c
@@ -206,13 +206,16 @@ err:
return NOTIFY_OK;
}

-#ifdef CONFIG_CRYPTO_MANAGER_TESTS
static int cryptomgr_test(void *data)
{
struct crypto_test_param *param = data;
u32 type = param->type;
int err = 0;

+#ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS
+ goto skiptest;
+#endif
+
if (type & CRYPTO_ALG_TESTED)
goto skiptest;

@@ -267,7 +270,6 @@ err_put_module:
err:
return NOTIFY_OK;
}
-#endif /* CONFIG_CRYPTO_MANAGER_TESTS */

static int cryptomgr_notify(struct notifier_block *this, unsigned long msg,
void *data)
@@ -275,10 +277,8 @@ static int cryptomgr_notify(struct notifier_block *this, unsigned long msg,
switch (msg) {
case CRYPTO_MSG_ALG_REQUEST:
return cryptomgr_schedule_probe(data);
-#ifdef CONFIG_CRYPTO_MANAGER_TESTS
case CRYPTO_MSG_ALG_REGISTER:
return cryptomgr_schedule_test(data);
-#endif
}

return NOTIFY_DONE;
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index abd980c..fa8c8f7 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -23,7 +23,7 @@

#include "internal.h"

-#ifndef CONFIG_CRYPTO_MANAGER_TESTS
+#ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS

/* a perfect nop */
int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
@@ -2542,6 +2542,6 @@ non_fips_alg:
return -EINVAL;
}

-#endif /* CONFIG_CRYPTO_MANAGER_TESTS */
+#endif /* CONFIG_CRYPTO_MANAGER_DISABLE_TESTS */

EXPORT_SYMBOL_GPL(alg_test);

Thanks,
--
Email: Herbert Xu <herbert(a)gondor.apana.org.au>
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 majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Herbert Xu on
On Thu, Aug 05, 2010 at 07:01:21PM -0700, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 6:40 PM, Herbert Xu <herbert(a)gondor.hengli.com.au> wrote:
> >
> > -config CRYPTO_MANAGER_TESTS
> > - � � � bool "Run algolithms' self-tests"
> > - � � � default y
> > - � � � depends on CRYPTO_MANAGER2
> > +config CRYPTO_MANAGER_DISABLE_TESTS
> > + � � � bool "Disable run-time self tests"
> > + � � � depends on CRYPTO_MANAGER2 && EMBEDDED
>
> Why do you still want to force-enable those tests? I was going to
> complain about the "default y" anyway, now I'm _really_ complaining,
> because you've now made it impossible to disable those tests. Why?

Because it can save data. Each cryptographic algorithm (such as
AES) may have multiple impelmentations, some of which are hardware-
based.

The purpose of these tests are to make a particular driver or
implementation available only if it passes them. So your encrypted
disk/file system will not be subject to a hardware/software combo
without at least some semblance of testing.

The last thing you want to is to upgrade your kernel with a new
hardware crypto driver that detects that you have a piece of rarely-
used crypto hardeware, decides to use it and ends up making your
data toast.

But whatever, if you want the default to be no tests, that's fine.
Here's the patch to do just that.

commit 00ca28a507b215dcd121735f16764ea4173c4ff9
Author: Herbert Xu <herbert(a)gondor.apana.org.au>
Date: Fri Aug 6 10:34:00 2010 +0800

crypto: testmgr - Default to no tests

On Thu, Aug 05, 2010 at 07:01:21PM -0700, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 6:40 PM, Herbert Xu <herbert(a)gondor.hengli.com.au> wrote:
> >
> > -config CRYPTO_MANAGER_TESTS
> > - bool "Run algolithms' self-tests"
> > - default y
> > - depends on CRYPTO_MANAGER2
> > +config CRYPTO_MANAGER_DISABLE_TESTS
> > + bool "Disable run-time self tests"
> > + depends on CRYPTO_MANAGER2 && EMBEDDED
>
> Why do you still want to force-enable those tests? I was going to
> complain about the "default y" anyway, now I'm _really_ complaining,
> because you've now made it impossible to disable those tests. Why?

As requested, this patch sets the default to y and removes the
EMBEDDED dependency.

Signed-off-by: Herbert Xu <herbert(a)gondor.apana.org.au>

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 6f5c50f..e573077 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -103,7 +103,8 @@ config CRYPTO_MANAGER2

config CRYPTO_MANAGER_DISABLE_TESTS
bool "Disable run-time self tests"
- depends on CRYPTO_MANAGER2 && EMBEDDED
+ default y
+ depends on CRYPTO_MANAGER2
help
Disable run-time self tests that normally take place at
algorithm registration.

Cheers,
--
Email: Herbert Xu <herbert(a)gondor.apana.org.au>
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 majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Herbert Xu on
Olivier Galibert <galibert(a)pobox.com> wrote:
>
> Of course in practice without the tests your boot would probably just
> have failed. Badly-decrypted root partitions tend to be noticed as
> such long before trying to write to them. Then you would have bitched
> on the list and the driver would have been fixed or removed faster
> than having to wait for you (or other people with the hardware issue)
> to notice the spew in dmesg.

So you'd rather have a box that doesn't boot rather than one
that automatically falls back to software crypto allowing you
to diagnose and report the problem.

Yes that makes a lot of sense.

Thanks,
--
Email: Herbert Xu <herbert(a)gondor.apana.org.au>
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 majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/