linux-kernel - Re: [PATCH] memory: samsung: exynos5422-dmc: Do not ignore return code of regmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1f9f9bf2-d93e-5d8b-adcd-c63939b8cf8a@arm.com>
Date:   Mon, 27 Jul 2020 09:35:35 +0100
From:   Lukasz Luba <lukasz.luba@....com>
To:     Krzysztof Kozlowski <krzk@...nel.org>,
        Kukjin Kim <kgene@...nel.org>, linux-pm@...r.kernel.org,
        linux-samsung-soc@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] memory: samsung: exynos5422-dmc: Do not ignore return
 code of regmap_read()

Hi Krzysztof,

On 7/20/20 12:03 PM, Krzysztof Kozlowski wrote:
> Check for regmap_read() return code before using the read value in
> following write in exynos5_switch_timing_regs().  Pass reading error
> code to the callers.
> 
> This does not introduce proper error handling for such failed reads (and
> obviously regmap_write() error is still ignored) because the driver
> ignored this in all places.  Therefor it only fixes reported issue while
> matching current driver coding style:
> 
>         drivers/memory/samsung/exynos5422-dmc.c: In function 'exynos5_switch_timing_regs':
>      >> drivers/memory/samsung/exynos5422-dmc.c:216:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
> 
> Reported-by: kernel test robot <lkp@...el.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@...nel.org>
> ---
>   drivers/memory/samsung/exynos5422-dmc.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
> index 93e9c2429c0d..2c326998cb1c 100644
> --- a/drivers/memory/samsung/exynos5422-dmc.c
> +++ b/drivers/memory/samsung/exynos5422-dmc.c
> @@ -270,12 +270,14 @@ static int find_target_freq_idx(struct exynos5_dmc *dmc,
>    * This function switches between these banks according to the
>    * currently used clock source.
>    */
> -static void exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set)
> +static int exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set)
>   {
>   	unsigned int reg;
>   	int ret;
>   
>   	ret = regmap_read(dmc->clk_regmap, CDREX_LPDDR3PHY_CON3, &reg);
> +	if (ret)
> +		return ret;
>   
>   	if (set)
>   		reg |= EXYNOS5_TIMING_SET_SWI;
> @@ -283,6 +285,8 @@ static void exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set)
>   		reg &= ~EXYNOS5_TIMING_SET_SWI;
>   
>   	regmap_write(dmc->clk_regmap, CDREX_LPDDR3PHY_CON3, reg);
> +
> +	return 0;
>   }
>   
>   /**
> @@ -516,7 +520,7 @@ exynos5_dmc_switch_to_bypass_configuration(struct exynos5_dmc *dmc,
>   	/*
>   	 * Delays are long enough, so use them for the new coming clock.
>   	 */
> -	exynos5_switch_timing_regs(dmc, USE_MX_MSPLL_TIMINGS);
> +	ret = exynos5_switch_timing_regs(dmc, USE_MX_MSPLL_TIMINGS);
>   
>   	return ret;
>   }
> @@ -577,7 +581,9 @@ exynos5_dmc_change_freq_and_volt(struct exynos5_dmc *dmc,
>   
>   	clk_set_rate(dmc->fout_bpll, target_rate);
>   
> -	exynos5_switch_timing_regs(dmc, USE_BPLL_TIMINGS);
> +	ret = exynos5_switch_timing_regs(dmc, USE_BPLL_TIMINGS);
> +	if (ret)
> +		goto disable_clocks;
>   
>   	ret = clk_set_parent(dmc->mout_mclk_cdrex, dmc->mout_bpll);
>   	if (ret)
> 

LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@....com>

Regards,
Lukasz