Re: crypto: run initcalls for generic implementations earlier

Re: crypto: run initcalls for generic implementations earlier

[Geert Uytterhoeven]

Hi Eric,

On Tue, May 7, 2019 at 5:26 AM Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Commit:     c4741b23059794bd99beef0f700103b0d983b3fd
> Parent:     40153b10d91c9e25f912344ba6ce1f0874400659
> Refname:    refs/heads/master
> Web:        https://git.kernel.org/torvalds/c/c4741b23059794bd99beef0f700103b0d983b3fd
> Author:     Eric Biggers <ebiggers@google.com>
> AuthorDate: Thu Apr 11 21:57:42 2019 -0700
> Committer:  Herbert Xu <herbert@gondor.apana.org.au>
> CommitDate: Thu Apr 18 22:15:03 2019 +0800
>
>     crypto: run initcalls for generic implementations earlier
>
>     Use subsys_initcall for registration of all templates and generic
>     algorithm implementations, rather than module_init.  Then change
>     cryptomgr to use arch_initcall, to place it before the subsys_initcalls.
>
>     This is needed so that when both a generic and optimized implementation
>     of an algorithm are built into the kernel (not loadable modules), the
>     generic implementation is registered before the optimized one.
>     Otherwise, the self-tests for the optimized implementation are unable to
>     allocate the generic implementation for the new comparison fuzz tests.
>
>     Note that on arm, a side effect of this change is that self-tests for
>     generic implementations may run before the unaligned access handler has
>     been installed.  So, unaligned accesses will crash the kernel.  This is
>     arguably a good thing as it makes it easier to detect that type of bug.
>
>     Signed-off-by: Eric Biggers <ebiggers@google.com>
>     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
>         crypto_unregister_rng(&jent_alg);
>  }
>
> -module_init(jent_mod_init);
> +subsys_initcall(jent_mod_init);
>  module_exit(jent_mod_exit);
>
>  MODULE_LICENSE("Dual BSD/GPL");

This change causes jitterentropy to fail on Renesas SoCs based on
single-core Cortex A9 with:

    jitterentropy: Initialization failed with host not compliant with
requirements: 2

This happens because jitterentropy is now initialized before the main
clocksource is activated, i.e. before

    clocksource: Switched to clocksource ostm timer (on RZ/A1)
    clocksource: Switched to clocksource fff80000.timer (on R-Mobile A1)

is printed.
RZ/A1 and R-Mobile A1 SoCs rely on the OSTM resp. TMU timers.

The issue does not happen on SoCs with Cortex A15 cores (with ARM
architectured timer) or Cortex A9 multicore (with ARM global timer).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: crypto: run initcalls for generic implementations earlier

[Geert Uytterhoeven]

Hi Eric,

On Tue, May 7, 2019 at 5:26 AM Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Commit:     c4741b23059794bd99beef0f700103b0d983b3fd
> Parent:     40153b10d91c9e25f912344ba6ce1f0874400659
> Refname:    refs/heads/master
> Web:        https://git.kernel.org/torvalds/c/c4741b23059794bd99beef0f700103b0d983b3fd
> Author:     Eric Biggers <ebiggers@google.com>
> AuthorDate: Thu Apr 11 21:57:42 2019 -0700
> Committer:  Herbert Xu <herbert@gondor.apana.org.au>
> CommitDate: Thu Apr 18 22:15:03 2019 +0800
>
>     crypto: run initcalls for generic implementations earlier
>
>     Use subsys_initcall for registration of all templates and generic
>     algorithm implementations, rather than module_init.  Then change
>     cryptomgr to use arch_initcall, to place it before the subsys_initcalls.
>
>     This is needed so that when both a generic and optimized implementation
>     of an algorithm are built into the kernel (not loadable modules), the
>     generic implementation is registered before the optimized one.
>     Otherwise, the self-tests for the optimized implementation are unable to
>     allocate the generic implementation for the new comparison fuzz tests.
>
>     Note that on arm, a side effect of this change is that self-tests for
>     generic implementations may run before the unaligned access handler has
>     been installed.  So, unaligned accesses will crash the kernel.  This is
>     arguably a good thing as it makes it easier to detect that type of bug.
>
>     Signed-off-by: Eric Biggers <ebiggers@google.com>
>     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
>         crypto_unregister_rng(&jent_alg);
>  }
>
> -module_init(jent_mod_init);
> +subsys_initcall(jent_mod_init);
>  module_exit(jent_mod_exit);
>
>  MODULE_LICENSE("Dual BSD/GPL");

This change causes jitterentropy to fail on Renesas SoCs based on
single-core Cortex A9 with:

    jitterentropy: Initialization failed with host not compliant with
requirements: 2

This happens because jitterentropy is now initialized before the main
clocksource is activated, i.e. before

    clocksource: Switched to clocksource ostm timer (on RZ/A1)
    clocksource: Switched to clocksource fff80000.timer (on R-Mobile A1)

is printed.
RZ/A1 and R-Mobile A1 SoCs rely on the OSTM resp. TMU timers.

The issue does not happen on SoCs with Cortex A15 cores (with ARM
architectured timer) or Cortex A9 multicore (with ARM global timer).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: crypto: run initcalls for generic implementations earlier

[Eric Biggers]

On Tue, May 21, 2019 at 06:39:00PM +0200, Geert Uytterhoeven wrote:
> Hi Eric,
> 
> On Tue, May 7, 2019 at 5:26 AM Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org> wrote:
> > Commit:     c4741b23059794bd99beef0f700103b0d983b3fd
> > Parent:     40153b10d91c9e25f912344ba6ce1f0874400659
> > Refname:    refs/heads/master
> > Web:        https://git.kernel.org/torvalds/c/c4741b23059794bd99beef0f700103b0d983b3fd
> > Author:     Eric Biggers <ebiggers@google.com>
> > AuthorDate: Thu Apr 11 21:57:42 2019 -0700
> > Committer:  Herbert Xu <herbert@gondor.apana.org.au>
> > CommitDate: Thu Apr 18 22:15:03 2019 +0800
> >
> >     crypto: run initcalls for generic implementations earlier
> >
> >     Use subsys_initcall for registration of all templates and generic
> >     algorithm implementations, rather than module_init.  Then change
> >     cryptomgr to use arch_initcall, to place it before the subsys_initcalls.
> >
> >     This is needed so that when both a generic and optimized implementation
> >     of an algorithm are built into the kernel (not loadable modules), the
> >     generic implementation is registered before the optimized one.
> >     Otherwise, the self-tests for the optimized implementation are unable to
> >     allocate the generic implementation for the new comparison fuzz tests.
> >
> >     Note that on arm, a side effect of this change is that self-tests for
> >     generic implementations may run before the unaligned access handler has
> >     been installed.  So, unaligned accesses will crash the kernel.  This is
> >     arguably a good thing as it makes it easier to detect that type of bug.
> >
> >     Signed-off-by: Eric Biggers <ebiggers@google.com>
> >     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> > --- a/crypto/jitterentropy-kcapi.c
> > +++ b/crypto/jitterentropy-kcapi.c
> > @@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
> >         crypto_unregister_rng(&jent_alg);
> >  }
> >
> > -module_init(jent_mod_init);
> > +subsys_initcall(jent_mod_init);
> >  module_exit(jent_mod_exit);
> >
> >  MODULE_LICENSE("Dual BSD/GPL");
> 
> This change causes jitterentropy to fail on Renesas SoCs based on
> single-core Cortex A9 with:
> 
>     jitterentropy: Initialization failed with host not compliant with
> requirements: 2
> 
> This happens because jitterentropy is now initialized before the main
> clocksource is activated, i.e. before
> 
>     clocksource: Switched to clocksource ostm timer (on RZ/A1)
>     clocksource: Switched to clocksource fff80000.timer (on R-Mobile A1)
> 
> is printed.
> RZ/A1 and R-Mobile A1 SoCs rely on the OSTM resp. TMU timers.
> 
> The issue does not happen on SoCs with Cortex A15 cores (with ARM
> architectured timer) or Cortex A9 multicore (with ARM global timer).
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Thanks for the bug report.  It seems there was no point for my patch to change
jitterentropy_rng, since it's not a generic crypto algorithm that has multiple
implementations, nor is it testable by the crypto self-tests.  So I'll send a
patch that changes it back to module_init().

- Eric

Re: crypto: run initcalls for generic implementations earlier

[Eric Biggers]

On Tue, May 21, 2019 at 06:39:00PM +0200, Geert Uytterhoeven wrote:
> Hi Eric,
> 
> On Tue, May 7, 2019 at 5:26 AM Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org> wrote:
> > Commit:     c4741b23059794bd99beef0f700103b0d983b3fd
> > Parent:     40153b10d91c9e25f912344ba6ce1f0874400659
> > Refname:    refs/heads/master
> > Web:        https://git.kernel.org/torvalds/c/c4741b23059794bd99beef0f700103b0d983b3fd
> > Author:     Eric Biggers <ebiggers@google.com>
> > AuthorDate: Thu Apr 11 21:57:42 2019 -0700
> > Committer:  Herbert Xu <herbert@gondor.apana.org.au>
> > CommitDate: Thu Apr 18 22:15:03 2019 +0800
> >
> >     crypto: run initcalls for generic implementations earlier
> >
> >     Use subsys_initcall for registration of all templates and generic
> >     algorithm implementations, rather than module_init.  Then change
> >     cryptomgr to use arch_initcall, to place it before the subsys_initcalls.
> >
> >     This is needed so that when both a generic and optimized implementation
> >     of an algorithm are built into the kernel (not loadable modules), the
> >     generic implementation is registered before the optimized one.
> >     Otherwise, the self-tests for the optimized implementation are unable to
> >     allocate the generic implementation for the new comparison fuzz tests.
> >
> >     Note that on arm, a side effect of this change is that self-tests for
> >     generic implementations may run before the unaligned access handler has
> >     been installed.  So, unaligned accesses will crash the kernel.  This is
> >     arguably a good thing as it makes it easier to detect that type of bug.
> >
> >     Signed-off-by: Eric Biggers <ebiggers@google.com>
> >     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> > --- a/crypto/jitterentropy-kcapi.c
> > +++ b/crypto/jitterentropy-kcapi.c
> > @@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
> >         crypto_unregister_rng(&jent_alg);
> >  }
> >
> > -module_init(jent_mod_init);
> > +subsys_initcall(jent_mod_init);
> >  module_exit(jent_mod_exit);
> >
> >  MODULE_LICENSE("Dual BSD/GPL");
> 
> This change causes jitterentropy to fail on Renesas SoCs based on
> single-core Cortex A9 with:
> 
>     jitterentropy: Initialization failed with host not compliant with
> requirements: 2
> 
> This happens because jitterentropy is now initialized before the main
> clocksource is activated, i.e. before
> 
>     clocksource: Switched to clocksource ostm timer (on RZ/A1)
>     clocksource: Switched to clocksource fff80000.timer (on R-Mobile A1)
> 
> is printed.
> RZ/A1 and R-Mobile A1 SoCs rely on the OSTM resp. TMU timers.
> 
> The issue does not happen on SoCs with Cortex A15 cores (with ARM
> architectured timer) or Cortex A9 multicore (with ARM global timer).
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Thanks for the bug report.  It seems there was no point for my patch to change
jitterentropy_rng, since it's not a generic crypto algorithm that has multiple
implementations, nor is it testable by the crypto self-tests.  So I'll send a
patch that changes it back to module_init().

- Eric

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] crypto: jitterentropy - change back to module_init()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

"jitterentropy_rng" doesn't have any other implementations, nor is it
tested by the crypto self-tests.  So it was unnecessary to change it to
subsys_initcall.  Also it depends on the main clocksource being
initialized, which may happen after subsys_initcall, causing this error:

    jitterentropy: Initialization failed with host not compliant with requirements: 2

Change it back to module_init().

Fixes: c4741b230597 ("crypto: run initcalls for generic implementations earlier")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/jitterentropy-kcapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 6ea1a270b8dc2..787dccca37159 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
 	crypto_unregister_rng(&jent_alg);
 }
 
-subsys_initcall(jent_mod_init);
+module_init(jent_mod_init);
 module_exit(jent_mod_exit);
 
 MODULE_LICENSE("Dual BSD/GPL");
-- 
2.21.0.1020.gf2820cf01a-goog

[PATCH] crypto: jitterentropy - change back to module_init()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

"jitterentropy_rng" doesn't have any other implementations, nor is it
tested by the crypto self-tests.  So it was unnecessary to change it to
subsys_initcall.  Also it depends on the main clocksource being
initialized, which may happen after subsys_initcall, causing this error:

    jitterentropy: Initialization failed with host not compliant with requirements: 2

Change it back to module_init().

Fixes: c4741b230597 ("crypto: run initcalls for generic implementations earlier")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/jitterentropy-kcapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 6ea1a270b8dc2..787dccca37159 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -198,7 +198,7 @@ static void __exit jent_mod_exit(void)
 	crypto_unregister_rng(&jent_alg);
 }
 
-subsys_initcall(jent_mod_init);
+module_init(jent_mod_init);
 module_exit(jent_mod_exit);
 
 MODULE_LICENSE("Dual BSD/GPL");
-- 
2.21.0.1020.gf2820cf01a-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] crypto: jitterentropy - change back to module_init()

[Geert Uytterhoeven]

On Tue, May 21, 2019 at 8:46 PM Eric Biggers <ebiggers@kernel.org> wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> "jitterentropy_rng" doesn't have any other implementations, nor is it
> tested by the crypto self-tests.  So it was unnecessary to change it to
> subsys_initcall.  Also it depends on the main clocksource being
> initialized, which may happen after subsys_initcall, causing this error:
>
>     jitterentropy: Initialization failed with host not compliant with requirements: 2
>
> Change it back to module_init().
>
> Fixes: c4741b230597 ("crypto: run initcalls for generic implementations earlier")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] crypto: jitterentropy - change back to module_init()

[Geert Uytterhoeven]

On Tue, May 21, 2019 at 8:46 PM Eric Biggers <ebiggers@kernel.org> wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> "jitterentropy_rng" doesn't have any other implementations, nor is it
> tested by the crypto self-tests.  So it was unnecessary to change it to
> subsys_initcall.  Also it depends on the main clocksource being
> initialized, which may happen after subsys_initcall, causing this error:
>
>     jitterentropy: Initialization failed with host not compliant with requirements: 2
>
> Change it back to module_init().
>
> Fixes: c4741b230597 ("crypto: run initcalls for generic implementations earlier")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] crypto: jitterentropy - change back to module_init()

[Herbert Xu]

On Tue, May 21, 2019 at 11:46:22AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> "jitterentropy_rng" doesn't have any other implementations, nor is it
> tested by the crypto self-tests.  So it was unnecessary to change it to
> subsys_initcall.  Also it depends on the main clocksource being
> initialized, which may happen after subsys_initcall, causing this error:
> 
>     jitterentropy: Initialization failed with host not compliant with requirements: 2
> 
> Change it back to module_init().
> 
> Fixes: c4741b230597 ("crypto: run initcalls for generic implementations earlier")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  crypto/jitterentropy-kcapi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: jitterentropy - change back to module_init()

[Herbert Xu]

On Tue, May 21, 2019 at 11:46:22AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> "jitterentropy_rng" doesn't have any other implementations, nor is it
> tested by the crypto self-tests.  So it was unnecessary to change it to
> subsys_initcall.  Also it depends on the main clocksource being
> initialized, which may happen after subsys_initcall, causing this error:
> 
>     jitterentropy: Initialization failed with host not compliant with requirements: 2
> 
> Change it back to module_init().
> 
> Fixes: c4741b230597 ("crypto: run initcalls for generic implementations earlier")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  crypto/jitterentropy-kcapi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel