From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A819AC4338F for ; Tue, 27 Jul 2021 03:26:51 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 98DC360F51 for ; Tue, 27 Jul 2021 03:26:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 98DC360F51 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 051D98335E; Tue, 27 Jul 2021 05:26:48 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Jvtv4/BL"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 21B1A8335E; Tue, 27 Jul 2021 05:26:46 +0200 (CEST) Received: from mail-qk1-x732.google.com (mail-qk1-x732.google.com [IPv6:2607:f8b0:4864:20::732]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id A49CC83358 for ; Tue, 27 Jul 2021 05:26:41 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qk1-x732.google.com with SMTP id c9so7292590qkc.13 for ; Mon, 26 Jul 2021 20:26:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:subject:to:references:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=8ng6yQitURcsfOKojA5o9ZakOCDA9k+CddMDPmZ0lD0=; b=Jvtv4/BL+hWR8kUpe3R8HmmA52ZS2H+ZVwXMO8YDEBMNGbnvf4QAz0V28+BWarGd3S ybHvWnoDJppxX47C4J2j3cVsI0Z4TJdDY7HklrjJA/JBnVDF7OU9GPFNAO0DmTy8/+AV /IZZoIcGpmlxUtblkmhZCm5BSvcCMr9qv5w50U6+L9PA6o0CuAbN0FtZSCaTgyhn5vT1 OFGXuygVZp4FhQqwGInEH1TJpJD0cCtepD8dYOqhs0NUleB0+/OgLqitJcB3o3zoMDoO 5kEGDgad75l4bdICCAWm3kWfza81KBneK5uVqUav+xQbr4QhcYGL6GjwL1XWdysgjU1T kC0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=8ng6yQitURcsfOKojA5o9ZakOCDA9k+CddMDPmZ0lD0=; b=OnokOdU3qxVDO039BV1kXVt59SEAq6aqtjPlNHRCsgXxzCxoEFWZCmJ0Upx8EmyNb9 L1z2HoWKUq04Mh02bnFEsyx1SChSeEjGK83O9a3p7Ynh1tT1I1uo910XWER+/IXyLNKa 5BNFDXfrCk9ByREgxmc3oWpW0uKbqV+h2+qu2aVjP3zdHwK30ZELOJ6bCoq8DyGrAnLi WZnwAKyvYmhlRVKDeX+gVKnSYdPQukLcaizQ9IFELEHl16yYpbyDmr/x/es/0COxFG2w 37sFk8mSKTUcTm1+4wC/9RgRh7HMY54n4v0c6rVUKFA/LbJVDkcQGeubXFgPhS7SXjOx r+Ag== X-Gm-Message-State: AOAM5337pTRgWSsuqNuD13DgJnT21vVWOHzgb5UxEVBqhYwpcxcLMAq8 Y91qCNdDbL2zg5VueNVqR9c= X-Google-Smtp-Source: ABdhPJxgGbpdx4lmdu6T/n8CKXANThOG6C8OnzLtT0lZfnDc2Kjxw11TbKd0BVho1gvg9fQigid+HA== X-Received: by 2002:a05:620a:62c:: with SMTP id 12mr20347253qkv.159.1627356400438; Mon, 26 Jul 2021 20:26:40 -0700 (PDT) Received: from [192.168.1.201] (pool-74-96-87-9.washdc.fios.verizon.net. [74.96.87.9]) by smtp.googlemail.com with ESMTPSA id y1sm1067000qki.59.2021.07.26.20.26.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 26 Jul 2021 20:26:40 -0700 (PDT) From: Sean Anderson Subject: Re: [scan-admin@coverity.com: New Defects reported by Coverity Scan for Das U-Boot] To: Tom Rini , u-boot@lists.denx.de, Simon Glass References: <20210727025210.GE9379@bill-the-cat> Message-ID: Date: Mon, 26 Jul 2021 23:26:39 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20210727025210.GE9379@bill-the-cat> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 7/26/21 10:52 PM, Tom Rini wrote: > ----- Forwarded message from scan-admin@coverity.com ----- > > Date: Tue, 27 Jul 2021 01:10:27 +0000 (UTC) > From: scan-admin@coverity.com > To: tom.rini@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