* nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline'
@ 2017-09-10 3:58 Guenter Roeck
2017-09-11 16:36 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2017-09-10 3:58 UTC (permalink / raw)
To: Daniel Micay
Cc: linux-kernel, Ley Foon Tan, nios2-dev, Laura Abbott, Kees Cook,
Andrew Morton
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/175/steps/qemubuildcommand/logs/stdio
Guenter
---
# bad: [4dfc2788033d30dfccfd4268e06dd73ce2c654ed] Merge tag 'iommu-updates-v4.14' of git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu
# good: [5969d1bb3082b41eba8fd2c826559abe38ccb6df] Merge branch 'gperf-removal'
git bisect start 'HEAD' '5969d1bb3082'
# bad: [fbd01410e89a66f346ba1b3c0161e1198449b746] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
git bisect bad fbd01410e89a66f346ba1b3c0161e1198449b746
# good: [0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c] Merge tag 'pci-v4.14-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci
git bisect good 0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c
# bad: [229cf16d3c8ac2e9b082c223fd0e619dc8f62cc1] sh: defconfig: cleanup from old Kconfig options
git bisect bad 229cf16d3c8ac2e9b082c223fd0e619dc8f62cc1
# good: [1caffba9db4aa27c3e7ebc05668afca1f991ab8d] drivers/scsi/sym53c8xx_2/sym_hipd.c: convert to use memset32
git bisect good 1caffba9db4aa27c3e7ebc05668afca1f991ab8d
# good: [7c61bd6983b185272315722787cceacf5f5d2e7d] lib/cmdline.c: remove meaningless comment
git bisect good 7c61bd6983b185272315722787cceacf5f5d2e7d
# bad: [718b303b49893d8f9dd469f710d76f77673bd35d] autofs: use AUTOFS_DEV_IOCTL_SIZE
git bisect bad 718b303b49893d8f9dd469f710d76f77673bd35d
# good: [9367bb730e4d9d85a8911a08a3542ec2aa873d37] binfmt_flat: delete two error messages for a failed memory allocation in decompress_exec()
git bisect good 9367bb730e4d9d85a8911a08a3542ec2aa873d37
# bad: [e54c7bcbf14a25dc3a913b4c808b52121c522e9b] autofs: make disc device user accessible
git bisect bad e54c7bcbf14a25dc3a913b4c808b52121c522e9b
# bad: [33d72f3822d7ff8a9e45bd7413c811085cb87aa5] init/main.c: extract early boot entropy from the passed cmdline
git bisect bad 33d72f3822d7ff8a9e45bd7413c811085cb87aa5
# good: [121388a31362b0d3176dc1190ac8064b98a61b20] init: move stack canary initialization after setup_arch
git bisect good 121388a31362b0d3176dc1190ac8064b98a61b20
# first bad commit: [33d72f3822d7ff8a9e45bd7413c811085cb87aa5] init/main.c: extract early boot entropy from the passed cmdline
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' 2017-09-10 3:58 nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' Guenter Roeck @ 2017-09-11 16:36 ` Kees Cook 2017-09-11 17:35 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Kees Cook @ 2017-09-11 16:36 UTC (permalink / raw) To: Guenter Roeck Cc: Daniel Micay, LKML, Ley Foon Tan, nios2-dev, Laura Abbott, Andrew Morton 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/175/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... -Kees > > Guenter > > --- > # bad: [4dfc2788033d30dfccfd4268e06dd73ce2c654ed] Merge tag 'iommu-updates-v4.14' of git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu > # good: [5969d1bb3082b41eba8fd2c826559abe38ccb6df] Merge branch 'gperf-removal' > git bisect start 'HEAD' '5969d1bb3082' > # bad: [fbd01410e89a66f346ba1b3c0161e1198449b746] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net > git bisect bad fbd01410e89a66f346ba1b3c0161e1198449b746 > # good: [0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c] Merge tag 'pci-v4.14-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci > git bisect good 0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c > # bad: [229cf16d3c8ac2e9b082c223fd0e619dc8f62cc1] sh: defconfig: cleanup from old Kconfig options > git bisect bad 229cf16d3c8ac2e9b082c223fd0e619dc8f62cc1 > # good: [1caffba9db4aa27c3e7ebc05668afca1f991ab8d] drivers/scsi/sym53c8xx_2/sym_hipd.c: convert to use memset32 > git bisect good 1caffba9db4aa27c3e7ebc05668afca1f991ab8d > # good: [7c61bd6983b185272315722787cceacf5f5d2e7d] lib/cmdline.c: remove meaningless comment > git bisect good 7c61bd6983b185272315722787cceacf5f5d2e7d > # bad: [718b303b49893d8f9dd469f710d76f77673bd35d] autofs: use AUTOFS_DEV_IOCTL_SIZE > git bisect bad 718b303b49893d8f9dd469f710d76f77673bd35d > # good: [9367bb730e4d9d85a8911a08a3542ec2aa873d37] binfmt_flat: delete two error messages for a failed memory allocation in decompress_exec() > git bisect good 9367bb730e4d9d85a8911a08a3542ec2aa873d37 > # bad: [e54c7bcbf14a25dc3a913b4c808b52121c522e9b] autofs: make disc device user accessible > git bisect bad e54c7bcbf14a25dc3a913b4c808b52121c522e9b > # bad: [33d72f3822d7ff8a9e45bd7413c811085cb87aa5] init/main.c: extract early boot entropy from the passed cmdline > git bisect bad 33d72f3822d7ff8a9e45bd7413c811085cb87aa5 > # good: [121388a31362b0d3176dc1190ac8064b98a61b20] init: move stack canary initialization after setup_arch > git bisect good 121388a31362b0d3176dc1190ac8064b98a61b20 > # first bad commit: [33d72f3822d7ff8a9e45bd7413c811085cb87aa5] init/main.c: extract early boot entropy from the passed cmdline -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' 2017-09-11 16:36 ` Kees Cook @ 2017-09-11 17:35 ` Guenter Roeck 2017-09-11 18:25 ` Daniel Micay 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2017-09-11 17:35 UTC (permalink / raw) To: Kees Cook Cc: Daniel Micay, LKML, Ley Foon Tan, nios2-dev, Laura Abbott, Andrew Morton 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/175/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 > -Kees > > > > > Guenter > > > > --- > > # bad: [4dfc2788033d30dfccfd4268e06dd73ce2c654ed] Merge tag 'iommu-updates-v4.14' of git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu > > # good: [5969d1bb3082b41eba8fd2c826559abe38ccb6df] Merge branch 'gperf-removal' > > git bisect start 'HEAD' '5969d1bb3082' > > # bad: [fbd01410e89a66f346ba1b3c0161e1198449b746] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net > > git bisect bad fbd01410e89a66f346ba1b3c0161e1198449b746 > > # good: [0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c] Merge tag 'pci-v4.14-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci > > git bisect good 0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c > > # bad: [229cf16d3c8ac2e9b082c223fd0e619dc8f62cc1] sh: defconfig: cleanup from old Kconfig options > > git bisect bad 229cf16d3c8ac2e9b082c223fd0e619dc8f62cc1 > > # good: [1caffba9db4aa27c3e7ebc05668afca1f991ab8d] drivers/scsi/sym53c8xx_2/sym_hipd.c: convert to use memset32 > > git bisect good 1caffba9db4aa27c3e7ebc05668afca1f991ab8d > > # good: [7c61bd6983b185272315722787cceacf5f5d2e7d] lib/cmdline.c: remove meaningless comment > > git bisect good 7c61bd6983b185272315722787cceacf5f5d2e7d > > # bad: [718b303b49893d8f9dd469f710d76f77673bd35d] autofs: use AUTOFS_DEV_IOCTL_SIZE > > git bisect bad 718b303b49893d8f9dd469f710d76f77673bd35d > > # good: [9367bb730e4d9d85a8911a08a3542ec2aa873d37] binfmt_flat: delete two error messages for a failed memory allocation in decompress_exec() > > git bisect good 9367bb730e4d9d85a8911a08a3542ec2aa873d37 > > # bad: [e54c7bcbf14a25dc3a913b4c808b52121c522e9b] autofs: make disc device user accessible > > git bisect bad e54c7bcbf14a25dc3a913b4c808b52121c522e9b > > # bad: [33d72f3822d7ff8a9e45bd7413c811085cb87aa5] init/main.c: extract early boot entropy from the passed cmdline > > git bisect bad 33d72f3822d7ff8a9e45bd7413c811085cb87aa5 > > # good: [121388a31362b0d3176dc1190ac8064b98a61b20] init: move stack canary initialization after setup_arch > > git bisect good 121388a31362b0d3176dc1190ac8064b98a61b20 > > # first bad commit: [33d72f3822d7ff8a9e45bd7413c811085cb87aa5] init/main.c: extract early boot entropy from the passed cmdline > > > > -- > Kees Cook > Pixel Security ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' 2017-09-11 17:35 ` Guenter Roeck @ 2017-09-11 18:25 ` Daniel Micay 2017-09-11 18:41 ` Kees Cook 0 siblings, 1 reply; 6+ messages in thread From: Daniel Micay @ 2017-09-11 18:25 UTC (permalink / raw) To: Guenter Roeck, Kees Cook Cc: LKML, Ley Foon Tan, nios2-dev, Laura Abbott, Andrew Morton 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' 2017-09-11 18:25 ` Daniel Micay @ 2017-09-11 18:41 ` Kees Cook 2017-09-11 19:09 ` Laura Abbott 0 siblings, 1 reply; 6+ messages in thread From: Kees Cook @ 2017-09-11 18:41 UTC (permalink / raw) To: Daniel Micay Cc: Guenter Roeck, LKML, Ley Foon Tan, nios2-dev, Laura Abbott, Andrew Morton On Mon, Sep 11, 2017 at 11:25 AM, Daniel Micay <danielmicay@gmail.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@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? 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' 2017-09-11 18:41 ` Kees Cook @ 2017-09-11 19:09 ` Laura Abbott 0 siblings, 0 replies; 6+ messages in thread From: Laura Abbott @ 2017-09-11 19:09 UTC (permalink / raw) To: Kees Cook, Daniel Micay Cc: Guenter Roeck, LKML, Ley Foon Tan, nios2-dev, Andrew Morton On 09/11/2017 11:41 AM, Kees Cook wrote: > On Mon, Sep 11, 2017 at 11:25 AM, Daniel Micay <danielmicay@gmail.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@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? > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-11 19:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-10 3:58 nios2 crash due to 'init/main.c: extract early boot entropy from the passed cmdline' Guenter Roeck 2017-09-11 16:36 ` Kees Cook 2017-09-11 17:35 ` Guenter Roeck 2017-09-11 18:25 ` Daniel Micay 2017-09-11 18:41 ` Kees Cook 2017-09-11 19:09 ` Laura Abbott
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox