Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1608501

Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver

Show all headers | View raw

Am Freitag, 24. März 2017, 15:43:48 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

> On Fri, Mar 24, 2017 at 03:37:59PM +0100, Stephan Müller wrote:
> > Am Freitag, 24. März 2017, 15:24:44 CET schrieb Krzysztof Kozlowski:
> > 
> > Hi Krzysztof,
> > 
> > > +
> > > +static int exynos_rng_set_seed(struct exynos_rng_dev *rng,
> > > +			       const u8 *seed, unsigned int slen)
> > > +{
> > > +	int ret, i;
> > > +	u32 val;
> > > +
> > > +	dev_dbg(rng->dev, "Seeding with %u bytes\n", slen);
> > > +
> > > +	ret = clk_prepare_enable(rng->clk);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (slen < EXYNOS_RNG_SEED_SIZE) {
> > > +		dev_warn(rng->dev, "Seed too short (only %u bytes)\n", slen);
> > > +		ret = -EINVAL;
> > > +		goto out;
> > > +	}
> > > +
> > > +	for (i = 0 ; i < EXYNOS_RNG_SEED_REGS ; i++) {
> > > +		val = seed[i * 4] << 24;
> > > +		val |= seed[i * 4 + 1] << 16;
> > > +		val |= seed[i * 4 + 2] << 8;
> > > +		val |= seed[i * 4 + 3] << 0;
> > > +
> > > +		exynos_rng_writel(rng, val, EXYNOS_RNG_SEED(i));
> > > +	}
> > > +
> > > +	val = exynos_rng_readl(rng, EXYNOS_RNG_STATUS);
> > > +	if (!(val & EXYNOS_RNG_STATUS_SEED_SETTING_DONE)) {
> > > +		dev_warn(rng->dev, "Seed setting not finished\n");
> > > +		ret = -EIO;
> > > +		goto out;
> > > +	}
> > > +
> > > +	ret = 0;
> > > +	/* Save seed for suspend */
> > > +	memcpy(rng->seed_save, seed, slen);
> > 
> > Is this really necessary? If you need to save some seed, shouldn't that be
> > an output of the DRNG and not the real seed?
> 
> When suspended to RAM device will loose the contents of registers
> (including SEED registers) so it has to be initialized with something on
> resume.
> 
> The seed registers are write-only so I cannot read them and store
> contents just before suspend.
> 
> I understand that real seed should not be stored... but then if I am not
> able to re-seed it with same values, I will loose the continuous and
> reproducible pseudo-random generation after suspend. Aren't this
> expected out of PRNG?

An RNG has to be stateless from the perspective of the caller -- this is the 
core implication of entropy.

Then, if you add the initial seed after the RNG lost its state implies that 
the same sequence of random numbers starts again. I.e. where is the 
randomness?
> 
> > Besides, how do you know that slen is not larger than
> > EXYNOS_RNG_SEED_SIZE?
> 
> Right, there is a overflow here. It should be sizeof(rng->seed_save);

shouldn't it be min(rng->seed_save, slen)?
> 
> Thanks for review!
> 
> Best regards,
> Krzysztof



Ciao
Stephan

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 15:30 +0100
  Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Stephan Müller <smueller@chronox.de> - 2017-03-24 15:40 +0100
    Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Stephan Müller <smueller@chronox.de> - 2017-03-24 15:50 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 16:00 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Stephan Müller <smueller@chronox.de> - 2017-03-24 16:00 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 16:00 +0100
    Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 15:50 +0100
  Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2017-03-24 16:30 +0100
    Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 16:50 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2017-03-24 17:20 +0100
        Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 17:30 +0100
          Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2017-03-24 17:50 +0100
            Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 18:10 +0100

csiph-web