2022-03-25 18:30:18

by Davidlohr Bueso

Subject: [PATCH 0/3] thermal: int340x: Misc acpi_buffer handling updates

Hello,

The following is a fallout of eyeballing _OSC handling in the driver.
These changes have only been compile-tested. Patch 1 is a fix and the
other two are cleanups.

Thanks!

Davidlohr Bueso (3):
thermal: int340x: Fix bogus acpi_buffer pointer freeing
thermal: int340x: Consolidate freeing of acpi_buffer pointer
thermal: int340x: Cleanup osc context init

.../intel/int340x_thermal/int3400_thermal.c | 24 +++++++------------
1 file changed, 9 insertions(+), 15 deletions(-)

--
2.26.2

2022-03-25 18:42:14

by Zhang, Rui

Subject: Re: [PATCH 0/3] thermal: int340x: Misc acpi_buffer handling updates

Acked-by: Zhang Rui <[email protected]>

for the whole patch series.

thanks,
rui

On Thu, 2022-03-24 at 12:09 -0700, Davidlohr Bueso wrote:
> Hello,
>
> The following is a fallout of eyeballing _OSC handling in the driver.
> These changes have only been compile-tested. Patch 1 is a fix and the
> other two are cleanups.
>
> Thanks!
>
> Davidlohr Bueso (3):
> thermal: int340x: Fix bogus acpi_buffer pointer freeing
> thermal: int340x: Consolidate freeing of acpi_buffer pointer
> thermal: int340x: Cleanup osc context init
>
> .../intel/int340x_thermal/int3400_thermal.c | 24 +++++++--------
> ----
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> --
> 2.26.2
>

2022-03-25 19:06:13

by Davidlohr Bueso

Subject: [PATCH 1/3] thermal: int340x: Fix bogus acpi_buffer pointer freeing

It is the caller's responsibility to free only upon ACPI_SUCCESS.

Signed-off-by: Davidlohr Bueso <[email protected]>
---
drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 4954800b9850..0e7931c286ec 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -186,11 +186,11 @@ static int int3400_thermal_run_osc(acpi_handle handle, char *uuid_str, int *enab
ret = *((u32 *)(context.ret.pointer + 4));
if (ret != *enable)
result = -EPERM;
+
+ kfree(context.ret.pointer);
} else
result = -EPERM;

- kfree(context.ret.pointer);
-
return result;
}

--
2.26.2

2022-03-25 19:09:15

by srinivas pandruvada

Subject: Re: [PATCH 1/3] thermal: int340x: Fix bogus acpi_buffer pointer freeing

On Thu, 2022-03-24 at 12:09 -0700, Davidlohr Bueso wrote:
> It is the caller's responsibility to free only upon ACPI_SUCCESS.
>

Since context->ret.pointer will be NULL on failure so calling kfree
will just return.

Although we can avoid this call to kfree.

Thanks,
Srinivas

> Signed-off-by: Davidlohr Bueso <[email protected]>
> ---
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 4954800b9850..0e7931c286ec 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -186,11 +186,11 @@ static int int3400_thermal_run_osc(acpi_handle
> handle, char *uuid_str, int *enab
>                 ret = *((u32 *)(context.ret.pointer + 4));
>                 if (ret != *enable)
>                         result = -EPERM;
> +
> +               kfree(context.ret.pointer);
>         } else
>                 result = -EPERM;
>  
> -       kfree(context.ret.pointer);
> -
>         return result;
>  }
>  

2022-04-06 05:41:21

by Rafael J. Wysocki

Subject: Re: [PATCH 1/3] thermal: int340x: Fix bogus acpi_buffer pointer freeing

On Thu, Mar 24, 2022 at 8:10 PM Davidlohr Bueso <[email protected]> wrote:
>
> It is the caller's responsibility to free only upon ACPI_SUCCESS.
>
> Signed-off-by: Davidlohr Bueso <[email protected]>
> ---
> drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 4954800b9850..0e7931c286ec 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -186,11 +186,11 @@ static int int3400_thermal_run_osc(acpi_handle handle, char *uuid_str, int *enab
> ret = *((u32 *)(context.ret.pointer + 4));
> if (ret != *enable)
> result = -EPERM;
> +
> + kfree(context.ret.pointer);
> } else
> result = -EPERM;
>
> - kfree(context.ret.pointer);
> -
> return result;
> }
>
> --

Because the code before the change is not incorrect, this is a cleanup
rather than a fix, so I've adjusted the subject a bit and applied this
along with the [2-3/3] as 5.19 material.

Thanks!