Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline'

[Laura Abbott]

On 09/11/2017 11:41 AM, Kees Cook wrote:
> On Mon, Sep 11, 2017 at 11:25 AM, Daniel Micay <danielmicay@xxxxxxxxx> wrote:
> > On Mon, 2017-09-11 at 10:35 -0700, Guenter Roeck wrote:
> > > On Mon, Sep 11, 2017 at 09:36:00AM -0700, Kees Cook wrote:
> > > > On Sat, Sep 9, 2017 at 8:58 PM, Guenter Roeck <linux@xxxxxxxxxxxx>
> > > > wrote:
> > > > > Hi,
> > > > >
> > > > > I noticed that nios2 images crash in mainline. Bisect points to
> > > > > commit
> > > > > 33d72f3822d7 ("init/main.c: extract early boot entropy from the
> > > > > passed
> > > > > cmdline").  Bisect log is attached.
> > > > >
> > > > > As far as I can see, the problem is seen because
> > > > > add_device_randomness()
> > > > > calls random_get_entropy(). However, the underlying timer function
> > > > > used by the nios2 architecture (nios2_timer_read) is not yet
> > > > > initialized,
> > > > > causing a NULL pointer access and crash. A sample crash log is at
> > > > >          http://kerneltests.org/builders/qemu-nios2-master/builds/1
> > > > > 75/steps/qemubuildcommand/logs/stdio
> > > >
> > > > Oh, yikes. Do you have a full call trace? (Does this come through
> > > > get_cycles() or via the It seems like we could either initialize the
> > > > timer earlier or allow it to fall back when not initialized...
> > >
> > > nios2 doesn't give me a traceback. I followed it by adding debug
> > > messages.
> > > The code path is through get_cycles().
> > >
> > > On nios2:
> > >
> > > static u64 nios2_timer_read(struct clocksource *cs)
> > > {
> > >        struct nios2_clocksource *nios2_cs = to_nios2_clksource(cs);
> > >        unsigned long flags;
> > >        u32 count;
> > >
> > >        local_irq_save(flags);
> > >        count = read_timersnapshot(&nios2_cs->timer);   // <- not
> > > initialized
> > >        local_irq_restore(flags);
> > >
> > >        /* Counter is counting down */
> > >        return ~count;
> > > }
> > >
> > > cycles_t get_cycles(void)
> > > {
> > >          return nios2_timer_read(&nios2_cs.cs);
> > > }
> > > EXPORT_SYMBOL(get_cycles);
> > >
> > > Guenter
> >
> > Maybe it should WARN and return 0 for now if that's NULL?
>
> In this case, we'd always WARN. :P But yeah, 0 return on NULL timer
> seems okay to me here. I am curious if it's possible to start the
> timer earlier, though. It's not clear to me where nios2_cs->timer gets
> set.
>
> -Kees

At the bottom of the file is
TIMER_OF_DECLARE(nios2_timer, ALTR_TIMER_COMPATIBLE, nios2_time_init);
so I don't think initialization can happen any earlier if this is
tied to devicetree.

Thanks,
Laura