All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Colin King <colin.king@canonical.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Len Brown <len.brown@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Marc Titinger <mtitinger+renesas@baylibre.com>,
	Lina Iyer <lina.iyer@linaro.org>,
	linux-pm@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PM / Domains: Fix integer overflows on u32 bit multiplies
Date: Mon, 8 Feb 2021 08:54:43 +0100	[thread overview]
Message-ID: <20210208075442.GA13982@amd> (raw)
In-Reply-To: <20210207224648.8137-1-colin.king@canonical.com>

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On Sun 2021-02-07 22:46:48, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.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@canonical.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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Pavel Machek <pavel@ucw.cz>
To: Colin King <colin.king@canonical.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Len Brown <len.brown@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Marc Titinger <mtitinger+renesas@baylibre.com>,
	Lina Iyer <lina.iyer@linaro.org>,
	linux-pm@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PM / Domains: Fix integer overflows on u32 bit multiplies
Date: Mon, 08 Feb 2021 07:54:43 +0000	[thread overview]
Message-ID: <20210208075442.GA13982@amd> (raw)
In-Reply-To: <20210207224648.8137-1-colin.king@canonical.com>

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On Sun 2021-02-07 22:46:48, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.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@canonical.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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  reply	other threads:[~2021-02-08  7:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-07 22:46 [PATCH] PM / Domains: Fix integer overflows on u32 bit multiplies Colin King
2021-02-07 22:46 ` Colin King
2021-02-08  7:54 ` Pavel Machek [this message]
2021-02-08  7:54   ` Pavel Machek
2021-03-18 18:57 ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210208075442.GA13982@amd \
    --to=pavel@ucw.cz \
    --cc=colin.king@canonical.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=khilman@kernel.org \
    --cc=len.brown@intel.com \
    --cc=lina.iyer@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mtitinger+renesas@baylibre.com \
    --cc=rjw@rjwysocki.net \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.