linux-kernel - Re: [PATCH v2] ata: libata: Fix memory leak for error path in ata_host

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]

Message-ID: <96b9bedb-891e-4db3-abfe-bf6ff18c6048@huaweicloud.com>
Date: Tue, 27 Aug 2024 09:46:58 +0800
From: Zheng Qixing <zhengqixing@...weicloud.com>
To: Damien Le Moal <dlemoal@...nel.org>, cassel@...nel.org
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
 yukuai3@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v2] ata: libata: Fix memory leak for error path in
 ata_host_alloc()


在 2024/8/26 12:02, Damien Le Moal 写道:
> On 8/26/24 11:49 AM, Zheng Qixing wrote:
>> 在 2024/8/23 9:10, Damien Le Moal 写道:
>>> On 8/22/24 12:30 PM, Zheng Qixing wrote:
>>>> From: Zheng Qixing <zhengqixing@...wei.com>
>>>>
>>>> In ata_host_alloc(), if ata_port_alloc(host) fails to allocate memory
>>>> for a port, the allocated 'host' structure is not freed before returning
>>>> from the function. This results in a potential memory leak.
>>>>
>>>> This patch adds a kfree(host) before the error handling code is executed
>>>> to ensure that the 'host' structure is properly freed in case of an
>>>> allocation failure.
>>>>
>>>> Signed-off-by: Zheng Qixing <zhengqixing@...wei.com>
>>> This needs a Fixes tag. So I added:
>>>
>>> Fixes: 2623c7a5f279 ("libata: add refcounting to ata_host")
>>> Cc: stable@...r.kernel.org>
>>>
>>> and applied to for-6.11-fixes. Thanks.
>>
>> Based on Niklas Cassel's suggestion, the commit message and the actual
>> content of the patch do not match.
>>
>> It should state "if devres_alloc(ata_devres_release, 0, GFP_KERNEL)
>> fails to allocate memory" instead of "if ata_port_alloc(host) fails to allocate
>> memory for a port".
>>
>> Should I modify the commit message and submit a new version of the patch?
> No need. I fixed it up. The commit message now is:
>
>      ata: libata: Fix memory leak for error path in ata_host_alloc()
>
>      In ata_host_alloc(), if devres_alloc() fails to allocate the device host
>      resource data pointer, the already allocated ata_host structure is not
>      freed before returning from the function. This results in a potential
>      memory leak.
>
>      Call kfree(host) before jumping to the error handling path to ensure
>      that the ata_host structure is properly freed if devres_alloc() fails.


Thanks.  :)


Zheng Qixing

>
>>
>> Zheng Qixing
>>
>>
>>>> ---
>>>> Changes in v2:
>>>>    - error path is wrong in v1
>>>>
>>>>    drivers/ata/libata-core.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>>>> index e4023fc288ac..f27a18990c38 100644
>>>> --- a/drivers/ata/libata-core.c
>>>> +++ b/drivers/ata/libata-core.c
>>>> @@ -5663,8 +5663,10 @@ struct ata_host *ata_host_alloc(struct device *dev,
>>>> int n_ports)
>>>>        }
>>>>          dr = devres_alloc(ata_devres_release, 0, GFP_KERNEL);
>>>> -    if (!dr)
>>>> +    if (!dr) {
>>>> +        kfree(host);
>>>>            goto err_out;
>>>> +    }
>>>>          devres_add(dev, dr);
>>>>        dev_set_drvdata(dev, host);
>>