linux-kernel - Re: [PATCH] ACPI: PCC: Fix memory leak in address space setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 9 Sep 2022 10:49:43 +0100
From:   Sudeep Holla <sudeep.holla@....com>
To:     Rafael Mendonca <rafaelmendsr@...il.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Sudeep Holla <sudeep.holla@....com>,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ACPI: PCC: Fix memory leak in address space setup

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@...il.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")

-- 
Regards,
Sudeep