Message-ID: <1505154351.14903.1.camel@gmail.com>
Subject: Re: nios2 crash due to 'init/main.c: extract early boot entropy
 from the passed cmdline'
From: Daniel Micay <danielmicay@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>, Kees Cook <keescook@chromium.org>
Cc: LKML <linux-kernel@vger.kernel.org>, Ley Foon Tan <lftan@altera.com>,
        nios2-dev@lists.rocketboards.org, Laura Abbott <labbott@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 11 Sep 2017 14:25:51 -0400
In-Reply-To: <20170911173516.GB27315@roeck-us.net>
References: <20170910035815.GA9834@roeck-us.net>
         <CAGXu5j+JAuxdhuWfhXgq8rGtnJgM8zzGuY-i3sFr-eaPH7W5PQ@mail.gmail.com>
         <20170911173516.GB27315@roeck-us.net>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1763
Lines: 56

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@roeck-us.net>
> > 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?