diff mbox series

crypto: jitterentropy - bind statically into kernel

Message ID 5692716.lOV4Wx5bFT@positron.chronox.de (mailing list archive)
State New, archived
Headers show
Series crypto: jitterentropy - bind statically into kernel | expand

Commit Message

Stephan Mueller Oct. 4, 2020, 6:48 p.m. UTC
The RISC-V architecture is about to implement the callback
random_get_entropy with a function that is not exported to modules.
Thus, the Jitter RNG is changed to be only bound statically into the
kernel removing the option to compile it as module.

Reported-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ard Biesheuvel Oct. 4, 2020, 9:16 p.m. UTC | #1
On Sun, 4 Oct 2020 at 20:48, Stephan Müller <smueller@chronox.de> wrote:
>
> The RISC-V architecture is about to implement the callback
> random_get_entropy with a function that is not exported to modules.

Why is that? Wouldn't it be better to export the symbol instead?

> Thus, the Jitter RNG is changed to be only bound statically into the
> kernel removing the option to compile it as module.
>
> Reported-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>  crypto/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 094ef56ab7b4..5b20087b117f 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1853,7 +1853,7 @@ config CRYPTO_DRBG
>  endif  # if CRYPTO_DRBG_MENU
>
>  config CRYPTO_JITTERENTROPY
> -       tristate "Jitterentropy Non-Deterministic Random Number Generator"
> +       bool "Jitterentropy Non-Deterministic Random Number Generator"
>         select CRYPTO_RNG
>         help
>           The Jitterentropy RNG is a noise that is intended
> --
> 2.26.2
>
>
>
>
Palmer Dabbelt Oct. 4, 2020, 10:05 p.m. UTC | #2
On Sun, 04 Oct 2020 14:16:10 PDT (-0700), ardb@kernel.org wrote:
> On Sun, 4 Oct 2020 at 20:48, Stephan Müller <smueller@chronox.de> wrote:
>>
>> The RISC-V architecture is about to implement the callback
>> random_get_entropy with a function that is not exported to modules.
>
> Why is that? Wouldn't it be better to export the symbol instead?

It's static inline (in our timex.h), so I thought we didn't need to export the
symbol?  Did this just arise because clint_time_val wasn't exported?  That was
fixed before the random_get_entropy() change landed in Linus' tree, so as far
as I know we should be OK here.

If I broke something here it seem better to fix this in the RISC-V port than by
just banning modular compilation of jitterentropy, as that seems like a useful
feature to me.

>> Thus, the Jitter RNG is changed to be only bound statically into the
>> kernel removing the option to compile it as module.
>>
>> Reported-by: Christoph Hellwig <hch@infradead.org>
>> Signed-off-by: Stephan Mueller <smueller@chronox.de>
>> ---
>>  crypto/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/crypto/Kconfig b/crypto/Kconfig
>> index 094ef56ab7b4..5b20087b117f 100644
>> --- a/crypto/Kconfig
>> +++ b/crypto/Kconfig
>> @@ -1853,7 +1853,7 @@ config CRYPTO_DRBG
>>  endif  # if CRYPTO_DRBG_MENU
>>
>>  config CRYPTO_JITTERENTROPY
>> -       tristate "Jitterentropy Non-Deterministic Random Number Generator"
>> +       bool "Jitterentropy Non-Deterministic Random Number Generator"
>>         select CRYPTO_RNG
>>         help
>>           The Jitterentropy RNG is a noise that is intended
>> --
>> 2.26.2
>>
>>
>>
>>
'Christoph Hellwig' Oct. 5, 2020, 6:19 a.m. UTC | #3
On Sun, Oct 04, 2020 at 11:16:10PM +0200, Ard Biesheuvel wrote:
> On Sun, 4 Oct 2020 at 20:48, Stephan M??ller <smueller@chronox.de> wrote:
> >
> > The RISC-V architecture is about to implement the callback
> > random_get_entropy with a function that is not exported to modules.
> 
> Why is that? Wouldn't it be better to export the symbol instead?

get_cycles is a low-level time keeping detail that really should not
be exported, and at least for RISC-V this would be the only modular
user.  Once that is sorted out I'll audit other common architectures
to drop the export, as it isn't something that should be used in ramdom
driver code.
Ard Biesheuvel Oct. 5, 2020, 6:24 a.m. UTC | #4
On Mon, 5 Oct 2020 at 08:19, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Sun, Oct 04, 2020 at 11:16:10PM +0200, Ard Biesheuvel wrote:
> > On Sun, 4 Oct 2020 at 20:48, Stephan M??ller <smueller@chronox.de> wrote:
> > >
> > > The RISC-V architecture is about to implement the callback
> > > random_get_entropy with a function that is not exported to modules.
> >
> > Why is that? Wouldn't it be better to export the symbol instead?
>
> get_cycles is a low-level time keeping detail that really should not
> be exported, and at least for RISC-V this would be the only modular
> user.  Once that is sorted out I'll audit other common architectures
> to drop the export, as it isn't something that should be used in ramdom
> driver code.

Fair enough.

But this means we should fix the jitterentropy driver rather than
sidestepping the issue by only allowing it to be built in a way where
we don't happen to notice that the symbol in question is not meant for
general consumption.

If jitterentropy is a special case, we could put a alternate
non-'static inline' version of random_get_entropy() in the core
kernel, and only export it if JITTER_ENTROPY is built as a module in
the first place. But I'd prefer it if jitterentropy switches to an API
that is suitable for driver consumption.
Stephan Mueller Oct. 5, 2020, 6:40 a.m. UTC | #5
Am Montag, 5. Oktober 2020, 08:24:46 CEST schrieb Ard Biesheuvel:

Hi Ard,

> If jitterentropy is a special case, we could put a alternate
> non-'static inline' version of random_get_entropy() in the core
> kernel, and only export it if JITTER_ENTROPY is built as a module in
> the first place. But I'd prefer it if jitterentropy switches to an API
> that is suitable for driver consumption.

Which API do you have in mind? In user space, I use 
clock_gettime(CLOCK_REALTIME) which also considers the clock source.

Thanks
Stephan
'Christoph Hellwig' Oct. 5, 2020, 6:44 a.m. UTC | #6
[adding Thomas]

On Mon, Oct 05, 2020 at 08:40:25AM +0200, Stephan Mueller wrote:
> > If jitterentropy is a special case, we could put a alternate
> > non-'static inline' version of random_get_entropy() in the core
> > kernel, and only export it if JITTER_ENTROPY is built as a module in
> > the first place. But I'd prefer it if jitterentropy switches to an API
> > that is suitable for driver consumption.
> 
> Which API do you have in mind? In user space, I use 
> clock_gettime(CLOCK_REALTIME) which also considers the clock source.

We could probably add a kernel_clock_gettime which contains the
clock_gettime syscal implementation minus the put_timespec64.  Thomas,
is this something that fits your timekeeping vision?
Ard Biesheuvel Oct. 5, 2020, 6:44 a.m. UTC | #7
On Mon, 5 Oct 2020 at 08:40, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Montag, 5. Oktober 2020, 08:24:46 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > If jitterentropy is a special case, we could put a alternate
> > non-'static inline' version of random_get_entropy() in the core
> > kernel, and only export it if JITTER_ENTROPY is built as a module in
> > the first place. But I'd prefer it if jitterentropy switches to an API
> > that is suitable for driver consumption.
>
> Which API do you have in mind? In user space, I use
> clock_gettime(CLOCK_REALTIME) which also considers the clock source.
>

AFAICT, that call is backed by ktime_get_real_ts64(), which is already
being exported to modules.

Could you please check whether that works for your driver?
'Christoph Hellwig' Oct. 5, 2020, 6:56 a.m. UTC | #8
On Mon, Oct 05, 2020 at 08:44:39AM +0200, Ard Biesheuvel wrote:
> On Mon, 5 Oct 2020 at 08:40, Stephan Mueller <smueller@chronox.de> wrote:
> >
> > Am Montag, 5. Oktober 2020, 08:24:46 CEST schrieb Ard Biesheuvel:
> >
> > Hi Ard,
> >
> > > If jitterentropy is a special case, we could put a alternate
> > > non-'static inline' version of random_get_entropy() in the core
> > > kernel, and only export it if JITTER_ENTROPY is built as a module in
> > > the first place. But I'd prefer it if jitterentropy switches to an API
> > > that is suitable for driver consumption.
> >
> > Which API do you have in mind? In user space, I use
> > clock_gettime(CLOCK_REALTIME) which also considers the clock source.
> >
> 
> AFAICT, that call is backed by ktime_get_real_ts64(), which is already
> being exported to modules.

Indeed. No need for my earlier idea..
Stephan Mueller Oct. 5, 2020, 7:45 a.m. UTC | #9
Am Montag, 5. Oktober 2020, 08:44:39 CEST schrieb Ard Biesheuvel:

Hi Ard,

> On Mon, 5 Oct 2020 at 08:40, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Montag, 5. Oktober 2020, 08:24:46 CEST schrieb Ard Biesheuvel:
> > 
> > Hi Ard,
> > 
> > > If jitterentropy is a special case, we could put a alternate
> > > non-'static inline' version of random_get_entropy() in the core
> > > kernel, and only export it if JITTER_ENTROPY is built as a module in
> > > the first place. But I'd prefer it if jitterentropy switches to an API
> > > that is suitable for driver consumption.
> > 
> > Which API do you have in mind? In user space, I use
> > clock_gettime(CLOCK_REALTIME) which also considers the clock source.
> 
> AFAICT, that call is backed by ktime_get_real_ts64(), which is already
> being exported to modules.
> 
> Could you please check whether that works for your driver?

Yes, will do. Thanks.

Ciao
Stephan
diff mbox series

Patch

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 094ef56ab7b4..5b20087b117f 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1853,7 +1853,7 @@  config CRYPTO_DRBG
 endif	# if CRYPTO_DRBG_MENU
 
 config CRYPTO_JITTERENTROPY
-	tristate "Jitterentropy Non-Deterministic Random Number Generator"
+	bool "Jitterentropy Non-Deterministic Random Number Generator"
 	select CRYPTO_RNG
 	help
 	  The Jitterentropy RNG is a noise that is intended