linux-kernel - Re: [PATCH] spi: spi-sprd-adi: Fix double free in probe error path

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: <tencent_6FE7974351445022117CFBCC5E8D4D38AB07@qq.com>
Date: Fri, 9 Jan 2026 20:06:52 +0800
From: Felix Gu <gu_0233@...com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Mark Brown <broonie@...nel.org>, Orson Zhai <orsonzhai@...il.com>,
 Chunyan Zhang <zhang.lyra@...il.com>
Cc: linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: spi-sprd-adi: Fix double free in probe error path

Hi Baolin,

Thanks for review, I will fix it in V2.


Best regards,

Felix Gu

在 2026/1/9 8:41, Baolin Wang 写道:
>
>
> On 1/9/26 12:12 AM, Felix Gu wrote:
>> The driver currently uses spi_alloc_host() to allocate the controller
>> but registers it using devm_spi_register_controller().
>>
>> If devm_register_restart_handler() fails, the code jumps to the
>> put_ctlr label and calls spi_controller_put(). However, since the
>> controller was registered via a devm function, the device core will
>> automatically call spi_controller_put() again when the probe fails.
>> This results in a double-free of the spi_controller structure.
>>
>> Fix this by switching to devm_spi_alloc_host() and removing the
>> manual spi_controller_put() call.
>>
>> Signed-off-by: Felix Gu <gu_0233@...com>
>> ---
>
> Thanks. Can you add a 'Fixes:' tag? With that, you can add:
> Reviewed-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
>
>>   drivers/spi/spi-sprd-adi.c | 33 ++++++++++-----------------------
>>   1 file changed, 10 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
>> index 262c11d977ea..f25b34a91756 100644
>> --- a/drivers/spi/spi-sprd-adi.c
>> +++ b/drivers/spi/spi-sprd-adi.c
>> @@ -528,7 +528,7 @@ static int sprd_adi_probe(struct platform_device 
>> *pdev)
>>       pdev->id = of_alias_get_id(np, "spi");
>>       num_chipselect = of_get_child_count(np);
>>   -    ctlr = spi_alloc_host(&pdev->dev, sizeof(struct sprd_adi));
>> +    ctlr = devm_spi_alloc_host(&pdev->dev, sizeof(struct sprd_adi));
>>       if (!ctlr)
>>           return -ENOMEM;
>>   @@ -536,10 +536,8 @@ static int sprd_adi_probe(struct 
>> platform_device *pdev)
>>       sadi = spi_controller_get_devdata(ctlr);
>>         sadi->base = devm_platform_get_and_ioremap_resource(pdev, 0, 
>> &res);
>> -    if (IS_ERR(sadi->base)) {
>> -        ret = PTR_ERR(sadi->base);
>> -        goto put_ctlr;
>> -    }
>> +    if (IS_ERR(sadi->base))
>> +        return PTR_ERR(sadi->base);
>>         sadi->slave_vbase = (unsigned long)sadi->base +
>>                   data->slave_offset;
>> @@ -551,18 +549,15 @@ static int sprd_adi_probe(struct 
>> platform_device *pdev)
>>       if (ret > 0 || (IS_ENABLED(CONFIG_HWSPINLOCK) && ret == 0)) {
>>           sadi->hwlock =
>> devm_hwspin_lock_request_specific(&pdev->dev, ret);
>> -        if (!sadi->hwlock) {
>> -            ret = -ENXIO;
>> -            goto put_ctlr;
>> -        }
>> +        if (!sadi->hwlock)
>> +            return -ENXIO;
>>       } else {
>>           switch (ret) {
>>           case -ENOENT:
>>               dev_info(&pdev->dev, "no hardware spinlock supplied\n");
>>               break;
>>           default:
>> -            dev_err_probe(&pdev->dev, ret, "failed to find hwlock 
>> id\n");
>> -            goto put_ctlr;
>> +            return dev_err_probe(&pdev->dev, ret, "failed to find 
>> hwlock id\n");
>>           }
>>       }
>>   @@ -579,26 +574,18 @@ static int sprd_adi_probe(struct 
>> platform_device *pdev)
>>       ctlr->transfer_one = sprd_adi_transfer_one;
>>         ret = devm_spi_register_controller(&pdev->dev, ctlr);
>> -    if (ret) {
>> -        dev_err(&pdev->dev, "failed to register SPI controller\n");
>> -        goto put_ctlr;
>> -    }
>> +    if (ret)
>> +        return dev_err_probe(&pdev->dev, ret, "failed to register 
>> SPI controller\n");
>>         if (sadi->data->restart) {
>>           ret = devm_register_restart_handler(&pdev->dev,
>>                               sadi->data->restart,
>>                               sadi);
>> -        if (ret) {
>> -            dev_err(&pdev->dev, "can not register restart handler\n");
>> -            goto put_ctlr;
>> -        }
>> +        if (ret)
>> +            return dev_err_probe(&pdev->dev, ret, "can not register 
>> restart handler\n");
>>       }
>>         return 0;
>> -
>> -put_ctlr:
>> -    spi_controller_put(ctlr);
>> -    return ret;
>>   }
>>     static struct sprd_adi_data sc9860_data = {
>>
>> ---
>> base-commit: fc4e91c639c0af93d63c3d5bc0ee45515dd7504a
>> change-id: 20260108-spi-sprd-adi-fix-c071bf124bf6
>>
>> Best regards,