2013-09-09 07:35:54

by Linus Walleij

[permalink] [raw]
Subject: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

The way I read the Tegra AES RNG is that it has a homebrew
algorithm for initializing the 128bit RNG using timespec and
the unique chip ID. This looks like reinventing the (square)
wheel, instead just grab 128bits from the kernel entropy pool
where the time and (after another patch) chip unique ID is
already mixed in.

Incidentally this also gets rid of a rather ugly
cross-dependence on the machine using an extern declaration.

Cc: Stephen Warren <[email protected]>
Cc: Varun Wadekar <[email protected]>
Cc: Neil Horman <[email protected]>
Cc: [email protected]
Signed-off-by: Linus Walleij <[email protected]>
---
Only compile-tested as I don't have this platform.
---
drivers/crypto/tegra-aes.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/tegra-aes.c b/drivers/crypto/tegra-aes.c
index 2d58da9..7f42bfe 100644
--- a/drivers/crypto/tegra-aes.c
+++ b/drivers/crypto/tegra-aes.c
@@ -199,8 +199,6 @@ static void aes_workqueue_handler(struct work_struct *work);
static DECLARE_WORK(aes_work, aes_workqueue_handler);
static struct workqueue_struct *aes_wq;

-extern unsigned long long tegra_chip_uid(void);
-
static inline u32 aes_readl(struct tegra_aes_dev *dd, u32 offset)
{
return readl(dd->io_base + offset);
@@ -713,9 +711,8 @@ static int tegra_aes_rng_reset(struct crypto_rng *tfm, u8 *seed,
struct tegra_aes_dev *dd = aes_dev;
struct tegra_aes_ctx *ctx = &rng_ctx;
struct tegra_aes_slot *key_slot;
- struct timespec ts;
int ret = 0;
- u64 nsec, tmp[2];
+ u8 tmp[16]; /* 16 bytes = 128 bits of entropy */
u8 *dt;

if (!ctx || !dd) {
@@ -778,14 +775,8 @@ static int tegra_aes_rng_reset(struct crypto_rng *tfm, u8 *seed,
if (dd->ivlen >= (2 * DEFAULT_RNG_BLK_SZ + AES_KEYSIZE_128)) {
dt = dd->iv + DEFAULT_RNG_BLK_SZ + AES_KEYSIZE_128;
} else {
- getnstimeofday(&ts);
- nsec = timespec_to_ns(&ts);
- do_div(nsec, 1000);
- nsec ^= dd->ctr << 56;
- dd->ctr++;
- tmp[0] = nsec;
- tmp[1] = tegra_chip_uid();
- dt = (u8 *)tmp;
+ get_random_bytes(tmp, sizeof(tmp));
+ dt = tmp;
}
memcpy(dd->dt, dt, DEFAULT_RNG_BLK_SZ);

--
1.8.3.1


2013-09-09 16:02:04

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

On 09/09/2013 01:35 AM, Linus Walleij wrote:
> The way I read the Tegra AES RNG is that it has a homebrew
> algorithm for initializing the 128bit RNG using timespec and
> the unique chip ID. This looks like reinventing the (square)
> wheel, instead just grab 128bits from the kernel entropy pool
> where the time and (after another patch) chip unique ID is
> already mixed in.
>
> Incidentally this also gets rid of a rather ugly
> cross-dependence on the machine using an extern declaration.

This sounds reasonable to me, although I know little about the driver.
Varun, can you please comment?

Acked-by: Stephen Warren <[email protected]>

2013-09-13 12:23:38

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

On Mon, Sep 09, 2013 at 10:02:04AM -0600, Stephen Warren wrote:
> On 09/09/2013 01:35 AM, Linus Walleij wrote:
> > The way I read the Tegra AES RNG is that it has a homebrew
> > algorithm for initializing the 128bit RNG using timespec and
> > the unique chip ID. This looks like reinventing the (square)
> > wheel, instead just grab 128bits from the kernel entropy pool
> > where the time and (after another patch) chip unique ID is
> > already mixed in.
> >
> > Incidentally this also gets rid of a rather ugly
> > cross-dependence on the machine using an extern declaration.
>
> This sounds reasonable to me, although I know little about the driver.
> Varun, can you please comment?
>
> Acked-by: Stephen Warren <[email protected]>

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2013-09-13 16:12:36

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

On 09/13/2013 06:23 AM, Herbert Xu wrote:
> On Mon, Sep 09, 2013 at 10:02:04AM -0600, Stephen Warren wrote:
>> On 09/09/2013 01:35 AM, Linus Walleij wrote:
>>> The way I read the Tegra AES RNG is that it has a homebrew
>>> algorithm for initializing the 128bit RNG using timespec and
>>> the unique chip ID. This looks like reinventing the (square)
>>> wheel, instead just grab 128bits from the kernel entropy pool
>>> where the time and (after another patch) chip unique ID is
>>> already mixed in.
>>>
>>> Incidentally this also gets rid of a rather ugly
>>> cross-dependence on the machine using an extern declaration.
>>
>> This sounds reasonable to me, although I know little about the driver.
>> Varun, can you please comment?
>>
>> Acked-by: Stephen Warren <[email protected]>
>
> Patch applied. Thanks.

I'm curious which kernel version it was merged for; it'd be nice to
remove tegra_chip_uid() from the Tegra tree now since it's unused, but
that obviously requires this patch in the history.

2013-09-13 23:39:12

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

On Fri, Sep 13, 2013 at 10:12:36AM -0600, Stephen Warren wrote:
>
> I'm curious which kernel version it was merged for; it'd be nice to
> remove tegra_chip_uid() from the Tegra tree now since it's unused, but
> that obviously requires this patch in the history.

This is in cryptodev which will be pushed upstream for 3.13.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2013-09-16 15:28:22

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

On 09/13/2013 05:39 PM, Herbert Xu wrote:
> On Fri, Sep 13, 2013 at 10:12:36AM -0600, Stephen Warren wrote:
>>
>> I'm curious which kernel version it was merged for; it'd be nice to
>> remove tegra_chip_uid() from the Tegra tree now since it's unused, but
>> that obviously requires this patch in the history.
>
> This is in cryptodev which will be pushed upstream for 3.13.

OK, I guess I'll send you the follow-on cleanup patch then. While it
does touch arch/arm/mach-tegra code, I hope it won't conflict with
anything else going through the Tegra tree, so should be safe.