[v1] drm/bochs: Drop manual put on probe error path

[PATCH] drm/bochs: Drop manual put on probe error path

Posted by Myeonghun Pak 1 month, 3 weeks ago

bochs_pci_probe() allocates the DRM device with devm_drm_dev_alloc(),
which registers a devres action to drop the initial DRM device reference
on driver detach or probe failure.

The error path currently calls drm_dev_put() manually. If probe then
returns an error, devres will run the registered release action and put
the same device again, after the first put may already have released it.

Return the probe error directly and let devres own the final put.

Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/gpu/drm/tiny/bochs.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 222e4ae1ab..5d8dc5efec 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -761,25 +761,21 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
 
 	ret = pcim_enable_device(pdev);
 	if (ret)
-		goto err_free_dev;
+		return ret;
 
 	pci_set_drvdata(pdev, dev);
 
 	ret = bochs_load(bochs);
 	if (ret)
-		goto err_free_dev;
+		return ret;
 
 	ret = drm_dev_register(dev, 0);
 	if (ret)
-		goto err_free_dev;
+		return ret;
 
 	drm_client_setup(dev, NULL);
 
 	return ret;
-
-err_free_dev:
-	drm_dev_put(dev);
-	return ret;
 }
 
 static void bochs_pci_remove(struct pci_dev *pdev)
-- 
2.39.5

Re: [PATCH] drm/bochs: Drop manual put on probe error path

Posted by Thomas Zimmermann 1 month, 1 week ago


Am 24.04.26 um 14:34 schrieb Myeonghun Pak:
> bochs_pci_probe() allocates the DRM device with devm_drm_dev_alloc(),
> which registers a devres action to drop the initial DRM device reference
> on driver detach or probe failure.
>
> The error path currently calls drm_dev_put() manually. If probe then
> returns an error, devres will run the registered release action and put
> the same device again, after the first put may already have released it.
>
> Return the probe error directly and let devres own the final put.
>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

Fixes: 04826f588682 ("drm/bochs: Allocate DRM device in struct 
bochs_device")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux.dev
Cc: <stable@vger.kernel.org> # v6.13+


> ---
>   drivers/gpu/drm/tiny/bochs.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 222e4ae1ab..5d8dc5efec 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -761,25 +761,21 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
>   
>   	ret = pcim_enable_device(pdev);
>   	if (ret)
> -		goto err_free_dev;
> +		return ret;
>   
>   	pci_set_drvdata(pdev, dev);
>   
>   	ret = bochs_load(bochs);
>   	if (ret)
> -		goto err_free_dev;
> +		return ret;
>   
>   	ret = drm_dev_register(dev, 0);
>   	if (ret)
> -		goto err_free_dev;
> +		return ret;
>   
>   	drm_client_setup(dev, NULL);
>   
>   	return ret;
> -
> -err_free_dev:
> -	drm_dev_put(dev);
> -	return ret;
>   }
>   
>   static void bochs_pci_remove(struct pci_dev *pdev)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: drm/bochs: Drop manual put on probe error path

Posted by Markus Elfring 1 month, 1 week ago

…
> > Return the probe error directly and let devres own the final put.
…
> Fixes: 04826f588682 ("drm/bochs: Allocate DRM device in struct 
> bochs_device")
…

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.1-rc2#n145

Regards,
Markus

Re: [PATCH] drm/bochs: Drop manual put on probe error path

Posted by Thomas Zimmermann 1 month, 3 weeks ago

Hi,

please add fixes tags to all these patches you're sending. You also need 
to CC stable so that they can be backported easily. Also list the AI 
you're using to find and create these patches.

Best regards
Thomas

Am 24.04.26 um 14:34 schrieb Myeonghun Pak:
> bochs_pci_probe() allocates the DRM device with devm_drm_dev_alloc(),
> which registers a devres action to drop the initial DRM device reference
> on driver detach or probe failure.
>
> The error path currently calls drm_dev_put() manually. If probe then
> returns an error, devres will run the registered release action and put
> the same device again, after the first put may already have released it.
>
> Return the probe error directly and let devres own the final put.
>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>   drivers/gpu/drm/tiny/bochs.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 222e4ae1ab..5d8dc5efec 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -761,25 +761,21 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
>   
>   	ret = pcim_enable_device(pdev);
>   	if (ret)
> -		goto err_free_dev;
> +		return ret;
>   
>   	pci_set_drvdata(pdev, dev);
>   
>   	ret = bochs_load(bochs);
>   	if (ret)
> -		goto err_free_dev;
> +		return ret;
>   
>   	ret = drm_dev_register(dev, 0);
>   	if (ret)
> -		goto err_free_dev;
> +		return ret;
>   
>   	drm_client_setup(dev, NULL);
>   
>   	return ret;
> -
> -err_free_dev:
> -	drm_dev_put(dev);
> -	return ret;
>   }
>   
>   static void bochs_pci_remove(struct pci_dev *pdev)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)