linux-kernel - Re: [PATCH v2 1/3] crypto: hw_random

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170325073648.cbphcouvi6trmhcj@kozik-lap>
Date:   Sat, 25 Mar 2017 10:36:48 +0300
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Stephan Müller <smueller@...onox.de>
Cc:     Kukjin Kim <kgene@...nel.org>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Matt Mackall <mpm@...enic.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org, linux-crypto@...r.kernel.org,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Arnd Bergmann <arnd@...db.de>, Olof Johansson <olof@...om.net>
Subject: Re: [PATCH v2 1/3] crypto: hw_random - Add new Exynos RNG driver

On Fri, Mar 24, 2017 at 09:41:59PM +0100, Stephan Müller wrote:
> Am Freitag, 24. März 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:
> 
> Hi Krzysztof,
> 
> > +static unsigned int exynos_rng_copy_random(struct exynos_rng_dev *rng,
> > +					   u8 *dst, unsigned int dlen)
> > +{
> > +	unsigned int cnt = 0;
> > +	int i, j;
> > +	u32 val;
> > +
> > +	for (j = 0; j < EXYNOS_RNG_SEED_REGS; j++) {
> > +		val = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
> > +
> > +		for (i = 0; i < 4; i++) {
> > +			dst[cnt] = val & 0xff;
> > +			val >>= 8;
> > +			if (++cnt >= dlen)
> > +				return cnt;
> > +		}
> > +		rng->seed_save[j] = val;
> 
> Just to clarify: is this call right? Shouldn't that be removed? Any RNG that 
> is given to a caller is tainted and should not serve as seed.

In that case I could either re-use RNGs not passed to the caller (like
in the block quoted below) or generate another round of them just for
purpose of next seeding.

With the first approach the problem is that I might wait for such unused
numbers pretty long. If user is requesting large amount of data, then I
will always give him all five output numbers. I will not have unused
numbers.

The second approach seems safe, but requires additional engine run which
will slow down some of the generate() calls.

> > +	}
> > +
> > +	/*
> > +	 * Engine filled all output registers, so read the remaining registers
> > +	 * for storing data as future seed.
> > +	 */
> > +	for (; j < EXYNOS_RNG_SEED_REGS; j++)
> > +		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
> 
> With this call, I guess the questioned line above could go away, right?

This is used in combination with the previous line so I will get five
seeds (for five registers).

Best regards,
Krzysztof