[scan-admin at coverity.com: New Defects reported by Coverity Scan for Das U-Boot]
Sean Anderson
seanga2 at gmail.com
Tue Jul 27 05:26:39 CEST 2021
On 7/26/21 10:52 PM, Tom Rini wrote:
> ----- Forwarded message from scan-admin at coverity.com -----
>
> Date: Tue, 27 Jul 2021 01:10:27 +0000 (UTC)
> From: scan-admin at coverity.com
> To: tom.rini at gmail.com
> Subject: New Defects reported by Coverity Scan for Das U-Boot
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
> 6 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 9 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 6 of 6 defect(s)
>
>
> ** CID 332931: Control flow issues (NO_EFFECT)
> /drivers/clk/clk_kendryte.c: 852 in k210_pll_set_rate()
>
>
> ________________________________________________________________________________________________________
> *** CID 332931: Control flow issues (NO_EFFECT)
> /drivers/clk/clk_kendryte.c: 852 in k210_pll_set_rate()
> 846 int err;
> 847 const struct k210_pll_params *pll = &k210_plls[id];
> 848 struct k210_pll_config config = {};
> 849 u32 reg;
> 850 ulong calc_rate;
> 851
>>>> CID 332931: Control flow issues (NO_EFFECT)
>>>> This less-than-zero comparison of an unsigned value is never true. "rate_in < 0UL".
> 852 if (rate_in < 0)
> 853 return rate_in;
> 854
> 855 err = k210_pll_calc_config(rate, rate_in, &config);
> 856 if (err)
> 857 return err;
>
> ** CID 332929: Integer handling issues (NO_EFFECT)
> /drivers/clk/clk_kendryte.c: 898 in k210_pll_get_rate()
>
>
> ________________________________________________________________________________________________________
> *** CID 332929: Integer handling issues (NO_EFFECT)
> /drivers/clk/clk_kendryte.c: 898 in k210_pll_get_rate()
> 892 static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id,
> 893 ulong rate_in)
> 894 {
> 895 u64 r, f, od;
> 896 u32 reg = readl(priv->base + k210_plls[id].off);
> 897
>>>> CID 332929: Integer handling issues (NO_EFFECT)
>>>> This less-than-zero comparison of an unsigned value is never true. "rate_in < 0UL".
> 898 if (rate_in < 0 || (reg & K210_PLL_BYPASS))
> 899 return rate_in;
> 900
> 901 if (!(reg & K210_PLL_PWRD))
> 902 return 0;
> 903
>
Will send a patch for these.
> ** CID 332927: (DIVIDE_BY_ZERO)
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
>
>
> ________________________________________________________________________________________________________
> *** CID 332927: (DIVIDE_BY_ZERO)
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In function call "__div64_32", division by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In expression "(u32)_tmp % __base", modulo by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In function call "__div64_32", division by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In function call "__div64_32", division by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In function call "__div64_32", division by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In function call "__div64_32", division by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In expression "(u32)_tmp % __base", modulo by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In expression "(u32)_tmp % __base", modulo by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In expression "(u32)_tmp % __base", modulo by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In expression "(u32)_tmp % __base", modulo by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
> /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config()
> 778 } else {
> 779 /*
> 780 * There is no way to only divide once; we need
> 781 * to examine the frequency with and without the
> 782 * effect of od.
> 783 */
>>>> CID 332927: (DIVIDE_BY_ZERO)
>>>> In expression "(u32)_tmp % __base", modulo by expression "__base" which may be zero has undefined behavior.
> 784 u64 vco = DIV_ROUND_CLOSEST_ULL(rate_in * f, r);
> 785
> 786 if (vco > 1750000000 || vco < 340000000)
> 787 out_of_spec = true;
> 788 }
> 789
These are completely safe, but it is relatively non-obvious why. The
only way that r can be 0 is on the very first iteration. When rate >
rate_in, r gets assigned (to a non-zero number) immediately. For the
converse, we only assign to r and od when r * od < goal. goal is
calculated by multiplying f (which is always at least 1) with inv_ratio,
shifted right by 32 bits. In the worst-case (the first iteration), this
is just inv_ratio >> 32. But inv_ratio is rate_in << 32 / rate, and
above we assumed that rate <= rate_in. So inv_ratio is always at least 1
<< 32, and we never divide by 0 :)
In the course of investigating the above, I added some additional test
cases and discovered that we don't always get the best factors in some
cases. I will also send a patch for this.
--Sean
More information about the U-Boot
mailing list