| Message ID | 20220909021348.472674-1-rafaelmendsr@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: PCC: Fix memory leak in address space setup | expand |
On Thu, Sep 08, 2022 at 11:13:47PM -0300, Rafael Mendonca wrote: > The allocated memory for the pcc_data struct doesn't get freed under an > error path in pcc_mbox_request_channel() or acpi_os_ioremap(). > > Fixes: 77e2a04745ff8 ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype") > Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com> > --- > drivers/acpi/acpi_pcc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c > index a12b55d81209..fe5ab0fdc3bf 100644 > --- a/drivers/acpi/acpi_pcc.c > +++ b/drivers/acpi/acpi_pcc.c > @@ -63,6 +63,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function, > if (IS_ERR(data->pcc_chan)) { > pr_err("Failed to find PCC channel for subspace %d\n", > ctx->subspace_id); > + kfree(data); > return AE_NOT_FOUND; > } > > @@ -72,6 +73,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function, > if (!data->pcc_comm_addr) { > pr_err("Failed to ioremap PCC comm region mem for %d\n", > ctx->subspace_id); I was wondering if pcc_mbox_free_channel() should be called here as well in case of acpi_os_ioremap() failure. > + kfree(data); > return AE_NO_MEMORY; > } > > -- > 2.34.1 >
On Thu, Sep 08, 2022 at 11:34:12PM -0300, Rafael Mendonca wrote: > On Thu, Sep 08, 2022 at 11:13:47PM -0300, Rafael Mendonca wrote: > > The allocated memory for the pcc_data struct doesn't get freed under an > > error path in pcc_mbox_request_channel() or acpi_os_ioremap(). > > > > Fixes: 77e2a04745ff8 ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype") > > Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com> > > --- > > drivers/acpi/acpi_pcc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c > > index a12b55d81209..fe5ab0fdc3bf 100644 > > --- a/drivers/acpi/acpi_pcc.c > > +++ b/drivers/acpi/acpi_pcc.c > > @@ -63,6 +63,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function, > > if (IS_ERR(data->pcc_chan)) { > > pr_err("Failed to find PCC channel for subspace %d\n", > > ctx->subspace_id); > > + kfree(data); > > return AE_NOT_FOUND; > > } > > > > @@ -72,6 +73,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function, > > if (!data->pcc_comm_addr) { > > pr_err("Failed to ioremap PCC comm region mem for %d\n", > > ctx->subspace_id); > > I was wondering if pcc_mbox_free_channel() should be called here as well > in case of acpi_os_ioremap() failure. > Yes please. There are not modules and shouldn't matter much but it is good to have it for correctness. Thanks for finding and fixing this. Also please add the fixes tag in next version. Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c index a12b55d81209..fe5ab0fdc3bf 100644 --- a/drivers/acpi/acpi_pcc.c +++ b/drivers/acpi/acpi_pcc.c @@ -63,6 +63,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function, if (IS_ERR(data->pcc_chan)) { pr_err("Failed to find PCC channel for subspace %d\n", ctx->subspace_id); + kfree(data); return AE_NOT_FOUND; } @@ -72,6 +73,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function, if (!data->pcc_comm_addr) { pr_err("Failed to ioremap PCC comm region mem for %d\n", ctx->subspace_id); + kfree(data); return AE_NO_MEMORY; }
The allocated memory for the pcc_data struct doesn't get freed under an error path in pcc_mbox_request_channel() or acpi_os_ioremap(). Fixes: 77e2a04745ff8 ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype") Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com> --- drivers/acpi/acpi_pcc.c | 2 ++ 1 file changed, 2 insertions(+)