2022-10-13 06:37:47

by Manank Patel

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

From: Manank Patel <[email protected]>

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

Signed-off-by: Manank Patel <[email protected]>
---
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


2022-10-13 12:14:13

by Rafael J. Wysocki

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

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

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
>

2022-10-13 13:19:29

by Manank Patel

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

changed PCC_CMD_WAIT_RETRIES_NUM to 500ULL


Signed-off-by: Manank Patel <[email protected]>
---
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

2022-10-17 18:12:43

by Manank Patel

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

Fixed unintentional u32 overflow by changing PCC_CMD_WAIT_RETRIES_NUM to 500ULL

Signed-off-by: Manank Patel <[email protected]>
---

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

2022-10-17 18:32:25

by Sudeep Holla

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

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 <[email protected]>
> ---
>
> 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 <[email protected]>

--
Regards,
Sudeep

2022-10-17 19:09:53

by Manank Patel

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

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 <[email protected]>
Acked-by: Sudeep Holla <[email protected]>

---
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

2022-10-26 11:41:13

by Rafael J. Wysocki

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

On Tue, Oct 18, 2022 at 3:13 AM lihuisong (C) <[email protected]> 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 <[email protected]>
> > Acked-by: Sudeep Holla <[email protected]>
> >
> > ---
> > 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 <[email protected]>

Applied as 6.1-rc material, thanks!