Re: [patch 3/6] New Generic HW RNG (#2)

[Sergey Vlasov]

On Sun, May 07, 2006 at 05:39:44PM +0200, Michael Buesch wrote:
> On Sunday 07 May 2006 17:22, you wrote:
> > On Sun, May 07, 2006 at 04:38:09PM +0200, Michael Buesch wrote:
> > > Add a driver for the x86 RNG.
> > > This driver is ported from the old hw_random.c
> > > 
> > [skip]
> > > +static int __init intel_init(struct hwrng *rng)
> > 
> > Cannot be __init anymore - now rng->init could be called at any time.
> 
> Sure, will fix this.
> 
> > Also, there is another problem with putting this function into
> > rng->init - if another RNG has been registered when this module is
> > loaded, ->init will not be called during hwrng_register(), so the
> > module load will succeed even if the chipset does not have RNG
> > hardware.
> 
> Ok, I see. The question is, are we going to hwrng_register() the
> intel, althought there is no device? We check for the PCI IDs.

Most Intel chipset do not really have the hardware RNG - PCI ID
matches, but the check for INTEL_RNG_PRESENT bit in intel_init()
fails.  (In fact, I have not ever seen a board which had that RNG.)

[skip]

> Ah, and I found another bug in hwrng_unregister:
> 	current_rng = list_entry(rng_list.prev, struct hwrng, list);
> current_rng->init() should be called here (if nonNULL). If that fails
> current_rng = NULL;

All that logic in hwrng_register() and hwrng_unregister() looks overly
complex.  Maybe we should just register the miscdevice
unconditionally, and make it return -ENODEV from open() if no RNG is
registered?

Attachment: pgp00000.pgp
Description: PGP signature