Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751041AbdIKSlG (ORCPT ); Mon, 11 Sep 2017 14:41:06 -0400 Received: from mail-io0-f171.google.com ([209.85.223.171]:36607 "EHLO mail-io0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750927AbdIKSlE (ORCPT ); Mon, 11 Sep 2017 14:41:04 -0400 X-Google-Smtp-Source: AOwi7QBSlZXqpTDySBaRU742kTzzkymmM/HFmtk5/iiDMl407j9WFJ7SWz21driWeaXREc6/C/14EGHl+rdKjtg5jt0= MIME-Version: 1.0 In-Reply-To: <1505154351.14903.1.camel@gmail.com> References: <20170910035815.GA9834@roeck-us.net> <20170911173516.GB27315@roeck-us.net> <1505154351.14903.1.camel@gmail.com> From: Kees Cook Date: Mon, 11 Sep 2017 11:41:03 -0700 X-Google-Sender-Auth: SA_LTui8hJ6Our98vOXg1-q3nGM Message-ID: Subject: Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' To: Daniel Micay Cc: Guenter Roeck , LKML , Ley Foon Tan , nios2-dev@lists.rocketboards.org, Laura Abbott , Andrew Morton Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2178 Lines: 68 On Mon, Sep 11, 2017 at 11:25 AM, Daniel Micay 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 >> > 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 -- Kees Cook Pixel Security