[PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[Linus Walleij]

This adds a hook at common late init to extract the 64 bits of
chip-unique data and throw it into the entropy pool to make it
more device-unique.

Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Stephen: since I don't have the Tegra reference manual I don't
know what "UID" means, but if it means "unique ID" then this
patch should be relevant for initializing the entropy pool.
---
 arch/arm/mach-tegra/common.c | 1 +
 arch/arm/mach-tegra/fuse.c   | 9 +++++++++
 arch/arm/mach-tegra/fuse.h   | 1 +
 3 files changed, 11 insertions(+)

diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..c1ab5f5 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -109,6 +109,7 @@ void __init tegra_init_early(void)
 
 void __init tegra_init_late(void)
 {
+	tegra_random_init();
 	tegra_init_suspend();
 	tegra_cpuidle_init();
 	tegra_powergate_debugfs_init();
diff --git a/arch/arm/mach-tegra/fuse.c b/arch/arm/mach-tegra/fuse.c
index e035cd2..16673de 100644
--- a/arch/arm/mach-tegra/fuse.c
+++ b/arch/arm/mach-tegra/fuse.c
@@ -22,6 +22,7 @@
 #include <linux/io.h>
 #include <linux/export.h>
 #include <linux/tegra-soc.h>
+#include <linux/random.h>
 
 #include "fuse.h"
 #include "iomap.h"
@@ -165,3 +166,11 @@ unsigned long long tegra_chip_uid(void)
 	return (hi << 32ull) | lo;
 }
 EXPORT_SYMBOL(tegra_chip_uid);
+
+void __init tegra_random_init(void)
+{
+	unsigned long long uid;
+
+	uid = tegra_chip_uid();
+	add_device_randomness(&uid, sizeof(uid));
+}
diff --git a/arch/arm/mach-tegra/fuse.h b/arch/arm/mach-tegra/fuse.h
index def7968..559d101 100644
--- a/arch/arm/mach-tegra/fuse.h
+++ b/arch/arm/mach-tegra/fuse.h
@@ -55,6 +55,7 @@ unsigned long long tegra_chip_uid(void);
 void tegra_init_fuse(void);
 bool tegra_spare_fuse(int bit);
 u32 tegra_fuse_readl(unsigned long offset);
+void tegra_random_init(void);
 
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 void tegra20_init_speedo_data(void);
-- 
1.8.3.1

Re: [PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[Stephen Warren]

On 09/05/2013 09:28 AM, Linus Walleij wrote:
> This adds a hook at common late init to extract the 64 bits of
> chip-unique data and throw it into the entropy pool to make it
> more device-unique.
> 
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Stephen: since I don't have the Tegra reference manual I don't
> know what "UID" means, but if it means "unique ID" then this
> patch should be relevant for initializing the entropy pool.

I believe that's what it means, although even the TRM doesn't actually say.

I'll try and remember to apply this after the merge window.

Note that the UID value appears to be incorrect on anything other than
Tegra20; I'll file an internal bug to track that down.

Re: [PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[Linus Walleij]

On Thu, Sep 5, 2013 at 9:43 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 09/05/2013 09:28 AM, Linus Walleij wrote:
>> This adds a hook at common late init to extract the 64 bits of
>> chip-unique data and throw it into the entropy pool to make it
>> more device-unique.
>>
>> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> Stephen: since I don't have the Tegra reference manual I don't
>> know what "UID" means, but if it means "unique ID" then this
>> patch should be relevant for initializing the entropy pool.
>
> I believe that's what it means, although even the TRM doesn't actually say.

I sort of half-guessed it from the fact that it was being used
as random seed in the crypto driver in
drivers/crypto/tegra-aes.c

> I'll try and remember to apply this after the merge window.
>
> Note that the UID value appears to be incorrect on anything other than
> Tegra20; I'll file an internal bug to track that down.

Hm, that will not affect the kernel entropy pool so much, but
the crypto driver is in trouble since that seems to be the only
entropy it's using :-(

If it will return something like a constant, that is actually a quite
valid case for initializing tmp[1] from the kernel entropy pool
with get_random_bytes() instead of using UID.

I don't know if the Tegra AES needs that specific initializer
though (it seems not). Shall I propose a patch?

Yours,
Linus Walleij

Re: [PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[Stephen Warren]

On 09/06/2013 02:53 AM, Linus Walleij wrote:
> On Thu, Sep 5, 2013 at 9:43 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
...
>> Note that the UID value appears to be incorrect on anything other than
>> Tegra20; I'll file an internal bug to track that down.
> 
> Hm, that will not affect the kernel entropy pool so much, but
> the crypto driver is in trouble since that seems to be the only
> entropy it's using :-(

Well, it calls getnstimeofday() too, so there's presumably a small
amount of entropy there, unless tegra_aes_rng_reset() gets called at
predictable times, which is probably true.

> If it will return something like a constant, that is actually a quite
> valid case for initializing tmp[1] from the kernel entropy pool
> with get_random_bytes() instead of using UID.
> 
> I don't know if the Tegra AES needs that specific initializer
> though (it seems not). Shall I propose a patch?

That sounds reasonable. Perhaps Varun can comment?

Re: [PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[Stephen Warren]

On 09/05/2013 09:28 AM, Linus Walleij wrote:
> This adds a hook at common late init to extract the 64 bits of
> chip-unique data and throw it into the entropy pool to make it
> more device-unique.

> diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c

>  void __init tegra_init_late(void)
>  {
> +	tegra_random_init();

Looking at this further, there's already a tegra_init_fuse() call made
from tegra_init_early(), so rather than adding a second init function
into the fuse code, I'd rather just augment that function, unless
there's some reason add_device_randomness() won't work at that time?

If that will work, I'll supply an alternate patch that replaces this,
since I also want to remove the tegra_chip_uid() function, and need to
make the add_device_randomness() call not use tegra_chip_uid() on
Tegra30 or later, since the set of fuses for the chip ID changed radically.

Re: [PATCH] ARM: tegra: throw the unique chip ID into the entropy pool

[Linus Walleij]

On Fri, Sep 13, 2013 at 6:33 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 09/05/2013 09:28 AM, Linus Walleij wrote:
>> This adds a hook at common late init to extract the 64 bits of
>> chip-unique data and throw it into the entropy pool to make it
>> more device-unique.
>
>> diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
>
>>  void __init tegra_init_late(void)
>>  {
>> +     tegra_random_init();
>
> Looking at this further, there's already a tegra_init_fuse() call made
> from tegra_init_early(), so rather than adding a second init function
> into the fuse code, I'd rather just augment that function, unless
> there's some reason add_device_randomness() won't work at that time?

This seems to be possible, due to the comment above
rand_initialize():

/*
 * Note that setup_arch() may call add_device_randomness()
 * long before we get here. This allows seeding of the pools
 * with some platform dependent data very early in the boot
 * process.
(...)

> If that will work, I'll supply an alternate patch that replaces this,
> since I also want to remove the tegra_chip_uid() function, and need to
> make the add_device_randomness() call not use tegra_chip_uid() on
> Tegra30 or later, since the set of fuses for the chip ID changed radically.

Sure, go ahead.

Yours,
Linus Walleij