| Message ID | 20221104154230.52836-1-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | hw_random: treat default_quality as a maximum and default to 1024 | expand |
Am Fri, Nov 04, 2022 at 04:42:30PM +0100 schrieb Jason A. Donenfeld: > Most hw_random devices return entropy which is assumed to be of full > quality, but driver authors don't bother setting the quality knob. Some > hw_random devices return less than full quality entropy, and then driver > authors set the quality knob. Therefore, the entropy crediting should be > opt-out rather than opt-in per-driver, to reflect the actual reality on > the ground. > > For example, the two Raspberry Pi RNG drivers produce full entropy > randomness, and both EDK2 and U-Boot's drivers for these treat them as > such. The result is that EFI then uses these numbers and passes the to > Linux, and Linux credits them as boot, thereby initializing the RNG. > Yet, in Linux, the quality knob was never set to anything, and so on the > chance that Linux is booted without EFI, nothing is ever credited. > That's annoying. > > The same pattern appears to repeat itself throughout various drivers. In > fact, very very few drivers have bothered setting quality=1024. > > So let's invert this logic. A hw_random struct's quality knob now > controls the maximum quality a driver can produce, or 0 to specify 1024. > Then, the module-wide switch called "default_quality" is changed to > represent the maximum quality of any driver. By default it's 1024, and > the quality of any particular driver is then given by: > > min(default_quality, rng->quality ?: 1024); > > This way, the user can still turn this off for weird reasons, yet we get > proper crediting for relevant RNGs. Hm. Wouldn't we need to verify that 1024 is appropriate for all drivers where the quality currently is not set? Thanks, Dominik
Hi Dominik, On Sun, Nov 06, 2022 at 08:05:25AM +0100, Dominik Brodowski wrote: > Am Fri, Nov 04, 2022 at 04:42:30PM +0100 schrieb Jason A. Donenfeld: > > Most hw_random devices return entropy which is assumed to be of full > > quality, but driver authors don't bother setting the quality knob. Some > > hw_random devices return less than full quality entropy, and then driver > > authors set the quality knob. Therefore, the entropy crediting should be > > opt-out rather than opt-in per-driver, to reflect the actual reality on > > the ground. > > > > For example, the two Raspberry Pi RNG drivers produce full entropy > > randomness, and both EDK2 and U-Boot's drivers for these treat them as > > such. The result is that EFI then uses these numbers and passes the to > > Linux, and Linux credits them as boot, thereby initializing the RNG. > > Yet, in Linux, the quality knob was never set to anything, and so on the > > chance that Linux is booted without EFI, nothing is ever credited. > > That's annoying. > > > > The same pattern appears to repeat itself throughout various drivers. In > > fact, very very few drivers have bothered setting quality=1024. > > > > So let's invert this logic. A hw_random struct's quality knob now > > controls the maximum quality a driver can produce, or 0 to specify 1024. > > Then, the module-wide switch called "default_quality" is changed to > > represent the maximum quality of any driver. By default it's 1024, and > > the quality of any particular driver is then given by: > > > > min(default_quality, rng->quality ?: 1024); > > > > This way, the user can still turn this off for weird reasons, yet we get > > proper crediting for relevant RNGs. > > Hm. Wouldn't we need to verify that 1024 is appropriate for all drivers > where the quality currently is not set? No, certainly not, and I think this sort of thought belies a really backwards attitude. Hardware RNGs are assumed to produce good randomness. Some manufacturers provide a caveat, "actually, we're giving raw entropy with only N bits quality", but for the ones who don't, the overarching assumption is that the bits are fully entropic. This is what's done every place else in the field, across operating systems, boot environments, firmwares, and otherwise. It's so much so, that both EDK2's EFI and U-Boot's DTB and U-Boot's EFI will use RNGs for which the Linux driver has an empty quality setting and provide output from these as fully entropic seeds to Linux. And why shouldn't they? It seems entirely reasonable to do, given very okay assumptions. But more generally, this fetishization of entropy estimation has got to come to a close at some point. It wasn't a very good idea in the first place to bake that into the heart of all the Linux RNG APIs, but here we are. Just consider how meaningless the count is: random.c will do some completely bogus hocus pocus with interrupt counting, with disk seeks, with input events, to draw a number out of hat. Or it will twiddle around with `struct timer_list` functions and count some entropy there, which is complete nonsense, but whatever, it's by and large "good enough". However, what we're talking about here are RNG hardware devices that say on the tin "hey I'm an RNG device", which is an infinitely better guarantee that we should count entropy from them (unless, of course, the tin also says, "only count half the bits", or whatever). I mention "good enough", because really, the more important thing here from the security angle is that we're getting bits into the RNG quite fast at boot, and we largely accomplish that now. The next important thing is getting the RNG initialized quickly so that userspace doesn't block. Adding hw_random to the equation makes perfect sense here. And, like RDRAND, there's still a switch to turn this off for lunatics who simply don't trust anything. So, no, the way hw_random is oriented now, the whole thing is backwards, in a way that's not reflected across the rest of the hardware RNG and OS ecosystem, and just results in a total waste. I think it's important that we don't hold up progress here. Jason
Hi Jason, Am Sun, Nov 06, 2022 at 03:40:44PM +0100 schrieb Jason A. Donenfeld: > On Sun, Nov 06, 2022 at 08:05:25AM +0100, Dominik Brodowski wrote: > > Am Fri, Nov 04, 2022 at 04:42:30PM +0100 schrieb Jason A. Donenfeld: > > > Most hw_random devices return entropy which is assumed to be of full > > > quality, but driver authors don't bother setting the quality knob. Some > > > hw_random devices return less than full quality entropy, and then driver > > > authors set the quality knob. Therefore, the entropy crediting should be > > > opt-out rather than opt-in per-driver, to reflect the actual reality on > > > the ground. > > > > > > For example, the two Raspberry Pi RNG drivers produce full entropy > > > randomness, and both EDK2 and U-Boot's drivers for these treat them as > > > such. The result is that EFI then uses these numbers and passes the to > > > Linux, and Linux credits them as boot, thereby initializing the RNG. > > > Yet, in Linux, the quality knob was never set to anything, and so on the > > > chance that Linux is booted without EFI, nothing is ever credited. > > > That's annoying. > > > > > > The same pattern appears to repeat itself throughout various drivers. In > > > fact, very very few drivers have bothered setting quality=1024. > > > > > > So let's invert this logic. A hw_random struct's quality knob now > > > controls the maximum quality a driver can produce, or 0 to specify 1024. > > > Then, the module-wide switch called "default_quality" is changed to > > > represent the maximum quality of any driver. By default it's 1024, and > > > the quality of any particular driver is then given by: > > > > > > min(default_quality, rng->quality ?: 1024); > > > > > > This way, the user can still turn this off for weird reasons, yet we get > > > proper crediting for relevant RNGs. > > > > Hm. Wouldn't we need to verify that 1024 is appropriate for all drivers > > where the quality currently is not set? > > No, certainly not, and I think this sort of thought belies a really > backwards attitude. Hardware RNGs are assumed to produce good > randomness. Some manufacturers provide a caveat, "actually, we're giving > raw entropy with only N bits quality", but for the ones who don't, the > overarching assumption is that the bits are fully entropic. My point is not about the 1024 as an exact value, it's more about "do the driver and the hardware really provide _something_ sensible or not". In the past, the default mode as to feed the output of hw_rng devies to some userspace daemon, which then tried to verify that the device works as expected, and then feeded the data back to the crng core. This userspace indirection is largely removed already (in particular by a patch of mine which starts up the hwrng kernel thread also for devices with quality==0) once the crng is fully initialized, on the rationale that even bad quality data will do no harm. Yet, we may need to be a tad more careful whether or not to trust devices for the initial seeding of the crng. Thanks, Dominik
On 2022-11-04 16:42, Jason A. Donenfeld wrote: > Most hw_random devices return entropy which is assumed to be of full > quality, but driver authors don't bother setting the quality knob. Some > hw_random devices return less than full quality entropy, and then > driver > authors set the quality knob. Therefore, the entropy crediting should > be > opt-out rather than opt-in per-driver, to reflect the actual reality on > the ground. > > For example, the two Raspberry Pi RNG drivers produce full entropy > randomness, and both EDK2 and U-Boot's drivers for these treat them as > such. The result is that EFI then uses these numbers and passes the to > Linux, and Linux credits them as boot, thereby initializing the RNG. > Yet, in Linux, the quality knob was never set to anything, and so on > the > chance that Linux is booted without EFI, nothing is ever credited. > That's annoying. > > The same pattern appears to repeat itself throughout various drivers. > In > fact, very very few drivers have bothered setting quality=1024. > > So let's invert this logic. A hw_random struct's quality knob now > controls the maximum quality a driver can produce, or 0 to specify > 1024. > Then, the module-wide switch called "default_quality" is changed to > represent the maximum quality of any driver. By default it's 1024, and > the quality of any particular driver is then given by: > > min(default_quality, rng->quality ?: 1024); > > This way, the user can still turn this off for weird reasons, yet we > get > proper crediting for relevant RNGs. > > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > arch/um/drivers/random.c | 1 - > drivers/char/hw_random/core.c | 9 +++------ > drivers/char/hw_random/mpfs-rng.c | 1 - > drivers/char/hw_random/s390-trng.c | 1 - > drivers/crypto/atmel-sha204a.c | 1 - > drivers/crypto/caam/caamrng.c | 1 - > drivers/firmware/turris-mox-rwtm.c | 1 - > drivers/usb/misc/chaoskey.c | 1 - > include/linux/hw_random.h | 2 +- > 9 files changed, 4 insertions(+), 14 deletions(-) > > diff --git a/arch/um/drivers/random.c b/arch/um/drivers/random.c > index 32b3341fe970..da985e0dc69a 100644 > --- a/arch/um/drivers/random.c > +++ b/arch/um/drivers/random.c > @@ -82,7 +82,6 @@ static int __init rng_init (void) > sigio_broken(random_fd); > hwrng.name = RNG_MODULE_NAME; > hwrng.read = rng_dev_read; > - hwrng.quality = 1024; > > err = hwrng_register(&hwrng); > if (err) { > diff --git a/drivers/char/hw_random/core.c > b/drivers/char/hw_random/core.c > index cc002b0c2f0c..afde685f5e0a 100644 > --- a/drivers/char/hw_random/core.c > +++ b/drivers/char/hw_random/core.c > @@ -41,14 +41,14 @@ static DEFINE_MUTEX(reading_mutex); > static int data_avail; > static u8 *rng_buffer, *rng_fillbuf; > static unsigned short current_quality; > -static unsigned short default_quality; /* = 0; default to "off" */ > +static unsigned short default_quality = 1024; /* default to maximum */ > > module_param(current_quality, ushort, 0644); > MODULE_PARM_DESC(current_quality, > "current hwrng entropy estimation per 1024 bits of input -- > obsolete, use rng_quality instead"); > module_param(default_quality, ushort, 0644); > MODULE_PARM_DESC(default_quality, > - "default entropy content of hwrng per 1024 bits of input"); > + "default maximum entropy content of hwrng per 1024 bits of input"); > > static void drop_current_rng(void); > static int hwrng_init(struct hwrng *rng); > @@ -170,10 +170,7 @@ static int hwrng_init(struct hwrng *rng) > reinit_completion(&rng->cleanup_done); > > skip_init: > - if (!rng->quality) > - rng->quality = default_quality; > - if (rng->quality > 1024) > - rng->quality = 1024; > + rng->quality = min_t(u16, min_t(u16, default_quality, 1024), > rng->quality ?: 1024); > current_quality = rng->quality; /* obsolete */ > > return 0; > diff --git a/drivers/char/hw_random/mpfs-rng.c > b/drivers/char/hw_random/mpfs-rng.c > index 5813da617a48..c6972734ae62 100644 > --- a/drivers/char/hw_random/mpfs-rng.c > +++ b/drivers/char/hw_random/mpfs-rng.c > @@ -78,7 +78,6 @@ static int mpfs_rng_probe(struct platform_device > *pdev) > > rng_priv->rng.read = mpfs_rng_read; > rng_priv->rng.name = pdev->name; > - rng_priv->rng.quality = 1024; > > platform_set_drvdata(pdev, rng_priv); > > diff --git a/drivers/char/hw_random/s390-trng.c > b/drivers/char/hw_random/s390-trng.c > index 795853dfc46b..cffa326ddc8d 100644 > --- a/drivers/char/hw_random/s390-trng.c > +++ b/drivers/char/hw_random/s390-trng.c > @@ -191,7 +191,6 @@ static struct hwrng trng_hwrng_dev = { > .name = "s390-trng", > .data_read = trng_hwrng_data_read, > .read = trng_hwrng_read, > - .quality = 1024, > }; > > > diff --git a/drivers/crypto/atmel-sha204a.c > b/drivers/crypto/atmel-sha204a.c > index a84b657598c6..c0103e7fc2e7 100644 > --- a/drivers/crypto/atmel-sha204a.c > +++ b/drivers/crypto/atmel-sha204a.c > @@ -107,7 +107,6 @@ static int atmel_sha204a_probe(struct i2c_client > *client, > > i2c_priv->hwrng.name = dev_name(&client->dev); > i2c_priv->hwrng.read = atmel_sha204a_rng_read; > - i2c_priv->hwrng.quality = 1024; > > ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng); > if (ret) > diff --git a/drivers/crypto/caam/caamrng.c > b/drivers/crypto/caam/caamrng.c > index 77d048dfe5d0..1f0e82050976 100644 > --- a/drivers/crypto/caam/caamrng.c > +++ b/drivers/crypto/caam/caamrng.c > @@ -246,7 +246,6 @@ int caam_rng_init(struct device *ctrldev) > ctx->rng.cleanup = caam_cleanup; > ctx->rng.read = caam_read; > ctx->rng.priv = (unsigned long)ctx; > - ctx->rng.quality = 1024; > > dev_info(ctrldev, "registering rng-caam\n"); > > diff --git a/drivers/firmware/turris-mox-rwtm.c > b/drivers/firmware/turris-mox-rwtm.c > index c2d34dc8ba46..6ea5789a89e2 100644 > --- a/drivers/firmware/turris-mox-rwtm.c > +++ b/drivers/firmware/turris-mox-rwtm.c > @@ -528,7 +528,6 @@ static int turris_mox_rwtm_probe(struct > platform_device *pdev) > rwtm->hwrng.name = DRIVER_NAME "_hwrng"; > rwtm->hwrng.read = mox_hwrng_read; > rwtm->hwrng.priv = (unsigned long) rwtm; > - rwtm->hwrng.quality = 1024; > > ret = devm_hwrng_register(dev, &rwtm->hwrng); > if (ret < 0) { > diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c > index 87067c3d6109..6fb5140e29b9 100644 > --- a/drivers/usb/misc/chaoskey.c > +++ b/drivers/usb/misc/chaoskey.c > @@ -200,7 +200,6 @@ static int chaoskey_probe(struct usb_interface > *interface, > > dev->hwrng.name = dev->name ? dev->name : chaoskey_driver.name; > dev->hwrng.read = chaoskey_rng_read; > - dev->hwrng.quality = 1024; > > dev->hwrng_registered = (hwrng_register(&dev->hwrng) == 0); > if (!dev->hwrng_registered) > diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h > index 77c2885c4c13..8a3115516a1b 100644 > --- a/include/linux/hw_random.h > +++ b/include/linux/hw_random.h > @@ -34,7 +34,7 @@ > * @priv: Private data, for use by the RNG driver. > * @quality: Estimation of true entropy in RNG's bitstream > * (in bits of entropy per 1024 bits of input; > - * valid values: 1 to 1024, or 0 for unknown). > + * valid values: 1 to 1024, or 0 for maximum). > */ > struct hwrng { > const char *name; Well, I am not sure if this is the right way to go. So by default a hw rng which does not implement the registration correctly is rewarded with the implicit assumption that it produces 100% of entropy. I see your point - a grep through the kernel code gives the impression that a whole bunch of registrations is done with an empty quality field. What about assuming a default quality of 50% if the field is not filled ?
Hi Dominik, On Mon, Nov 07, 2022 at 08:35:02AM +0100, Dominik Brodowski wrote: > Hi Jason, > > Am Sun, Nov 06, 2022 at 03:40:44PM +0100 schrieb Jason A. Donenfeld: > > On Sun, Nov 06, 2022 at 08:05:25AM +0100, Dominik Brodowski wrote: > > > Am Fri, Nov 04, 2022 at 04:42:30PM +0100 schrieb Jason A. Donenfeld: > > > > Most hw_random devices return entropy which is assumed to be of full > > > > quality, but driver authors don't bother setting the quality knob. Some > > > > hw_random devices return less than full quality entropy, and then driver > > > > authors set the quality knob. Therefore, the entropy crediting should be > > > > opt-out rather than opt-in per-driver, to reflect the actual reality on > > > > the ground. > > > > > > > > For example, the two Raspberry Pi RNG drivers produce full entropy > > > > randomness, and both EDK2 and U-Boot's drivers for these treat them as > > > > such. The result is that EFI then uses these numbers and passes the to > > > > Linux, and Linux credits them as boot, thereby initializing the RNG. > > > > Yet, in Linux, the quality knob was never set to anything, and so on the > > > > chance that Linux is booted without EFI, nothing is ever credited. > > > > That's annoying. > > > > > > > > The same pattern appears to repeat itself throughout various drivers. In > > > > fact, very very few drivers have bothered setting quality=1024. > > > > > > > > So let's invert this logic. A hw_random struct's quality knob now > > > > controls the maximum quality a driver can produce, or 0 to specify 1024. > > > > Then, the module-wide switch called "default_quality" is changed to > > > > represent the maximum quality of any driver. By default it's 1024, and > > > > the quality of any particular driver is then given by: > > > > > > > > min(default_quality, rng->quality ?: 1024); > > > > > > > > This way, the user can still turn this off for weird reasons, yet we get > > > > proper crediting for relevant RNGs. > > > > > > Hm. Wouldn't we need to verify that 1024 is appropriate for all drivers > > > where the quality currently is not set? > > > > No, certainly not, and I think this sort of thought belies a really > > backwards attitude. Hardware RNGs are assumed to produce good > > randomness. Some manufacturers provide a caveat, "actually, we're giving > > raw entropy with only N bits quality", but for the ones who don't, the > > overarching assumption is that the bits are fully entropic. > > My point is not about the 1024 as an exact value, it's more about "do the > driver and the hardware really provide _something_ sensible or not". In the > past, the default mode as to feed the output of hw_rng devies to some > userspace daemon, which then tried to verify that the device works as > expected, and then feeded the data back to the crng core. This userspace > indirection is largely removed already (in particular by a patch of mine > which starts up the hwrng kernel thread also for devices with quality==0) > once the crng is fully initialized, on the rationale that even bad quality > data will do no harm. Yet, we may need to be a tad more careful whether or > not to trust devices for the initial seeding of the crng. I got your point, and I still think it's a bad one, for the reasons already explained to you. If it's a hardware RNG, then it's sensible to assume it provides hardware random bits, unless we have documentation that says it provides something less than perfect. Now you've moved on to talking again about entropy estimation. Stop with this nonsense. Entropy estimation is an impossible proposition that actually results in an infoleak. With that said, a self-test to make sure the hardware isn't completely borked would be a nice thing, but this applies for any device no matter what assumptions are made. So if you want to work on that, go ahead, but it's completely orthogonal to this change here. Jason
Hi Harald, On Mon, Nov 07, 2022 at 10:24:42AM +0100, Harald Freudenberger wrote: > Well, I am not sure if this is the right way to go. So by default a > hw rng which does not implement the registration correctly is > rewarded with the implicit assumption that it produces 100% of > entropy. > I see your point - a grep through the kernel code gives the impression > that a whole bunch of registrations is done with an empty quality > field. What about assuming a default quality of 50% if the field > is not filled ? The vast majority of hardware RNGs do *not* work this way. The reasonable assumption is to assume that a hardware RNG provides fully random bits, unless the documentation leads the driver author to specify something less. Really, just quit with all the nutty mailing list stuff here. Next: "how about 74.4% because that matches the vibrations of cedar trees"... If you want this to be different on a particular kernel, you can set your exact value as a command line. This patch here is simply about a sensible default. Jason
On Mon, Nov 7, 2022 at 12:14 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Dominik, > > On Mon, Nov 07, 2022 at 08:35:02AM +0100, Dominik Brodowski wrote: > > Hi Jason, > > > > Am Sun, Nov 06, 2022 at 03:40:44PM +0100 schrieb Jason A. Donenfeld: > > > On Sun, Nov 06, 2022 at 08:05:25AM +0100, Dominik Brodowski wrote: > > > > Am Fri, Nov 04, 2022 at 04:42:30PM +0100 schrieb Jason A. Donenfeld: > > > > > Most hw_random devices return entropy which is assumed to be of full > > > > > quality, but driver authors don't bother setting the quality knob. Some > > > > > hw_random devices return less than full quality entropy, and then driver > > > > > authors set the quality knob. Therefore, the entropy crediting should be > > > > > opt-out rather than opt-in per-driver, to reflect the actual reality on > > > > > the ground. > > > > > > > > > > For example, the two Raspberry Pi RNG drivers produce full entropy > > > > > randomness, and both EDK2 and U-Boot's drivers for these treat them as > > > > > such. The result is that EFI then uses these numbers and passes the to > > > > > Linux, and Linux credits them as boot, thereby initializing the RNG. > > > > > Yet, in Linux, the quality knob was never set to anything, and so on the > > > > > chance that Linux is booted without EFI, nothing is ever credited. > > > > > That's annoying. > > > > > > > > > > The same pattern appears to repeat itself throughout various drivers. In > > > > > fact, very very few drivers have bothered setting quality=1024. > > > > > > > > > > So let's invert this logic. A hw_random struct's quality knob now > > > > > controls the maximum quality a driver can produce, or 0 to specify 1024. > > > > > Then, the module-wide switch called "default_quality" is changed to > > > > > represent the maximum quality of any driver. By default it's 1024, and > > > > > the quality of any particular driver is then given by: > > > > > > > > > > min(default_quality, rng->quality ?: 1024); > > > > > > > > > > This way, the user can still turn this off for weird reasons, yet we get > > > > > proper crediting for relevant RNGs. > > > > > > > > Hm. Wouldn't we need to verify that 1024 is appropriate for all drivers > > > > where the quality currently is not set? > > > > > > No, certainly not, and I think this sort of thought belies a really > > > backwards attitude. Hardware RNGs are assumed to produce good > > > randomness. Some manufacturers provide a caveat, "actually, we're giving > > > raw entropy with only N bits quality", but for the ones who don't, the > > > overarching assumption is that the bits are fully entropic. > > > > My point is not about the 1024 as an exact value, it's more about "do the > > driver and the hardware really provide _something_ sensible or not". In the > > past, the default mode as to feed the output of hw_rng devies to some > > userspace daemon, which then tried to verify that the device works as > > expected, and then feeded the data back to the crng core. This userspace > > indirection is largely removed already (in particular by a patch of mine > > which starts up the hwrng kernel thread also for devices with quality==0) > > once the crng is fully initialized, on the rationale that even bad quality > > data will do no harm. Yet, we may need to be a tad more careful whether or > > not to trust devices for the initial seeding of the crng. > > I got your point, and I still think it's a bad one, for the reasons > already explained to you. If it's a hardware RNG, then it's sensible to > assume it provides hardware random bits, unless we have documentation > that says it provides something less than perfect. > > Now you've moved on to talking again about entropy estimation. Stop with > this nonsense. Entropy estimation is an impossible proposition that > actually results in an infoleak. With that said, a self-test to make > sure the hardware isn't completely borked would be a nice thing, but > this applies for any device no matter what assumptions are made. So if > you want to work on that, go ahead, but it's completely orthogonal to > this change here. Based on IRC discussion, following up with a v2 with a better commit message. Jason
diff --git a/arch/um/drivers/random.c b/arch/um/drivers/random.c index 32b3341fe970..da985e0dc69a 100644 --- a/arch/um/drivers/random.c +++ b/arch/um/drivers/random.c @@ -82,7 +82,6 @@ static int __init rng_init (void) sigio_broken(random_fd); hwrng.name = RNG_MODULE_NAME; hwrng.read = rng_dev_read; - hwrng.quality = 1024; err = hwrng_register(&hwrng); if (err) { diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index cc002b0c2f0c..afde685f5e0a 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -41,14 +41,14 @@ static DEFINE_MUTEX(reading_mutex); static int data_avail; static u8 *rng_buffer, *rng_fillbuf; static unsigned short current_quality; -static unsigned short default_quality; /* = 0; default to "off" */ +static unsigned short default_quality = 1024; /* default to maximum */ module_param(current_quality, ushort, 0644); MODULE_PARM_DESC(current_quality, "current hwrng entropy estimation per 1024 bits of input -- obsolete, use rng_quality instead"); module_param(default_quality, ushort, 0644); MODULE_PARM_DESC(default_quality, - "default entropy content of hwrng per 1024 bits of input"); + "default maximum entropy content of hwrng per 1024 bits of input"); static void drop_current_rng(void); static int hwrng_init(struct hwrng *rng); @@ -170,10 +170,7 @@ static int hwrng_init(struct hwrng *rng) reinit_completion(&rng->cleanup_done); skip_init: - if (!rng->quality) - rng->quality = default_quality; - if (rng->quality > 1024) - rng->quality = 1024; + rng->quality = min_t(u16, min_t(u16, default_quality, 1024), rng->quality ?: 1024); current_quality = rng->quality; /* obsolete */ return 0; diff --git a/drivers/char/hw_random/mpfs-rng.c b/drivers/char/hw_random/mpfs-rng.c index 5813da617a48..c6972734ae62 100644 --- a/drivers/char/hw_random/mpfs-rng.c +++ b/drivers/char/hw_random/mpfs-rng.c @@ -78,7 +78,6 @@ static int mpfs_rng_probe(struct platform_device *pdev) rng_priv->rng.read = mpfs_rng_read; rng_priv->rng.name = pdev->name; - rng_priv->rng.quality = 1024; platform_set_drvdata(pdev, rng_priv); diff --git a/drivers/char/hw_random/s390-trng.c b/drivers/char/hw_random/s390-trng.c index 795853dfc46b..cffa326ddc8d 100644 --- a/drivers/char/hw_random/s390-trng.c +++ b/drivers/char/hw_random/s390-trng.c @@ -191,7 +191,6 @@ static struct hwrng trng_hwrng_dev = { .name = "s390-trng", .data_read = trng_hwrng_data_read, .read = trng_hwrng_read, - .quality = 1024, }; diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c index a84b657598c6..c0103e7fc2e7 100644 --- a/drivers/crypto/atmel-sha204a.c +++ b/drivers/crypto/atmel-sha204a.c @@ -107,7 +107,6 @@ static int atmel_sha204a_probe(struct i2c_client *client, i2c_priv->hwrng.name = dev_name(&client->dev); i2c_priv->hwrng.read = atmel_sha204a_rng_read; - i2c_priv->hwrng.quality = 1024; ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng); if (ret) diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c index 77d048dfe5d0..1f0e82050976 100644 --- a/drivers/crypto/caam/caamrng.c +++ b/drivers/crypto/caam/caamrng.c @@ -246,7 +246,6 @@ int caam_rng_init(struct device *ctrldev) ctx->rng.cleanup = caam_cleanup; ctx->rng.read = caam_read; ctx->rng.priv = (unsigned long)ctx; - ctx->rng.quality = 1024; dev_info(ctrldev, "registering rng-caam\n"); diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c index c2d34dc8ba46..6ea5789a89e2 100644 --- a/drivers/firmware/turris-mox-rwtm.c +++ b/drivers/firmware/turris-mox-rwtm.c @@ -528,7 +528,6 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) rwtm->hwrng.name = DRIVER_NAME "_hwrng"; rwtm->hwrng.read = mox_hwrng_read; rwtm->hwrng.priv = (unsigned long) rwtm; - rwtm->hwrng.quality = 1024; ret = devm_hwrng_register(dev, &rwtm->hwrng); if (ret < 0) { diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c index 87067c3d6109..6fb5140e29b9 100644 --- a/drivers/usb/misc/chaoskey.c +++ b/drivers/usb/misc/chaoskey.c @@ -200,7 +200,6 @@ static int chaoskey_probe(struct usb_interface *interface, dev->hwrng.name = dev->name ? dev->name : chaoskey_driver.name; dev->hwrng.read = chaoskey_rng_read; - dev->hwrng.quality = 1024; dev->hwrng_registered = (hwrng_register(&dev->hwrng) == 0); if (!dev->hwrng_registered) diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h index 77c2885c4c13..8a3115516a1b 100644 --- a/include/linux/hw_random.h +++ b/include/linux/hw_random.h @@ -34,7 +34,7 @@ * @priv: Private data, for use by the RNG driver. * @quality: Estimation of true entropy in RNG's bitstream * (in bits of entropy per 1024 bits of input; - * valid values: 1 to 1024, or 0 for unknown). + * valid values: 1 to 1024, or 0 for maximum). */ struct hwrng { const char *name;
Most hw_random devices return entropy which is assumed to be of full quality, but driver authors don't bother setting the quality knob. Some hw_random devices return less than full quality entropy, and then driver authors set the quality knob. Therefore, the entropy crediting should be opt-out rather than opt-in per-driver, to reflect the actual reality on the ground. For example, the two Raspberry Pi RNG drivers produce full entropy randomness, and both EDK2 and U-Boot's drivers for these treat them as such. The result is that EFI then uses these numbers and passes the to Linux, and Linux credits them as boot, thereby initializing the RNG. Yet, in Linux, the quality knob was never set to anything, and so on the chance that Linux is booted without EFI, nothing is ever credited. That's annoying. The same pattern appears to repeat itself throughout various drivers. In fact, very very few drivers have bothered setting quality=1024. So let's invert this logic. A hw_random struct's quality knob now controls the maximum quality a driver can produce, or 0 to specify 1024. Then, the module-wide switch called "default_quality" is changed to represent the maximum quality of any driver. By default it's 1024, and the quality of any particular driver is then given by: min(default_quality, rng->quality ?: 1024); This way, the user can still turn this off for weird reasons, yet we get proper crediting for relevant RNGs. Cc: Ard Biesheuvel <ardb@kernel.org> Cc: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- arch/um/drivers/random.c | 1 - drivers/char/hw_random/core.c | 9 +++------ drivers/char/hw_random/mpfs-rng.c | 1 - drivers/char/hw_random/s390-trng.c | 1 - drivers/crypto/atmel-sha204a.c | 1 - drivers/crypto/caam/caamrng.c | 1 - drivers/firmware/turris-mox-rwtm.c | 1 - drivers/usb/misc/chaoskey.c | 1 - include/linux/hw_random.h | 2 +- 9 files changed, 4 insertions(+), 14 deletions(-)