drivers/spi/spi-sprd-adi.c | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-)
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@qq.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,
--
Felix Gu <gu_0233@qq.com>
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@qq.com>
> ---
Thanks. Can you add a 'Fixes:' tag? With that, you can add:
Reviewed-by: Baolin Wang <baolin.wang@linux.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,
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@qq.com>
>> ---
>
> Thanks. Can you add a 'Fixes:' tag? With that, you can add:
> Reviewed-by: Baolin Wang <baolin.wang@linux.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,
© 2016 - 2026 Red Hat, Inc.