Skip to content

Commit

Permalink
hwrng: core - treat default_quality as a maximum and default to 1024
Browse files Browse the repository at this point in the history
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.

Looking at the git history of existing drivers and corresponding mailing
list discussion, this conclusion tracks. There's been a decent amount of
discussion about drivers that set quality < 1024 -- somebody read and
interepreted a datasheet, or made some back of the envelope calculation
somehow. But there's been very little, if any, discussion about most
drivers where the quality is just set to 1024 or unset (or set to 1000
when the authors misunderstood the API and assumed it was base-10 rather
than base-2); in both cases the intent was fairly clear of, "this is a
hardware random device; it's fine."

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 (and we can
replace whatever driver-specific disabling hacks existed in the past),
yet we get proper crediting for relevant RNGs.

Cc: Dominik Brodowski <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Herbert Xu <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
  • Loading branch information
zx2c4 authored and herbertx committed Nov 18, 2022
1 parent 557ffd5 commit 16bdbae
Show file tree
Hide file tree
Showing 16 changed files with 4 additions and 27 deletions.
1 change: 0 additions & 1 deletion arch/um/drivers/random.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion drivers/char/hw_random/cavium-rng-vf.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ static int cavium_rng_probe_vf(struct pci_dev *pdev,
return -ENOMEM;

rng->ops.read = cavium_rng_read;
rng->ops.quality = 1000;

pci_set_drvdata(pdev, rng);

Expand Down
1 change: 0 additions & 1 deletion drivers/char/hw_random/cn10k-rng.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ static int cn10k_rng_probe(struct pci_dev *pdev, const struct pci_device_id *id)
return -ENOMEM;

rng->ops.read = cn10k_rng_read;
rng->ops.quality = 1000;
rng->ops.priv = (unsigned long)rng;

reset_rng_health_state(rng);
Expand Down
9 changes: 3 additions & 6 deletions drivers/char/hw_random/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion drivers/char/hw_random/mpfs-rng.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 0 additions & 1 deletion drivers/char/hw_random/npcm-rng.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ static int npcm_rng_probe(struct platform_device *pdev)
priv->rng.name = pdev->name;
priv->rng.read = npcm_rng_read;
priv->rng.priv = (unsigned long)&pdev->dev;
priv->rng.quality = 1000;
priv->clkp = (u32)(uintptr_t)of_device_get_match_data(&pdev->dev);

writel(NPCM_RNG_M1ROSEL, priv->base + NPCM_RNGMODE_REG);
Expand Down
1 change: 0 additions & 1 deletion drivers/char/hw_random/s390-trng.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};


Expand Down
2 changes: 0 additions & 2 deletions drivers/char/hw_random/timeriomem-rng.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,6 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
if (!of_property_read_u32(pdev->dev.of_node,
"quality", &i))
priv->rng_ops.quality = i;
else
priv->rng_ops.quality = 0;
} else {
period = pdata->period;
priv->rng_ops.quality = pdata->quality;
Expand Down
1 change: 0 additions & 1 deletion drivers/char/hw_random/virtio-rng.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ static int probe_common(struct virtio_device *vdev)
.cleanup = virtio_cleanup,
.priv = (unsigned long)vi,
.name = vi->name,
.quality = 1000,
};
vdev->priv = vi;

Expand Down
1 change: 0 additions & 1 deletion drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ int sun8i_ce_hwrng_register(struct sun8i_ce_dev *ce)
}
ce->trng.name = "sun8i Crypto Engine TRNG";
ce->trng.read = sun8i_ce_trng_read;
ce->trng.quality = 1000;

ret = hwrng_register(&ce->trng);
if (ret)
Expand Down
1 change: 0 additions & 1 deletion drivers/crypto/atmel-sha204a.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion drivers/crypto/caam/caamrng.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
1 change: 0 additions & 1 deletion drivers/firmware/turris-mox-rwtm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 0 additions & 6 deletions drivers/s390/crypto/zcrypt_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ MODULE_LICENSE("GPL");
EXPORT_TRACEPOINT_SYMBOL(s390_zcrypt_req);
EXPORT_TRACEPOINT_SYMBOL(s390_zcrypt_rep);

static int zcrypt_hwrng_seed = 1;
module_param_named(hwrng_seed, zcrypt_hwrng_seed, int, 0440);
MODULE_PARM_DESC(hwrng_seed, "Turn on/off hwrng auto seed, default is 1 (on).");

DEFINE_SPINLOCK(zcrypt_list_lock);
LIST_HEAD(zcrypt_card_list);

Expand Down Expand Up @@ -2063,8 +2059,6 @@ int zcrypt_rng_device_add(void)
goto out;
}
zcrypt_rng_buffer_index = 0;
if (!zcrypt_hwrng_seed)
zcrypt_rng_dev.quality = 0;
rc = hwrng_register(&zcrypt_rng_dev);
if (rc)
goto out_free;
Expand Down
1 change: 0 additions & 1 deletion drivers/usb/misc/chaoskey.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion include/linux/hw_random.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 16bdbae

Please sign in to comment.