[PATCH v2] media: rtl28xxu: fix SDR platform device leak

[PATCH v2] media: rtl28xxu: fix SDR platform device leak

[Guangshuo Li]

rtl2832u_tuner_attach() registers an rtl2832_sdr platform device, but
does not unregister it if no driver is bound. It also does not take a
reference on the bound driver module.

Unregister the platform device when no driver is bound or the module
reference cannot be acquired. Drop the module reference in
rtl28xxu_tuner_detach() before unregistering the device.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Fixes: a2f7f220df5e ("[media] rtl28xxu: switch SDR module to platform driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - Add the missing try_module_get()/module_put() pair for the SDR driver
    module, as suggested by Hans Verkuil.

 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 487c6ab784ab..d2957e4bf017 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1391,8 +1391,16 @@ static int rtl2832u_tuner_attach(struct dvb_usb_adapter *adap)
 						     "rtl2832_sdr",
 						     PLATFORM_DEVID_AUTO,
 						     &pdata, sizeof(pdata));
-		if (IS_ERR(pdev) || pdev->dev.driver == NULL)
+		if (IS_ERR(pdev))
 			break;
+		if (!pdev->dev.driver) {
+			platform_device_unregister(pdev);
+			break;
+		}
+		if (!try_module_get(pdev->dev.driver->owner)) {
+			platform_device_unregister(pdev);
+			break;
+		}
 		dev->platform_device_sdr = pdev;
 		break;
 	default:
@@ -1426,8 +1434,10 @@ static int rtl28xxu_tuner_detach(struct dvb_usb_adapter *adap)
 
 	/* remove platform SDR */
 	pdev = dev->platform_device_sdr;
-	if (pdev)
+	if (pdev) {
+		module_put(pdev->dev.driver->owner);
 		platform_device_unregister(pdev);
+	}
 
 	/* remove I2C tuner */
 	client = dev->i2c_client_tuner;
-- 
2.43.0

Re: [PATCH v2] media: rtl28xxu: fix SDR platform device leak

[Markus Elfring]

…
> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> @@ -1391,8 +1391,16 @@ static int rtl2832u_tuner_attach(struct dvb_usb_adapter *adap)
>  						     "rtl2832_sdr",
>  						     PLATFORM_DEVID_AUTO,
>  						     &pdata, sizeof(pdata));
> -		if (IS_ERR(pdev) || pdev->dev.driver == NULL)
> +		if (IS_ERR(pdev))
>  			break;
> +		if (!pdev->dev.driver) {
> +			platform_device_unregister(pdev);
> +			break;
> +		}
> +		if (!try_module_get(pdev->dev.driver->owner)) {
> +			platform_device_unregister(pdev);
> +			break;
> +		}
…

How do you think about to avoid a bit of duplicate source code
by merging two condition checks into a disjunction?

Regards,
Markus