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

[Baolin Wang <baolin.wang@linux.alibaba.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,