| Message ID | 20230519153327.231806-1-n.zhandarovich@fintech.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/radeon: fix possible division-by-zero errors | expand |
In practice this should never happen. Applied with some minor coding style fixes. Alex On Fri, May 19, 2023 at 11:33 AM Nikita Zhandarovich <n.zhandarovich@fintech.ru> wrote: > > Function rv740_get_decoded_reference_divider() may return 0 due to > unpredictable reference divider value calculated in > radeon_atom_get_clock_dividers(). This will lead to > division-by-zero error once that value is used as a divider > in calculating 'clk_s'. > While unlikely, this issue should nonetheless be prevented so add a > sanity check for such cases by testing 'decoded_ref' value against 0. > > Found by Linux Verification Center (linuxtesting.org) with static > analysis tool SVACE. > > Fixes: 66229b200598 ("drm/radeon/kms: add dpm support for rv7xx (v4)") > Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> > > --- > drivers/gpu/drm/radeon/cypress_dpm.c | 7 +++++-- > drivers/gpu/drm/radeon/ni_dpm.c | 7 +++++-- > drivers/gpu/drm/radeon/rv740_dpm.c | 7 +++++-- > 3 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/cypress_dpm.c b/drivers/gpu/drm/radeon/cypress_dpm.c > index fdddbbaecbb7..3678b7e384e1 100644 > --- a/drivers/gpu/drm/radeon/cypress_dpm.c > +++ b/drivers/gpu/drm/radeon/cypress_dpm.c > @@ -555,10 +555,13 @@ static int cypress_populate_mclk_value(struct radeon_device *rdev, > > if (radeon_atombios_get_asic_ss_info(rdev, &ss, > ASIC_INTERNAL_MEMORY_SS, vco_freq)) { > + u32 clk_s, clk_v; > u32 reference_clock = rdev->clock.mpll.reference_freq; > u32 decoded_ref = rv740_get_decoded_reference_divider(dividers.ref_div); > - u32 clk_s = reference_clock * 5 / (decoded_ref * ss.rate); > - u32 clk_v = ss.percentage * > + if (!decoded_ref) > + return -EINVAL; > + clk_s = reference_clock * 5 / (decoded_ref * ss.rate); > + clk_v = ss.percentage * > (0x4000 * dividers.whole_fb_div + 0x800 * dividers.frac_fb_div) / (clk_s * 625); > > mpll_ss1 &= ~CLKV_MASK; > diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c > index 672d2239293e..9ce3e5635efc 100644 > --- a/drivers/gpu/drm/radeon/ni_dpm.c > +++ b/drivers/gpu/drm/radeon/ni_dpm.c > @@ -2239,10 +2239,13 @@ static int ni_populate_mclk_value(struct radeon_device *rdev, > > if (radeon_atombios_get_asic_ss_info(rdev, &ss, > ASIC_INTERNAL_MEMORY_SS, vco_freq)) { > + u32 clk_s, clk_v; > u32 reference_clock = rdev->clock.mpll.reference_freq; > u32 decoded_ref = rv740_get_decoded_reference_divider(dividers.ref_div); > - u32 clk_s = reference_clock * 5 / (decoded_ref * ss.rate); > - u32 clk_v = ss.percentage * > + if (!decoded_ref) > + return -EINVAL; > + clk_s = reference_clock * 5 / (decoded_ref * ss.rate); > + clk_v = ss.percentage * > (0x4000 * dividers.whole_fb_div + 0x800 * dividers.frac_fb_div) / (clk_s * 625); > > mpll_ss1 &= ~CLKV_MASK; > diff --git a/drivers/gpu/drm/radeon/rv740_dpm.c b/drivers/gpu/drm/radeon/rv740_dpm.c > index d57a3e1df8d6..ca76efa0f59d 100644 > --- a/drivers/gpu/drm/radeon/rv740_dpm.c > +++ b/drivers/gpu/drm/radeon/rv740_dpm.c > @@ -247,10 +247,13 @@ int rv740_populate_mclk_value(struct radeon_device *rdev, > > if (radeon_atombios_get_asic_ss_info(rdev, &ss, > ASIC_INTERNAL_MEMORY_SS, vco_freq)) { > + u32 clk_s, clk_v; > u32 reference_clock = rdev->clock.mpll.reference_freq; > u32 decoded_ref = rv740_get_decoded_reference_divider(dividers.ref_div); > - u32 clk_s = reference_clock * 5 / (decoded_ref * ss.rate); > - u32 clk_v = 0x40000 * ss.percentage * > + if (!decoded_ref) > + return -EINVAL; > + clk_s = reference_clock * 5 / (decoded_ref * ss.rate); > + clk_v = 0x40000 * ss.percentage * > (dividers.whole_fb_div + (dividers.frac_fb_div / 8)) / (clk_s * 10000); > > mpll_ss1 &= ~CLKV_MASK; > -- > 2.25.1 >
diff --git a/drivers/gpu/drm/radeon/cypress_dpm.c b/drivers/gpu/drm/radeon/cypress_dpm.c index fdddbbaecbb7..3678b7e384e1 100644 --- a/drivers/gpu/drm/radeon/cypress_dpm.c +++ b/drivers/gpu/drm/radeon/cypress_dpm.c @@ -555,10 +555,13 @@ static int cypress_populate_mclk_value(struct radeon_device *rdev, if (radeon_atombios_get_asic_ss_info(rdev, &ss, ASIC_INTERNAL_MEMORY_SS, vco_freq)) { + u32 clk_s, clk_v; u32 reference_clock = rdev->clock.mpll.reference_freq; u32 decoded_ref = rv740_get_decoded_reference_divider(dividers.ref_div); - u32 clk_s = reference_clock * 5 / (decoded_ref * ss.rate); - u32 clk_v = ss.percentage * + if (!decoded_ref) + return -EINVAL; + clk_s = reference_clock * 5 / (decoded_ref * ss.rate); + clk_v = ss.percentage * (0x4000 * dividers.whole_fb_div + 0x800 * dividers.frac_fb_div) / (clk_s * 625); mpll_ss1 &= ~CLKV_MASK; diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c index 672d2239293e..9ce3e5635efc 100644 --- a/drivers/gpu/drm/radeon/ni_dpm.c +++ b/drivers/gpu/drm/radeon/ni_dpm.c @@ -2239,10 +2239,13 @@ static int ni_populate_mclk_value(struct radeon_device *rdev, if (radeon_atombios_get_asic_ss_info(rdev, &ss, ASIC_INTERNAL_MEMORY_SS, vco_freq)) { + u32 clk_s, clk_v; u32 reference_clock = rdev->clock.mpll.reference_freq; u32 decoded_ref = rv740_get_decoded_reference_divider(dividers.ref_div); - u32 clk_s = reference_clock * 5 / (decoded_ref * ss.rate); - u32 clk_v = ss.percentage * + if (!decoded_ref) + return -EINVAL; + clk_s = reference_clock * 5 / (decoded_ref * ss.rate); + clk_v = ss.percentage * (0x4000 * dividers.whole_fb_div + 0x800 * dividers.frac_fb_div) / (clk_s * 625); mpll_ss1 &= ~CLKV_MASK; diff --git a/drivers/gpu/drm/radeon/rv740_dpm.c b/drivers/gpu/drm/radeon/rv740_dpm.c index d57a3e1df8d6..ca76efa0f59d 100644 --- a/drivers/gpu/drm/radeon/rv740_dpm.c +++ b/drivers/gpu/drm/radeon/rv740_dpm.c @@ -247,10 +247,13 @@ int rv740_populate_mclk_value(struct radeon_device *rdev, if (radeon_atombios_get_asic_ss_info(rdev, &ss, ASIC_INTERNAL_MEMORY_SS, vco_freq)) { + u32 clk_s, clk_v; u32 reference_clock = rdev->clock.mpll.reference_freq; u32 decoded_ref = rv740_get_decoded_reference_divider(dividers.ref_div); - u32 clk_s = reference_clock * 5 / (decoded_ref * ss.rate); - u32 clk_v = 0x40000 * ss.percentage * + if (!decoded_ref) + return -EINVAL; + clk_s = reference_clock * 5 / (decoded_ref * ss.rate); + clk_v = 0x40000 * ss.percentage * (dividers.whole_fb_div + (dividers.frac_fb_div / 8)) / (clk_s * 10000); mpll_ss1 &= ~CLKV_MASK;
Function rv740_get_decoded_reference_divider() may return 0 due to unpredictable reference divider value calculated in radeon_atom_get_clock_dividers(). This will lead to division-by-zero error once that value is used as a divider in calculating 'clk_s'. While unlikely, this issue should nonetheless be prevented so add a sanity check for such cases by testing 'decoded_ref' value against 0. Found by Linux Verification Center (linuxtesting.org) with static analysis tool SVACE. Fixes: 66229b200598 ("drm/radeon/kms: add dpm support for rv7xx (v4)") Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> --- drivers/gpu/drm/radeon/cypress_dpm.c | 7 +++++-- drivers/gpu/drm/radeon/ni_dpm.c | 7 +++++-- drivers/gpu/drm/radeon/rv740_dpm.c | 7 +++++-- 3 files changed, 15 insertions(+), 6 deletions(-)