* [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
@ 2018-05-31 18:23 Steve Longerbeam
2018-05-31 18:25 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Steve Longerbeam @ 2018-05-31 18:23 UTC (permalink / raw)
To: Marek Vasut, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Steve Longerbeam
Just return zero for a rounded rate if requested rate is zero.
This was caught by CONFIG_UBSAN:
[ 192.266748] UBSAN: Undefined behaviour in drivers/clk/clk-versaclock5.c:513:17
[ 192.274050] division by zero
[ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G B WC 4.14.17-02752-g13fb96f #1
[ 192.286378] Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT)
[ 192.293852] Call trace:
[ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
[ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
[ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
[ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
[ 192.317677] [<ffff2000087ab4d0>] __ubsan_handle_divrem_overflow+0x11c/0x170
[ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
[ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
[ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
[ 192.342595] [<ffff2000088483ac>] clk_core_set_rate_nolock+0x48/0x11c
[ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
[ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
[ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
[ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
[ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
[ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
[ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
[ 192.389099] [<ffff20000817f584>] suspend_devices_and_enter+0xf0/0x1778
[ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
[ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
[ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
[ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
[ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
[ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
[ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
[ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
[ 192.438967] Exception stack(0xffff8006cd1cfec0 to 0xffff8006cd1d0000)
[ 192.445480] fec0: 0000000000000001 000000001e303f00 0000000000000004 0000ffff959a5000
[ 192.453397] fee0: 0000000000000000 0000000155510004 0000000000000003 000000000000006d
[ 192.461314] ff00: 0000000000000040 0000000000000000 0000ffffcc304800 0000000000000020
[ 192.469230] ff20: 0000000000000000 0000000000000000 0000000000000001 0000000000000008
[ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840 000000000000003d 0000000000000001
[ 192.485065] ff60: 000000001e303f00 0000ffff959a1508 0000000000000004 000000001e303f00
[ 192.492982] ff80: 0000000000000004 00000000004d4c68 0000000000000001 0000000000000000
[ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820 0000ffff958bec64 0000ffffcc304820
[ 192.508816] ffc0: 0000ffff95912898 0000000020000000 0000000000000001 0000000000000040
[ 192.516733] ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/clk/clk-versaclock5.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index decffb3..113523d 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw *hw, unsigned long rate,
u32 div_int;
u64 div_frc;
+ /* prevent div-by-0 */
+ if (rate == 0)
+ return 0;
+
/* Determine integer part, which is 12 bit wide */
div_int = f_in / rate;
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-05-31 18:23 [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero Steve Longerbeam
@ 2018-05-31 18:25 ` Marek Vasut
2018-05-31 18:32 ` Steve Longerbeam
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2018-05-31 18:25 UTC (permalink / raw)
To: Steve Longerbeam, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Steve Longerbeam
On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
> Just return zero for a rounded rate if requested rate is zero.
>
> This was caught by CONFIG_UBSAN:
>
> [ 192.266748] UBSAN: Undefined behaviour in drivers/clk/clk-versaclock5.c:513:17
> [ 192.274050] division by zero
> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G B WC 4.14.17-02752-g13fb96f #1
> [ 192.286378] Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT)
> [ 192.293852] Call trace:
> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
> [ 192.317677] [<ffff2000087ab4d0>] __ubsan_handle_divrem_overflow+0x11c/0x170
> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
> [ 192.342595] [<ffff2000088483ac>] clk_core_set_rate_nolock+0x48/0x11c
> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
> [ 192.389099] [<ffff20000817f584>] suspend_devices_and_enter+0xf0/0x1778
> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to 0xffff8006cd1d0000)
> [ 192.445480] fec0: 0000000000000001 000000001e303f00 0000000000000004 0000ffff959a5000
> [ 192.453397] fee0: 0000000000000000 0000000155510004 0000000000000003 000000000000006d
> [ 192.461314] ff00: 0000000000000040 0000000000000000 0000ffffcc304800 0000000000000020
> [ 192.469230] ff20: 0000000000000000 0000000000000000 0000000000000001 0000000000000008
> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840 000000000000003d 0000000000000001
> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508 0000000000000004 000000001e303f00
> [ 192.492982] ff80: 0000000000000004 00000000004d4c68 0000000000000001 0000000000000000
> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820 0000ffff958bec64 0000ffffcc304820
> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000 0000000000000001 0000000000000040
> [ 192.516733] ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/clk/clk-versaclock5.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
> index decffb3..113523d 100644
> --- a/drivers/clk/clk-versaclock5.c
> +++ b/drivers/clk/clk-versaclock5.c
> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw *hw, unsigned long rate,
> u32 div_int;
> u64 div_frc;
>
> + /* prevent div-by-0 */
> + if (rate == 0)
> + return 0;
> +
> /* Determine integer part, which is 12 bit wide */
> div_int = f_in / rate;
> /*
>
Can this actually happen ?
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-05-31 18:25 ` Marek Vasut
@ 2018-05-31 18:32 ` Steve Longerbeam
2018-05-31 18:35 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Steve Longerbeam @ 2018-05-31 18:32 UTC (permalink / raw)
To: Marek Vasut, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Steve Longerbeam
Hi Marek,
On 05/31/2018 11:25 AM, Marek Vasut wrote:
> On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
>> Just return zero for a rounded rate if requested rate is zero.
>>
>> This was caught by CONFIG_UBSAN:
>>
>> [ 192.266748] UBSAN: Undefined behaviour in drivers/clk/clk-versaclock5.c:513:17
>> [ 192.274050] division by zero
>> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G B WC 4.14.17-02752-g13fb96f #1
>> [ 192.286378] Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT)
>> [ 192.293852] Call trace:
>> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
>> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
>> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
>> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
>> [ 192.317677] [<ffff2000087ab4d0>] __ubsan_handle_divrem_overflow+0x11c/0x170
>> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
>> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
>> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
>> [ 192.342595] [<ffff2000088483ac>] clk_core_set_rate_nolock+0x48/0x11c
>> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
>> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
>> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
>> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
>> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
>> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
>> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
>> [ 192.389099] [<ffff20000817f584>] suspend_devices_and_enter+0xf0/0x1778
>> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
>> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
>> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
>> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
>> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
>> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
>> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
>> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
>> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to 0xffff8006cd1d0000)
>> [ 192.445480] fec0: 0000000000000001 000000001e303f00 0000000000000004 0000ffff959a5000
>> [ 192.453397] fee0: 0000000000000000 0000000155510004 0000000000000003 000000000000006d
>> [ 192.461314] ff00: 0000000000000040 0000000000000000 0000ffffcc304800 0000000000000020
>> [ 192.469230] ff20: 0000000000000000 0000000000000000 0000000000000001 0000000000000008
>> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840 000000000000003d 0000000000000001
>> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508 0000000000000004 000000001e303f00
>> [ 192.492982] ff80: 0000000000000004 00000000004d4c68 0000000000000001 0000000000000000
>> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820 0000ffff958bec64 0000ffffcc304820
>> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000 0000000000000001 0000000000000040
>> [ 192.516733] ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> drivers/clk/clk-versaclock5.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
>> index decffb3..113523d 100644
>> --- a/drivers/clk/clk-versaclock5.c
>> +++ b/drivers/clk/clk-versaclock5.c
>> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw *hw, unsigned long rate,
>> u32 div_int;
>> u64 div_frc;
>>
>> + /* prevent div-by-0 */
>> + if (rate == 0)
>> + return 0;
>> +
>> /* Determine integer part, which is 12 bit wide */
>> div_int = f_in / rate;
>> /*
>>
> Can this actually happen ?
We caught this using the Renesas 3.6.0 BSP release, when performing
a suspend of rcar-du driver. The rcar_du_pm_suspend() in 3.6.0 BSP is
modified
from mainline version, including calling clk_set_rate() on the crtc
clocks with a
rate of zero. So this is not actually reproducible (yet) in mainline.
Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-05-31 18:32 ` Steve Longerbeam
@ 2018-05-31 18:35 ` Marek Vasut
2018-05-31 18:52 ` Steve Longerbeam
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2018-05-31 18:35 UTC (permalink / raw)
To: Steve Longerbeam, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Steve Longerbeam, Laurent Pinchart,
Kieran Bingham
On 05/31/2018 08:32 PM, Steve Longerbeam wrote:
> Hi Marek,
>
>
> On 05/31/2018 11:25 AM, Marek Vasut wrote:
>> On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
>>> Just return zero for a rounded rate if requested rate is zero.
>>>
>>> This was caught by CONFIG_UBSAN:
>>>
>>> [ 192.266748] UBSAN: Undefined behaviour in
>>> drivers/clk/clk-versaclock5.c:513:17
>>> [ 192.274050] division by zero
>>> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G
>>> B WC 4.14.17-02752-g13fb96f #1
>>> [ 192.286378] Hardware name: Renesas Salvator-X board based on
>>> r8a7795 ES2.0+ (DT)
>>> [ 192.293852] Call trace:
>>> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
>>> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
>>> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
>>> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
>>> [ 192.317677] [<ffff2000087ab4d0>]
>>> __ubsan_handle_divrem_overflow+0x11c/0x170
>>> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
>>> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
>>> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
>>> [ 192.342595] [<ffff2000088483ac>] clk_core_set_rate_nolock+0x48/0x11c
>>> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
>>> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
>>> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
>>> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
>>> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
>>> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
>>> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
>>> [ 192.389099] [<ffff20000817f584>]
>>> suspend_devices_and_enter+0xf0/0x1778
>>> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
>>> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
>>> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
>>> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
>>> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
>>> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
>>> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
>>> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
>>> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to 0xffff8006cd1d0000)
>>> [ 192.445480] fec0: 0000000000000001 000000001e303f00
>>> 0000000000000004 0000ffff959a5000
>>> [ 192.453397] fee0: 0000000000000000 0000000155510004
>>> 0000000000000003 000000000000006d
>>> [ 192.461314] ff00: 0000000000000040 0000000000000000
>>> 0000ffffcc304800 0000000000000020
>>> [ 192.469230] ff20: 0000000000000000 0000000000000000
>>> 0000000000000001 0000000000000008
>>> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840
>>> 000000000000003d 0000000000000001
>>> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508
>>> 0000000000000004 000000001e303f00
>>> [ 192.492982] ff80: 0000000000000004 00000000004d4c68
>>> 0000000000000001 0000000000000000
>>> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820
>>> 0000ffff958bec64 0000ffffcc304820
>>> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000
>>> 0000000000000001 0000000000000040
>>> [ 192.516733] ffe0: 0000000000000000 0000000000000000
>>> 0000000000000000 0000000000000000
>>> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>>
>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>> ---
>>> drivers/clk/clk-versaclock5.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/clk/clk-versaclock5.c
>>> b/drivers/clk/clk-versaclock5.c
>>> index decffb3..113523d 100644
>>> --- a/drivers/clk/clk-versaclock5.c
>>> +++ b/drivers/clk/clk-versaclock5.c
>>> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw
>>> *hw, unsigned long rate,
>>> u32 div_int;
>>> u64 div_frc;
>>> + /* prevent div-by-0 */
>>> + if (rate == 0)
>>> + return 0;
>>> +
>>> /* Determine integer part, which is 12 bit wide */
>>> div_int = f_in / rate;
>>> /*
>>>
>> Can this actually happen ?
>
> We caught this using the Renesas 3.6.0 BSP release, when performing
> a suspend of rcar-du driver. The rcar_du_pm_suspend() in 3.6.0 BSP is
> modified
> from mainline version, including calling clk_set_rate() on the crtc
> clocks with a
> rate of zero. So this is not actually reproducible (yet) in mainline.
So it sets clock to 0 ? Anyway, this looks sane, although maybe the
whole driver could use a once-over to see if there could be more of this.
Or should the clock framework even let us set clock to 0 Hz ?
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-05-31 18:35 ` Marek Vasut
@ 2018-05-31 18:52 ` Steve Longerbeam
2018-05-31 19:37 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Steve Longerbeam @ 2018-05-31 18:52 UTC (permalink / raw)
To: Marek Vasut, Steve Longerbeam, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Laurent Pinchart, Kieran Bingham
On 05/31/2018 11:35 AM, Marek Vasut wrote:
> On 05/31/2018 08:32 PM, Steve Longerbeam wrote:
>> Hi Marek,
>>
>>
>> On 05/31/2018 11:25 AM, Marek Vasut wrote:
>>> On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
>>>> Just return zero for a rounded rate if requested rate is zero.
>>>>
>>>> This was caught by CONFIG_UBSAN:
>>>>
>>>> [ 192.266748] UBSAN: Undefined behaviour in
>>>> drivers/clk/clk-versaclock5.c:513:17
>>>> [ 192.274050] division by zero
>>>> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G
>>>> B WC 4.14.17-02752-g13fb96f #1
>>>> [ 192.286378] Hardware name: Renesas Salvator-X board based on
>>>> r8a7795 ES2.0+ (DT)
>>>> [ 192.293852] Call trace:
>>>> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
>>>> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
>>>> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
>>>> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
>>>> [ 192.317677] [<ffff2000087ab4d0>]
>>>> __ubsan_handle_divrem_overflow+0x11c/0x170
>>>> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
>>>> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
>>>> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
>>>> [ 192.342595] [<ffff2000088483ac>] clk_core_set_rate_nolock+0x48/0x11c
>>>> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
>>>> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
>>>> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
>>>> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
>>>> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
>>>> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
>>>> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
>>>> [ 192.389099] [<ffff20000817f584>]
>>>> suspend_devices_and_enter+0xf0/0x1778
>>>> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
>>>> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
>>>> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
>>>> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
>>>> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
>>>> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
>>>> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
>>>> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
>>>> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to 0xffff8006cd1d0000)
>>>> [ 192.445480] fec0: 0000000000000001 000000001e303f00
>>>> 0000000000000004 0000ffff959a5000
>>>> [ 192.453397] fee0: 0000000000000000 0000000155510004
>>>> 0000000000000003 000000000000006d
>>>> [ 192.461314] ff00: 0000000000000040 0000000000000000
>>>> 0000ffffcc304800 0000000000000020
>>>> [ 192.469230] ff20: 0000000000000000 0000000000000000
>>>> 0000000000000001 0000000000000008
>>>> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840
>>>> 000000000000003d 0000000000000001
>>>> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508
>>>> 0000000000000004 000000001e303f00
>>>> [ 192.492982] ff80: 0000000000000004 00000000004d4c68
>>>> 0000000000000001 0000000000000000
>>>> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820
>>>> 0000ffff958bec64 0000ffffcc304820
>>>> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000
>>>> 0000000000000001 0000000000000040
>>>> [ 192.516733] ffe0: 0000000000000000 0000000000000000
>>>> 0000000000000000 0000000000000000
>>>> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>>>
>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>> ---
>>>> drivers/clk/clk-versaclock5.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/clk-versaclock5.c
>>>> b/drivers/clk/clk-versaclock5.c
>>>> index decffb3..113523d 100644
>>>> --- a/drivers/clk/clk-versaclock5.c
>>>> +++ b/drivers/clk/clk-versaclock5.c
>>>> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw
>>>> *hw, unsigned long rate,
>>>> u32 div_int;
>>>> u64 div_frc;
>>>> + /* prevent div-by-0 */
>>>> + if (rate == 0)
>>>> + return 0;
>>>> +
>>>> /* Determine integer part, which is 12 bit wide */
>>>> div_int = f_in / rate;
>>>> /*
>>>>
>>> Can this actually happen ?
>> We caught this using the Renesas 3.6.0 BSP release, when performing
>> a suspend of rcar-du driver. The rcar_du_pm_suspend() in 3.6.0 BSP is
>> modified
>> from mainline version, including calling clk_set_rate() on the crtc
>> clocks with a
>> rate of zero. So this is not actually reproducible (yet) in mainline.
> So it sets clock to 0 ?
Yep, see
https://kernel.googlesource.com/pub/scm/linux/kernel/git/horms/renesas-bsp/+/rcar-3.6.0/drivers/gpu/drm/rcar-du/rcar_du_drv.c#359
> Anyway, this looks sane, although maybe the
> whole driver could use a once-over to see if there could be more of this.
Actually I do see more potential divide-by-zeros due to a passed rate
of zero, including vc5_pfd_round_rate() and vc5_pfd_set_rate().
I can resubmit this patch fixing all cases in clk-versaclock5.c if you
like (and probably remove the misleading backtrace in the commit
message since it is a Renesas 3.6.0 kernel backtrace not a mainline
backtrace).
Or perhaps just treat this as a heads-up, I'll leave it up to you.
>
> Or should the clock framework even let us set clock to 0 Hz ?
That is a good question, it might make sense for the core clock framework
to not allow passing a rate of 0 on to the clock ops, and instead treat
it generically.
Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-05-31 18:52 ` Steve Longerbeam
@ 2018-05-31 19:37 ` Marek Vasut
2018-06-01 1:46 ` Steve Longerbeam
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2018-05-31 19:37 UTC (permalink / raw)
To: Steve Longerbeam, Steve Longerbeam, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Laurent Pinchart, Kieran Bingham
On 05/31/2018 08:52 PM, Steve Longerbeam wrote:
>
>
> On 05/31/2018 11:35 AM, Marek Vasut wrote:
>> On 05/31/2018 08:32 PM, Steve Longerbeam wrote:
>>> Hi Marek,
>>>
>>>
>>> On 05/31/2018 11:25 AM, Marek Vasut wrote:
>>>> On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
>>>>> Just return zero for a rounded rate if requested rate is zero.
>>>>>
>>>>> This was caught by CONFIG_UBSAN:
>>>>>
>>>>> [ 192.266748] UBSAN: Undefined behaviour in
>>>>> drivers/clk/clk-versaclock5.c:513:17
>>>>> [ 192.274050] division by zero
>>>>> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G
>>>>> B WC 4.14.17-02752-g13fb96f #1
>>>>> [ 192.286378] Hardware name: Renesas Salvator-X board based on
>>>>> r8a7795 ES2.0+ (DT)
>>>>> [ 192.293852] Call trace:
>>>>> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
>>>>> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
>>>>> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
>>>>> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
>>>>> [ 192.317677] [<ffff2000087ab4d0>]
>>>>> __ubsan_handle_divrem_overflow+0x11c/0x170
>>>>> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
>>>>> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
>>>>> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
>>>>> [ 192.342595] [<ffff2000088483ac>]
>>>>> clk_core_set_rate_nolock+0x48/0x11c
>>>>> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
>>>>> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
>>>>> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
>>>>> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
>>>>> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
>>>>> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
>>>>> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
>>>>> [ 192.389099] [<ffff20000817f584>]
>>>>> suspend_devices_and_enter+0xf0/0x1778
>>>>> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
>>>>> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
>>>>> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
>>>>> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
>>>>> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
>>>>> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
>>>>> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
>>>>> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
>>>>> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to
>>>>> 0xffff8006cd1d0000)
>>>>> [ 192.445480] fec0: 0000000000000001 000000001e303f00
>>>>> 0000000000000004 0000ffff959a5000
>>>>> [ 192.453397] fee0: 0000000000000000 0000000155510004
>>>>> 0000000000000003 000000000000006d
>>>>> [ 192.461314] ff00: 0000000000000040 0000000000000000
>>>>> 0000ffffcc304800 0000000000000020
>>>>> [ 192.469230] ff20: 0000000000000000 0000000000000000
>>>>> 0000000000000001 0000000000000008
>>>>> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840
>>>>> 000000000000003d 0000000000000001
>>>>> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508
>>>>> 0000000000000004 000000001e303f00
>>>>> [ 192.492982] ff80: 0000000000000004 00000000004d4c68
>>>>> 0000000000000001 0000000000000000
>>>>> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820
>>>>> 0000ffff958bec64 0000ffffcc304820
>>>>> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000
>>>>> 0000000000000001 0000000000000040
>>>>> [ 192.516733] ffe0: 0000000000000000 0000000000000000
>>>>> 0000000000000000 0000000000000000
>>>>> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>>>>
>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>> ---
>>>>> drivers/clk/clk-versaclock5.c | 4 ++++
>>>>> 1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/clk/clk-versaclock5.c
>>>>> b/drivers/clk/clk-versaclock5.c
>>>>> index decffb3..113523d 100644
>>>>> --- a/drivers/clk/clk-versaclock5.c
>>>>> +++ b/drivers/clk/clk-versaclock5.c
>>>>> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw
>>>>> *hw, unsigned long rate,
>>>>> u32 div_int;
>>>>> u64 div_frc;
>>>>> + /* prevent div-by-0 */
>>>>> + if (rate == 0)
>>>>> + return 0;
>>>>> +
>>>>> /* Determine integer part, which is 12 bit wide */
>>>>> div_int = f_in / rate;
>>>>> /*
>>>>>
>>>> Can this actually happen ?
>>> We caught this using the Renesas 3.6.0 BSP release, when performing
>>> a suspend of rcar-du driver. The rcar_du_pm_suspend() in 3.6.0 BSP is
>>> modified
>>> from mainline version, including calling clk_set_rate() on the crtc
>>> clocks with a
>>> rate of zero. So this is not actually reproducible (yet) in mainline.
>> So it sets clock to 0 ?
>
> Yep, see
>
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/horms/renesas-bsp/+/rcar-3.6.0/drivers/gpu/drm/rcar-du/rcar_du_drv.c#359
>
>
>> Anyway, this looks sane, although maybe the
>> whole driver could use a once-over to see if there could be more of this.
>
> Actually I do see more potential divide-by-zeros due to a passed rate
> of zero, including vc5_pfd_round_rate() and vc5_pfd_set_rate().
>
> I can resubmit this patch fixing all cases in clk-versaclock5.c if you
> like (and probably remove the misleading backtrace in the commit
> message since it is a Renesas 3.6.0 kernel backtrace not a mainline
> backtrace).
>
> Or perhaps just treat this as a heads-up, I'll leave it up to you.
It'd be nice if you resubmitted it fixing all the cases.
>> Or should the clock framework even let us set clock to 0 Hz ?
>
> That is a good question, it might make sense for the core clock framework
> to not allow passing a rate of 0 on to the clock ops, and instead treat
> it generically.
Jupp
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-05-31 19:37 ` Marek Vasut
@ 2018-06-01 1:46 ` Steve Longerbeam
2018-06-01 6:36 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Steve Longerbeam @ 2018-06-01 1:46 UTC (permalink / raw)
To: Marek Vasut, Steve Longerbeam, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Laurent Pinchart, Kieran Bingham
On 05/31/2018 12:37 PM, Marek Vasut wrote:
> On 05/31/2018 08:52 PM, Steve Longerbeam wrote:
>>
>> On 05/31/2018 11:35 AM, Marek Vasut wrote:
>>> On 05/31/2018 08:32 PM, Steve Longerbeam wrote:
>>>> Hi Marek,
>>>>
>>>>
>>>> On 05/31/2018 11:25 AM, Marek Vasut wrote:
>>>>> On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
>>>>>> Just return zero for a rounded rate if requested rate is zero.
>>>>>>
>>>>>> This was caught by CONFIG_UBSAN:
>>>>>>
>>>>>> [ 192.266748] UBSAN: Undefined behaviour in
>>>>>> drivers/clk/clk-versaclock5.c:513:17
>>>>>> [ 192.274050] division by zero
>>>>>> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G
>>>>>> B WC 4.14.17-02752-g13fb96f #1
>>>>>> [ 192.286378] Hardware name: Renesas Salvator-X board based on
>>>>>> r8a7795 ES2.0+ (DT)
>>>>>> [ 192.293852] Call trace:
>>>>>> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
>>>>>> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
>>>>>> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
>>>>>> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
>>>>>> [ 192.317677] [<ffff2000087ab4d0>]
>>>>>> __ubsan_handle_divrem_overflow+0x11c/0x170
>>>>>> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
>>>>>> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
>>>>>> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
>>>>>> [ 192.342595] [<ffff2000088483ac>]
>>>>>> clk_core_set_rate_nolock+0x48/0x11c
>>>>>> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
>>>>>> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
>>>>>> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
>>>>>> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
>>>>>> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
>>>>>> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
>>>>>> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
>>>>>> [ 192.389099] [<ffff20000817f584>]
>>>>>> suspend_devices_and_enter+0xf0/0x1778
>>>>>> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
>>>>>> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
>>>>>> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
>>>>>> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
>>>>>> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
>>>>>> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
>>>>>> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
>>>>>> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
>>>>>> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to
>>>>>> 0xffff8006cd1d0000)
>>>>>> [ 192.445480] fec0: 0000000000000001 000000001e303f00
>>>>>> 0000000000000004 0000ffff959a5000
>>>>>> [ 192.453397] fee0: 0000000000000000 0000000155510004
>>>>>> 0000000000000003 000000000000006d
>>>>>> [ 192.461314] ff00: 0000000000000040 0000000000000000
>>>>>> 0000ffffcc304800 0000000000000020
>>>>>> [ 192.469230] ff20: 0000000000000000 0000000000000000
>>>>>> 0000000000000001 0000000000000008
>>>>>> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840
>>>>>> 000000000000003d 0000000000000001
>>>>>> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508
>>>>>> 0000000000000004 000000001e303f00
>>>>>> [ 192.492982] ff80: 0000000000000004 00000000004d4c68
>>>>>> 0000000000000001 0000000000000000
>>>>>> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820
>>>>>> 0000ffff958bec64 0000ffffcc304820
>>>>>> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000
>>>>>> 0000000000000001 0000000000000040
>>>>>> [ 192.516733] ffe0: 0000000000000000 0000000000000000
>>>>>> 0000000000000000 0000000000000000
>>>>>> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>>>>>
>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>> ---
>>>>>> drivers/clk/clk-versaclock5.c | 4 ++++
>>>>>> 1 file changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/clk/clk-versaclock5.c
>>>>>> b/drivers/clk/clk-versaclock5.c
>>>>>> index decffb3..113523d 100644
>>>>>> --- a/drivers/clk/clk-versaclock5.c
>>>>>> +++ b/drivers/clk/clk-versaclock5.c
>>>>>> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw
>>>>>> *hw, unsigned long rate,
>>>>>> u32 div_int;
>>>>>> u64 div_frc;
>>>>>> + /* prevent div-by-0 */
>>>>>> + if (rate == 0)
>>>>>> + return 0;
>>>>>> +
>>>>>> /* Determine integer part, which is 12 bit wide */
>>>>>> div_int = f_in / rate;
>>>>>> /*
>>>>>>
>>>>> Can this actually happen ?
>>>> We caught this using the Renesas 3.6.0 BSP release, when performing
>>>> a suspend of rcar-du driver. The rcar_du_pm_suspend() in 3.6.0 BSP is
>>>> modified
>>>> from mainline version, including calling clk_set_rate() on the crtc
>>>> clocks with a
>>>> rate of zero. So this is not actually reproducible (yet) in mainline.
>>> So it sets clock to 0 ?
>> Yep, see
>>
>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/horms/renesas-bsp/+/rcar-3.6.0/drivers/gpu/drm/rcar-du/rcar_du_drv.c#359
>>
>>
>>> Anyway, this looks sane, although maybe the
>>> whole driver could use a once-over to see if there could be more of this.
>> Actually I do see more potential divide-by-zeros due to a passed rate
>> of zero, including vc5_pfd_round_rate() and vc5_pfd_set_rate().
>>
>> I can resubmit this patch fixing all cases in clk-versaclock5.c if you
>> like (and probably remove the misleading backtrace in the commit
>> message since it is a Renesas 3.6.0 kernel backtrace not a mainline
>> backtrace).
>>
>> Or perhaps just treat this as a heads-up, I'll leave it up to you.
> It'd be nice if you resubmitted it fixing all the cases.
>
Ok. Please disregard this patch since there won't be a v2, the
new patch will have a different title.
Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero
2018-06-01 1:46 ` Steve Longerbeam
@ 2018-06-01 6:36 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2018-06-01 6:36 UTC (permalink / raw)
To: Steve Longerbeam, Steve Longerbeam, Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Laurent Pinchart, Kieran Bingham
On 06/01/2018 03:46 AM, Steve Longerbeam wrote:
>
>
> On 05/31/2018 12:37 PM, Marek Vasut wrote:
>> On 05/31/2018 08:52 PM, Steve Longerbeam wrote:
>>>
>>> On 05/31/2018 11:35 AM, Marek Vasut wrote:
>>>> On 05/31/2018 08:32 PM, Steve Longerbeam wrote:
>>>>> Hi Marek,
>>>>>
>>>>>
>>>>> On 05/31/2018 11:25 AM, Marek Vasut wrote:
>>>>>> On 05/31/2018 08:23 PM, Steve Longerbeam wrote:
>>>>>>> Just return zero for a rounded rate if requested rate is zero.
>>>>>>>
>>>>>>> This was caught by CONFIG_UBSAN:
>>>>>>>
>>>>>>> [ 192.266748] UBSAN: Undefined behaviour in
>>>>>>> drivers/clk/clk-versaclock5.c:513:17
>>>>>>> [ 192.274050] division by zero
>>>>>>> [ 192.276976] CPU: 0 PID: 2579 Comm: vsp-unit-test-0 Tainted: G
>>>>>>> B WC 4.14.17-02752-g13fb96f #1
>>>>>>> [ 192.286378] Hardware name: Renesas Salvator-X board based on
>>>>>>> r8a7795 ES2.0+ (DT)
>>>>>>> [ 192.293852] Call trace:
>>>>>>> [ 192.296343] [<ffff2000080900dc>] dump_backtrace+0x0/0x390
>>>>>>> [ 192.301807] [<ffff200008090480>] show_stack+0x14/0x1c
>>>>>>> [ 192.306920] [<ffff200008f66574>] dump_stack+0x134/0x1a8
>>>>>>> [ 192.312213] [<ffff2000087aaa30>] ubsan_epilogue+0x14/0x60
>>>>>>> [ 192.317677] [<ffff2000087ab4d0>]
>>>>>>> __ubsan_handle_divrem_overflow+0x11c/0x170
>>>>>>> [ 192.324720] [<ffff200008852120>] vc5_fod_round_rate+0x68/0x148
>>>>>>> [ 192.330620] [<ffff20000884567c>] clk_calc_new_rates+0x238/0x3fc
>>>>>>> [ 192.336607] [<ffff2000088456e0>] clk_calc_new_rates+0x29c/0x3fc
>>>>>>> [ 192.342595] [<ffff2000088483ac>]
>>>>>>> clk_core_set_rate_nolock+0x48/0x11c
>>>>>>> [ 192.349019] [<ffff2000088484b4>] clk_set_rate+0x34/0x4c
>>>>>>> [ 192.354307] [<ffff20000895e304>] rcar_du_pm_suspend+0x274/0x2f4
>>>>>>> [ 192.360297] [<ffff20000898feac>] platform_pm_suspend+0x78/0xb8
>>>>>>> [ 192.366198] [<ffff2000089a5604>] dpm_run_callback+0x584/0xa18
>>>>>>> [ 192.372010] [<ffff2000089a69e0>] __device_suspend+0x1a8/0x534
>>>>>>> [ 192.377822] [<ffff2000089adc48>] dpm_suspend+0x130/0xea0
>>>>>>> [ 192.383197] [<ffff2000089b0344>] dpm_suspend_start+0x130/0x138
>>>>>>> [ 192.389099] [<ffff20000817f584>]
>>>>>>> suspend_devices_and_enter+0xf0/0x1778
>>>>>>> [ 192.395700] [<ffff200008183014>] pm_suspend+0x2408/0x245c
>>>>>>> [ 192.401162] [<ffff20000817c0a4>] state_store+0xf0/0x130
>>>>>>> [ 192.406451] [<ffff200008f6f19c>] kobj_attr_store+0x5c/0x6c
>>>>>>> [ 192.412002] [<ffff2000084f4c94>] sysfs_kf_write+0xe8/0xfc
>>>>>>> [ 192.417466] [<ffff2000084f30b0>] kernfs_fop_write+0x22c/0x2e4
>>>>>>> [ 192.423281] [<ffff2000083e46d4>] __vfs_write+0x104/0x34c
>>>>>>> [ 192.428656] [<ffff2000083e4cc4>] vfs_write+0x134/0x2d8
>>>>>>> [ 192.433857] [<ffff2000083e5150>] SyS_write+0xbc/0x12c
>>>>>>> [ 192.438967] Exception stack(0xffff8006cd1cfec0 to
>>>>>>> 0xffff8006cd1d0000)
>>>>>>> [ 192.445480] fec0: 0000000000000001 000000001e303f00
>>>>>>> 0000000000000004 0000ffff959a5000
>>>>>>> [ 192.453397] fee0: 0000000000000000 0000000155510004
>>>>>>> 0000000000000003 000000000000006d
>>>>>>> [ 192.461314] ff00: 0000000000000040 0000000000000000
>>>>>>> 0000ffffcc304800 0000000000000020
>>>>>>> [ 192.469230] ff20: 0000000000000000 0000000000000000
>>>>>>> 0000000000000001 0000000000000008
>>>>>>> [ 192.477148] ff40: 00000000004eb3b8 0000ffff958bb840
>>>>>>> 000000000000003d 0000000000000001
>>>>>>> [ 192.485065] ff60: 000000001e303f00 0000ffff959a1508
>>>>>>> 0000000000000004 000000001e303f00
>>>>>>> [ 192.492982] ff80: 0000000000000004 00000000004d4c68
>>>>>>> 0000000000000001 0000000000000000
>>>>>>> [ 192.500899] ffa0: 000000001e30d5c0 0000ffffcc304820
>>>>>>> 0000ffff958bec64 0000ffffcc304820
>>>>>>> [ 192.508816] ffc0: 0000ffff95912898 0000000020000000
>>>>>>> 0000000000000001 0000000000000040
>>>>>>> [ 192.516733] ffe0: 0000000000000000 0000000000000000
>>>>>>> 0000000000000000 0000000000000000
>>>>>>> [ 192.524650] [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>>>>>>>
>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>>> ---
>>>>>>> drivers/clk/clk-versaclock5.c | 4 ++++
>>>>>>> 1 file changed, 4 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/clk-versaclock5.c
>>>>>>> b/drivers/clk/clk-versaclock5.c
>>>>>>> index decffb3..113523d 100644
>>>>>>> --- a/drivers/clk/clk-versaclock5.c
>>>>>>> +++ b/drivers/clk/clk-versaclock5.c
>>>>>>> @@ -509,6 +509,10 @@ static long vc5_fod_round_rate(struct clk_hw
>>>>>>> *hw, unsigned long rate,
>>>>>>> u32 div_int;
>>>>>>> u64 div_frc;
>>>>>>> + /* prevent div-by-0 */
>>>>>>> + if (rate == 0)
>>>>>>> + return 0;
>>>>>>> +
>>>>>>> /* Determine integer part, which is 12 bit wide */
>>>>>>> div_int = f_in / rate;
>>>>>>> /*
>>>>>>>
>>>>>> Can this actually happen ?
>>>>> We caught this using the Renesas 3.6.0 BSP release, when performing
>>>>> a suspend of rcar-du driver. The rcar_du_pm_suspend() in 3.6.0 BSP is
>>>>> modified
>>>>> from mainline version, including calling clk_set_rate() on the crtc
>>>>> clocks with a
>>>>> rate of zero. So this is not actually reproducible (yet) in mainline.
>>>> So it sets clock to 0 ?
>>> Yep, see
>>>
>>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/horms/renesas-bsp/+/rcar-3.6.0/drivers/gpu/drm/rcar-du/rcar_du_drv.c#359
>>>
>>>
>>>
>>>> Anyway, this looks sane, although maybe the
>>>> whole driver could use a once-over to see if there could be more of
>>>> this.
>>> Actually I do see more potential divide-by-zeros due to a passed rate
>>> of zero, including vc5_pfd_round_rate() and vc5_pfd_set_rate().
>>>
>>> I can resubmit this patch fixing all cases in clk-versaclock5.c if you
>>> like (and probably remove the misleading backtrace in the commit
>>> message since it is a Renesas 3.6.0 kernel backtrace not a mainline
>>> backtrace).
>>>
>>> Or perhaps just treat this as a heads-up, I'll leave it up to you.
>> It'd be nice if you resubmitted it fixing all the cases.
>>
>
> Ok. Please disregard this patch since there won't be a v2, the
> new patch will have a different title.
Thanks!
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-01 7:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-31 18:23 [PATCH] clk: vc5: Fix div-by-0 when rounding a rate of zero Steve Longerbeam
2018-05-31 18:25 ` Marek Vasut
2018-05-31 18:32 ` Steve Longerbeam
2018-05-31 18:35 ` Marek Vasut
2018-05-31 18:52 ` Steve Longerbeam
2018-05-31 19:37 ` Marek Vasut
2018-06-01 1:46 ` Steve Longerbeam
2018-06-01 6:36 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.