| Message ID | 20200106203700.21009-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clocksource: timer-ti-dm: Fix regression | expand |
On Mon, Jan 6, 2020 at 12:37 PM Tony Lindgren <tony@atomide.com> wrote: > > Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to > platform_get_irq") caused a regression where we now try to access > uninitialized data for timer: > > drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': > drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used > uninitialized in this function [-Wmaybe-uninitialized] > > On boot we now get: > > Unable to handle kernel NULL pointer dereference at virtual address > 00000004 > ... > (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) > (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) > (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) > > Let's fix the issue by moving platform_get_irq to happen after timer has > been allocated. > > Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") > Cc: Yangtao Li <tiny.windzz@gmail.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> Acked-by: Olof Johansson <olof@lixom.net> > --- > > I did not notice simlar issue with other patches in the series, but > please do double check Yangtao. Yeah, this even seems to be caught at build (but our builds have been so noisy with warnings lately that they're hard to spot): /build/drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': /build/drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used uninitialized in this function [-Wmaybe-uninitialized] 798 | timer->irq = platform_get_irq(pdev, 0); | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ -Olof
On Tue, Jan 7, 2020 at 5:07 AM Olof Johansson <olof@lixom.net> wrote: > > On Mon, Jan 6, 2020 at 12:37 PM Tony Lindgren <tony@atomide.com> wrote: > > > > Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to > > platform_get_irq") caused a regression where we now try to access > > uninitialized data for timer: > > > > drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': > > drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used > > uninitialized in this function [-Wmaybe-uninitialized] > > > > On boot we now get: > > > > Unable to handle kernel NULL pointer dereference at virtual address > > 00000004 > > ... > > (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) > > (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) > > (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) > > > > Let's fix the issue by moving platform_get_irq to happen after timer has > > been allocated. > > > > Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") > > Cc: Yangtao Li <tiny.windzz@gmail.com> > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > Acked-by: Olof Johansson <olof@lixom.net> Acked-by: Yangtao Li <tiny.windzz@gmail.com> I am sorry. I will pay attention next time. > > > --- > > > > I did not notice simlar issue with other patches in the series, but > > please do double check Yangtao. > > Yeah, this even seems to be caught at build (but our builds have been > so noisy with warnings lately that they're hard to spot): > > /build/drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': > /build/drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may > be used uninitialized in this function [-Wmaybe-uninitialized] > 798 | timer->irq = platform_get_irq(pdev, 0); > | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > -Olof
On 06/01/2020 21:37, Tony Lindgren wrote: > Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to > platform_get_irq") caused a regression where we now try to access > uninitialized data for timer: > > drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': > drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used > uninitialized in this function [-Wmaybe-uninitialized] > > On boot we now get: > > Unable to handle kernel NULL pointer dereference at virtual address > 00000004 > ... > (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) > (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) > (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) > > Let's fix the issue by moving platform_get_irq to happen after timer has > been allocated. > > Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") > Cc: Yangtao Li <tiny.windzz@gmail.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- Applied, thanks.
On Tue, 7 Jan 2020 at 02:07, Tony Lindgren <tony@atomide.com> wrote: > > Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to > platform_get_irq") caused a regression where we now try to access > uninitialized data for timer: > > drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': > drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used > uninitialized in this function [-Wmaybe-uninitialized] > > On boot we now get: > > Unable to handle kernel NULL pointer dereference at virtual address > 00000004 > ... > (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) > (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) > (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) > > Let's fix the issue by moving platform_get_irq to happen after timer has > been allocated. > > Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") Thanks for fixing this issue. I have noticed arm BeagleBoard-X15 boot failed on linux next tree (5.5.0-rc5-next-20200110). [ 6.157822] 8<--- cut here --- [ 6.160911] Unable to handle kernel NULL pointer dereference at virtual address 00000004 [ 6.169120] pgd = 25d83e32 [ 6.171903] [00000004] *pgd=80000080204003, *pmd=00000000 [ 6.177358] Internal error: Oops: a06 [#1] SMP ARM [ 6.182179] Modules linked in: [ 6.185260] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.5.0-rc5-next-20200110 #1 [ 6.192694] Hardware name: Generic DRA74X (Flattened Device Tree) [ 6.198832] PC is at omap_dm_timer_probe+0x48/0x310 - Naresh
On 11/01/20 11:58 am, Naresh Kamboju wrote: > On Tue, 7 Jan 2020 at 02:07, Tony Lindgren <tony@atomide.com> wrote: >> >> Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to >> platform_get_irq") caused a regression where we now try to access >> uninitialized data for timer: >> >> drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': >> drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used >> uninitialized in this function [-Wmaybe-uninitialized] >> >> On boot we now get: >> >> Unable to handle kernel NULL pointer dereference at virtual address >> 00000004 >> ... >> (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) >> (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) >> (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) >> >> Let's fix the issue by moving platform_get_irq to happen after timer has >> been allocated. >> >> Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") > > Thanks for fixing this issue. > I have noticed arm BeagleBoard-X15 boot failed on linux next tree > (5.5.0-rc5-next-20200110). > > [ 6.157822] 8<--- cut here --- > [ 6.160911] Unable to handle kernel NULL pointer dereference at > virtual address 00000004 > [ 6.169120] pgd = 25d83e32 > [ 6.171903] [00000004] *pgd=80000080204003, *pmd=00000000 > [ 6.177358] Internal error: Oops: a06 [#1] SMP ARM > [ 6.182179] Modules linked in: > [ 6.185260] CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 5.5.0-rc5-next-20200110 #1 > [ 6.192694] Hardware name: Generic DRA74X (Flattened Device Tree) > [ 6.198832] PC is at omap_dm_timer_probe+0x48/0x310 Tony/Daniel, This is still not in linux-next. Any idea when this will be pushed to linux-next branch? - Keerthy > > - Naresh >
On 16/01/2020 04:46, Keerthy wrote: > > > On 11/01/20 11:58 am, Naresh Kamboju wrote: >> On Tue, 7 Jan 2020 at 02:07, Tony Lindgren <tony@atomide.com> wrote: >>> >>> Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: >>> Switch to >>> platform_get_irq") caused a regression where we now try to access >>> uninitialized data for timer: >>> >>> drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': >>> drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used >>> uninitialized in this function [-Wmaybe-uninitialized] >>> >>> On boot we now get: >>> >>> Unable to handle kernel NULL pointer dereference at virtual address >>> 00000004 >>> ... >>> (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) >>> (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) >>> (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) >>> >>> Let's fix the issue by moving platform_get_irq to happen after timer has >>> been allocated. >>> >>> Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to >>> platform_get_irq") >> >> Thanks for fixing this issue. >> I have noticed arm BeagleBoard-X15 boot failed on linux next tree >> (5.5.0-rc5-next-20200110). >> >> [ 6.157822] 8<--- cut here --- >> [ 6.160911] Unable to handle kernel NULL pointer dereference at >> virtual address 00000004 >> [ 6.169120] pgd = 25d83e32 >> [ 6.171903] [00000004] *pgd=80000080204003, *pmd=00000000 >> [ 6.177358] Internal error: Oops: a06 [#1] SMP ARM >> [ 6.182179] Modules linked in: >> [ 6.185260] CPU: 0 PID: 1 Comm: swapper/0 Not tainted >> 5.5.0-rc5-next-20200110 #1 >> [ 6.192694] Hardware name: Generic DRA74X (Flattened Device Tree) >> [ 6.198832] PC is at omap_dm_timer_probe+0x48/0x310 > > Tony/Daniel, > > This is still not in linux-next. Any idea when this will be pushed to > linux-next branch? It should be actually.
diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c --- a/drivers/clocksource/timer-ti-dm.c +++ b/drivers/clocksource/timer-ti-dm.c @@ -795,14 +795,14 @@ static int omap_dm_timer_probe(struct platform_device *pdev) return -ENODEV; } - timer->irq = platform_get_irq(pdev, 0); - if (timer->irq < 0) - return timer->irq; - timer = devm_kzalloc(dev, sizeof(*timer), GFP_KERNEL); if (!timer) return -ENOMEM; + timer->irq = platform_get_irq(pdev, 0); + if (timer->irq < 0) + return timer->irq; + timer->fclk = ERR_PTR(-ENODEV); timer->io_base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(timer->io_base))
Clean-up commit 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") caused a regression where we now try to access uninitialized data for timer: drivers/clocksource/timer-ti-dm.c: In function 'omap_dm_timer_probe': drivers/clocksource/timer-ti-dm.c:798:13: warning: 'timer' may be used uninitialized in this function [-Wmaybe-uninitialized] On boot we now get: Unable to handle kernel NULL pointer dereference at virtual address 00000004 ... (omap_dm_timer_probe) from [<c061ac7c>] (platform_drv_probe+0x48/0x98) (platform_drv_probe) from [<c0618c04>] (really_probe+0x1dc/0x348) (really_probe) from [<c0618ef4>] (driver_probe_device+0x5c/0x160) Let's fix the issue by moving platform_get_irq to happen after timer has been allocated. Fixes: 8c82723414d5 ("clocksource/drivers/timer-ti-dm: Switch to platform_get_irq") Cc: Yangtao Li <tiny.windzz@gmail.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- I did not notice simlar issue with other patches in the series, but please do double check Yangtao. --- drivers/clocksource/timer-ti-dm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)