linux-kernel - Re: [PATCH] PM / Domains: Fix integer overflows on u32 bit multiplies

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210208075442.GA13982@amd>
Date:   Mon, 8 Feb 2021 08:54:43 +0100
From:   Pavel Machek <pavel@....cz>
To:     Colin King <colin.king@...onical.com>
Cc:     "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Marc Titinger <mtitinger+renesas@...libre.com>,
        Lina Iyer <lina.iyer@...aro.org>, linux-pm@...r.kernel.org,
        kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PM / Domains: Fix integer overflows on u32 bit multiplies

On Sun 2021-02-07 22:46:48, Colin King wrote:
> From: Colin Ian King <colin.king@...onical.com>
> 
> There are three occurrances of u32 variables being multiplied by
> 1000 using 32 bit multiplies and the result being assigned to a
> 64 bit signed integer.  These can potentially lead to a 32 bit
> overflows, so fix this by casting 1000 to a UL first to force
> a 64 bit multiply hence avoiding the overflow.

Ummm. No?

a) Can you imagine any situation where they result in overflow?

b) How does casting to UL help on 32 bit system?

Best regards,

								Pavel

> Addresses-Coverity: ("Unintentional integer overflow")
> Fixes: 30f604283e05 ("PM / Domains: Allow domain power states to be read from DT")
> Signed-off-by: Colin Ian King <colin.king@...onical.com>
> ---
>  drivers/base/power/domain.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index aaf6c83b5cf6..ddeff69126ff 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2831,10 +2831,10 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
>  
>  	err = of_property_read_u32(state_node, "min-residency-us", &residency);
>  	if (!err)
> -		genpd_state->residency_ns = 1000 * residency;
> +		genpd_state->residency_ns = 1000UL * residency;
>  
> -	genpd_state->power_on_latency_ns = 1000 * exit_latency;
> -	genpd_state->power_off_latency_ns = 1000 * entry_latency;
> +	genpd_state->power_on_latency_ns = 1000UL * exit_latency;
> +	genpd_state->power_off_latency_ns = 1000UL * entry_latency;
>  	genpd_state->fwnode = &state_node->fwnode;
>  
>  	return 0;

-- 
http://www.livejournal.com/~pavelmachek

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)