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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Sep 2017 11:41:03 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Daniel Micay <danielmicay@...il.com>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        LKML <linux-kernel@...r.kernel.org>,
        Ley Foon Tan <lftan@...era.com>,
        nios2-dev@...ts.rocketboards.org,
        Laura Abbott <labbott@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: nios2 crash due to 'init/main.c: extract early boot entropy from
 the passed cmdline'

On Mon, Sep 11, 2017 at 11:25 AM, Daniel Micay <danielmicay@...il.com> 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@...ck-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?

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