diff mbox series

mmc: core: Add CIDs for cards to the entropy pool

Message ID 20220415222931.481352-1-linus.walleij@linaro.org
State New
Headers show
Series mmc: core: Add CIDs for cards to the entropy pool | expand

Commit Message

Linus Walleij April 15, 2022, 10:29 p.m. UTC
To make the entropy pool a bit better we can toss in the
CID for eMMC and SD cards into it, usually the serial
number portion is at least unique.

This does not count as improvement of the entropy but
in practice it makes it a bit more random to mix in these
numbers.

Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/core/mmc.c | 7 +++++++
 drivers/mmc/core/sd.c  | 7 +++++++
 2 files changed, 14 insertions(+)

Comments

Jason A. Donenfeld April 16, 2022, 10:52 a.m. UTC | #1
Hi Linus,

This seems like a pretty nice idea. It seems like a lot of small
embedded boards are fairly deterministic and have lots of identical
board information, with possibly the only thing differing
board-to-board on firstboot being whatever serial number is baked into
the SD card. And the input pool only ever accumulates with
add_device_randomness(), so in the worst case, this won't hurt
anything. So:

Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Jason
Ulf Hansson April 19, 2022, 6:14 p.m. UTC | #2
On Sat, 16 Apr 2022 at 00:31, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> To make the entropy pool a bit better we can toss in the
> CID for eMMC and SD cards into it, usually the serial
> number portion is at least unique.
>
> This does not count as improvement of the entropy but
> in practice it makes it a bit more random to mix in these
> numbers.
>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/core/mmc.c | 7 +++++++
>  drivers/mmc/core/sd.c  | 7 +++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index e7ea45386c22..974d8a02b966 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -12,6 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/stat.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/random.h>
>  #include <linux/sysfs.h>
>
>  #include <linux/mmc/host.h>
> @@ -1673,6 +1674,12 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>                 err = mmc_decode_cid(card);
>                 if (err)
>                         goto free_card;
> +               /*
> +                * Add card ID (cid) data to the entropy pool.
> +                * It doesn't matter that not all of it is unique,
> +                * it's just bonus entropy.
> +                */
> +               add_device_randomness(&card->cid, sizeof(card->cid));

We can move this into mmc_decode_cid() instead, to avoid open coding.
Moreover, this would make it work for SDIO/SD combo cards too,
whatever that matters.

One thing though, what is the typical execution time to do this?
Probably negligible, but as this may be a card that holds the rootfs,
it could delay the boot to be completed.

[...]

Kind regards
Uffe
Jason A. Donenfeld April 19, 2022, 6:45 p.m. UTC | #3
Hi Ulf,

On Tue, Apr 19, 2022 at 8:15 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> We can move this into mmc_decode_cid() instead, to avoid open coding.
> Moreover, this would make it work for SDIO/SD combo cards too,
> whatever that matters.

Making it work in more places sounds good.

> One thing though, what is the typical execution time to do this?
> Probably negligible, but as this may be a card that holds the rootfs,
> it could delay the boot to be completed.

Extremely negligible. Basically the only place you shouldn't call
add_device_randomness() is from a hard IRQ handler, but other than
that, go wild. It's actually used all over the boot sequence pretty
liberally.

Grep around a bit and you'll see all the odd drivers it's called from.
And perhaps in the process you'll notice a few other places that might
benefit from calling it. In general, if you have a strong sense of
niche hardware things where you notice that a certain identifier or
field is "probably quasi-unique-ish", an initiative like the one Linus
took with this patch seems like a positive thing to me.

Jason
diff mbox series

Patch

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index e7ea45386c22..974d8a02b966 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -12,6 +12,7 @@ 
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/pm_runtime.h>
+#include <linux/random.h>
 #include <linux/sysfs.h>
 
 #include <linux/mmc/host.h>
@@ -1673,6 +1674,12 @@  static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		err = mmc_decode_cid(card);
 		if (err)
 			goto free_card;
+		/*
+		 * Add card ID (cid) data to the entropy pool.
+		 * It doesn't matter that not all of it is unique,
+		 * it's just bonus entropy.
+		 */
+		add_device_randomness(&card->cid, sizeof(card->cid));
 	}
 
 	/*
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 68df6b2f49cc..65e0ac031e2a 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -12,6 +12,7 @@ 
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/pm_runtime.h>
+#include <linux/random.h>
 #include <linux/scatterlist.h>
 #include <linux/sysfs.h>
 
@@ -1443,6 +1444,12 @@  static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 			goto free_card;
 
 		mmc_decode_cid(card);
+		/*
+		 * Add card ID (cid) data to the entropy pool.
+		 * It doesn't matter that not all of it is unique,
+		 * it's just bonus entropy.
+		 */
+		add_device_randomness(&card->cid, sizeof(card->cid));
 	}
 
 	/*