| Message ID | 5692716.lOV4Wx5bFT@positron.chronox.de |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: jitterentropy - bind statically into kernel | expand |
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 > > > >
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 >> >> >> >>
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.
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.
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
[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?
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?
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..
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 --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
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(-)