[PATCH] ACPI: acpi_pcc.c: Fix unintentional integer overflow

[PATCH] ACPI: acpi_pcc.c: Fix unintentional integer overflow

[pmanank200502]

From: Manank Patel <pmanank200502@gmail.com>

Fixed unintentional u32 overflow by casting it to u64 before multiplication.

Signed-off-by: Manank Patel <pmanank200502@gmail.com>
---
 drivers/acpi/acpi_pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index ee4ce5ba1fb2..b929d2e5c622 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -112,7 +112,7 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
 		 * processor could be much slower to reply. So add an arbitrary
 		 * amount of wait on top of Nominal.
 		 */
-		usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * data->pcc_chan->latency;
+		usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * ((u64) data->pcc_chan->latency);
 		ret = wait_for_completion_timeout(&data->done,
 						  usecs_to_jiffies(usecs_lat));
 		if (ret == 0) {
-- 
2.38.0

Re: [PATCH] ACPI: acpi_pcc.c: Fix unintentional integer overflow

[Rafael J. Wysocki]

On Thu, Oct 13, 2022 at 7:50 AM <pmanank200502@gmail.com> wrote:
>
> From: Manank Patel <pmanank200502@gmail.com>
>
> Fixed unintentional u32 overflow by casting it to u64 before multiplication.
>
> Signed-off-by: Manank Patel <pmanank200502@gmail.com>

A Fixes tag would be nice to have here.

> ---
>  drivers/acpi/acpi_pcc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
> index ee4ce5ba1fb2..b929d2e5c622 100644
> --- a/drivers/acpi/acpi_pcc.c
> +++ b/drivers/acpi/acpi_pcc.c
> @@ -112,7 +112,7 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
>                  * processor could be much slower to reply. So add an arbitrary
>                  * amount of wait on top of Nominal.
>                  */
> -               usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * data->pcc_chan->latency;
> +               usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * ((u64) data->pcc_chan->latency);

Or redefine PCC_CMD_WAIT_RETRIES_NUM as 500ULL?

>                 ret = wait_for_completion_timeout(&data->done,
>                                                   usecs_to_jiffies(usecs_lat));
>                 if (ret == 0) {
> --
> 2.38.0
>

Re: [PATCH] Fixes: 91cefefb6991 ("ACPI: acpi_pcc.c: fixed unintentional u32 overflow by redefining a constant")

[Manank Patel]

changed PCC_CMD_WAIT_RETRIES_NUM to 500ULL


Signed-off-by: Manank Patel <pmanank200502@gmail.com>
---
 drivers/acpi/acpi_pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index ee4ce5ba1fb2..3e252be047b8 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -27,7 +27,7 @@
  * Arbitrary retries in case the remote processor is slow to respond
  * to PCC commands
  */
-#define PCC_CMD_WAIT_RETRIES_NUM	500
+#define PCC_CMD_WAIT_RETRIES_NUM	500ULL
 
 struct pcc_data {
 	struct pcc_mbox_chan *pcc_chan;
-- 
2.38.0

Re: [PATCH] Fixes: 91cefefb6991 ("ACPI: PCC: replace wait_for_completion()")

[Manank Patel]

Fixed unintentional u32 overflow by changing PCC_CMD_WAIT_RETRIES_NUM to 500ULL

Signed-off-by: Manank Patel <pmanank200502@gmail.com>
---

Sorry for the spam, I made a mistake in the previous patch (I had a confusion 
in your suggestion about the Fixes tag).As you would have realised, i'm new 
to this, and not so familiar with the workflow (Though I have read the 
Documentation). Let me know if you have any suggestions.

 drivers/acpi/acpi_pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index ee4ce5ba1fb2..3e252be047b8 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -27,7 +27,7 @@
  * Arbitrary retries in case the remote processor is slow to respond
  * to PCC commands
  */
-#define PCC_CMD_WAIT_RETRIES_NUM	500
+#define PCC_CMD_WAIT_RETRIES_NUM	500ULL
 
 struct pcc_data {
 	struct pcc_mbox_chan *pcc_chan;
-- 
2.38.0

Re: [PATCH] Fixes: 91cefefb6991 ("ACPI: PCC: replace wait_for_completion()")

[Sudeep Holla]

On Mon, Oct 17, 2022 at 11:19:15PM +0530, Manank Patel wrote:
> Fixed unintentional u32 overflow by changing PCC_CMD_WAIT_RETRIES_NUM to 500ULL
> 

You just need to add the below fixes line above Signed-off tag. You seemed
to have changed the $subject. Please retain the origin subject

Please post it again cleanly marking it as v2(as in the subject must be
[PATCH v2] "ACPI: PCC: Fix unintentional integer overflow" 

Fixes: 91cefefb6991 ("ACPI: PCC: replace wait_for_completion()")
> Signed-off-by: Manank Patel <pmanank200502@gmail.com>
> ---
> 
> Sorry for the spam, I made a mistake in the previous patch (I had a confusion 
> in your suggestion about the Fixes tag).As you would have realised, i'm new 
> to this, and not so familiar with the workflow (Though I have read the 
> Documentation). Let me know if you have any suggestions.
>

Add a text here pointing what changed from v1->v2
v1->v2:
	Change the macro itself ULL instead typecasting in the code

Hope this helps.

With all these changes incorporated in v2, you can add my
Acked-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

[PATCH v2] "ACPI: PCC: Fix unintentional integer overflow"

[Manank Patel]

Fixed unintentional u32 overflow by changing PCC_CMD_WAIT_RETRIES_NUM to 500ULL

Fixes: 91cefefb6991 ("ACPI: PCC: replace wait_for_completion()")

Signed-off-by: Manank Patel <pmanank200502@gmail.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>

---
Thank you so much @sudeep for your clarifications!

Changelog:
v1->v2:
        Change the macro itself to ULL instead of typecasting in the
        code

 drivers/acpi/acpi_pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index ee4ce5ba1fb2..3e252be047b8 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -27,7 +27,7 @@
  * Arbitrary retries in case the remote processor is slow to respond
  * to PCC commands
  */
-#define PCC_CMD_WAIT_RETRIES_NUM	500
+#define PCC_CMD_WAIT_RETRIES_NUM	500ULL
 
 struct pcc_data {
 	struct pcc_mbox_chan *pcc_chan;
-- 
2.38.0

Re: [PATCH v2] "ACPI: PCC: Fix unintentional integer overflow"

[lihuisong (C)]

在 2022/10/18 2:43, Manank Patel 写道:
> Fixed unintentional u32 overflow by changing PCC_CMD_WAIT_RETRIES_NUM to 500ULL
>
> Fixes: 91cefefb6991 ("ACPI: PCC: replace wait_for_completion()")
>
> Signed-off-by: Manank Patel <pmanank200502@gmail.com>
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
>
> ---
> Thank you so much @sudeep for your clarifications!
>
> Changelog:
> v1->v2:
>          Change the macro itself to ULL instead of typecasting in the
>          code
>
>   drivers/acpi/acpi_pcc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
> index ee4ce5ba1fb2..3e252be047b8 100644
> --- a/drivers/acpi/acpi_pcc.c
> +++ b/drivers/acpi/acpi_pcc.c
> @@ -27,7 +27,7 @@
>    * Arbitrary retries in case the remote processor is slow to respond
>    * to PCC commands
>    */
> -#define PCC_CMD_WAIT_RETRIES_NUM	500
> +#define PCC_CMD_WAIT_RETRIES_NUM	500ULL
>   
>   struct pcc_data {
>   	struct pcc_mbox_chan *pcc_chan;

Acked-by: Huisong Li <lihuisong@huawei.com>

Re: [PATCH v2] "ACPI: PCC: Fix unintentional integer overflow"

[Rafael J. Wysocki]

On Tue, Oct 18, 2022 at 3:13 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2022/10/18 2:43, Manank Patel 写道:
> > Fixed unintentional u32 overflow by changing PCC_CMD_WAIT_RETRIES_NUM to 500ULL
> >
> > Fixes: 91cefefb6991 ("ACPI: PCC: replace wait_for_completion()")
> >
> > Signed-off-by: Manank Patel <pmanank200502@gmail.com>
> > Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> > ---
> > Thank you so much @sudeep for your clarifications!
> >
> > Changelog:
> > v1->v2:
> >          Change the macro itself to ULL instead of typecasting in the
> >          code
> >
> >   drivers/acpi/acpi_pcc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
> > index ee4ce5ba1fb2..3e252be047b8 100644
> > --- a/drivers/acpi/acpi_pcc.c
> > +++ b/drivers/acpi/acpi_pcc.c
> > @@ -27,7 +27,7 @@
> >    * Arbitrary retries in case the remote processor is slow to respond
> >    * to PCC commands
> >    */
> > -#define PCC_CMD_WAIT_RETRIES_NUM     500
> > +#define PCC_CMD_WAIT_RETRIES_NUM     500ULL
> >
> >   struct pcc_data {
> >       struct pcc_mbox_chan *pcc_chan;
>
> Acked-by: Huisong Li <lihuisong@huawei.com>

Applied as 6.1-rc material, thanks!