[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c8e40eca-c84f-853f-f9f0-1f320fedeeb3@redhat.com>
Date: Mon, 11 Sep 2017 12:09:12 -0700
From: Laura Abbott <labbott@...hat.com>
To: Kees Cook <keescook@...omium.org>,
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,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: nios2 crash due to 'init/main.c: extract early boot entropy from
the passed cmdline'
On 09/11/2017 11:41 AM, Kees Cook wrote:
> 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
>
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
Powered by blists - more mailing lists