| Message ID | 1378394938-1551-1-git-send-email-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@linaro.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.
On Thu, Sep 5, 2013 at 9:43 PM, Stephen Warren <swarren@wwwdotorg.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@linaro.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
On 09/06/2013 02:53 AM, Linus Walleij wrote: > On Thu, Sep 5, 2013 at 9:43 PM, Stephen Warren <swarren@wwwdotorg.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?
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.
On Fri, Sep 13, 2013 at 6:33 PM, Stephen Warren <swarren@wwwdotorg.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
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);
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@linaro.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(+)