[PATCH v2] media: dvb-core: skip disable_source handler for a released entity

[YuanZhe Liu]

dvb_remove_device() frees the media entity of a dvb_device through
dvb_media_device_free() and sets dvbdev->entity to NULL.  The media
device of the adapter can still be registered at that point: au0828,
for example, only clears mdev->disable_source in au0828_usb_release(),
which runs after au0828_dvb_unregister() has unregistered the frontend
device.

A file descriptor that survives the disconnect then reaches
dvb_frontend_release() with dvbdev->entity already NULL and calls

	fe->dvb->mdev->disable_source(dvbdev->entity);

au0828_disable_source() dereferences that argument unconditionally:

	struct media_device *mdev = entity->graph_obj.mdev;

which faults:

	general protection fault: 0000 [#1] SMP KASAN
	KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
	RIP: au0828_disable_source+0x27/0xb70 au0828-core.c:445
	Call Trace:
	 dvb_frontend_release+0x2eb/0x500 dvb_frontend.c:2946

The error path of __dvb_frontend_open() has the same call.

Skip the handler when dvbdev->entity is gone.  For that check to be
sufficient, clear dvbdev->entity in dvb_media_device_free() before the
entity is unregistered and freed: both frontend paths read the pointer
while holding mdev->graph_mutex, and media_device_unregister_entity()
takes the same lock, so a reader either still sees a registered entity,
for which unregister waits, or sees NULL.  Clearing the pointer after
the unregister would leave a window in which the entity is read after
it was already freed.

Read the pointer only once, into a local variable, while the lock is
held.  dvb_media_device_free() stores NULL before taking graph_mutex,
so re-reading the field to pass it as the argument could still observe
NULL and hand it to the handler.

Fixes: 68ff5767ddc4 ("[media] media: dvb-frontend invoke enable/disable_source handlers")
Cc: stable@xxxxxxxxxxxxxxx
Co-developed-by: Bin Yu <byu@xxxxxxxxxxxxx>
Signed-off-by: Bin Yu <byu@xxxxxxxxxxxxx>
Co-developed-by: Cheng Wen <wencheng@xxxxxxxxxxxxx>
Signed-off-by: Cheng Wen <wencheng@xxxxxxxxxxxxx>
Signed-off-by: YuanZhe Liu <25031212351@xxxxxxxxxxxxxxxxx>
---
Changes in v2:
- Read dvbdev->entity once into a local variable under graph_mutex at both
  call sites.  dvb_media_device_free() clears the pointer without holding
  that lock, so re-reading the field for the call argument could still pass
  NULL to the handler.

 drivers/media/dvb-core/dvb_frontend.c | 14 ++++++++++----
 drivers/media/dvb-core/dvbdev.c       | 14 +++++++++++---
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 0286da57f382..37bd909fe0fe 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2826,9 +2826,12 @@ static int __dvb_frontend_open(struct inode *inode, struct file *file)
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
 	mutex_lock(&fe->dvb->mdev_lock);
 	if (fe->dvb->mdev) {
+		struct media_entity *entity;
+
 		mutex_lock(&fe->dvb->mdev->graph_mutex);
-		if (fe->dvb->mdev->disable_source)
-			fe->dvb->mdev->disable_source(dvbdev->entity);
+		entity = dvbdev->entity;
+		if (entity && fe->dvb->mdev->disable_source)
+			fe->dvb->mdev->disable_source(entity);
 		mutex_unlock(&fe->dvb->mdev->graph_mutex);
 	}
 	mutex_unlock(&fe->dvb->mdev_lock);
@@ -2941,9 +2944,12 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
 		mutex_lock(&fe->dvb->mdev_lock);
 		if (fe->dvb->mdev) {
+			struct media_entity *entity;
+
 			mutex_lock(&fe->dvb->mdev->graph_mutex);
-			if (fe->dvb->mdev->disable_source)
-				fe->dvb->mdev->disable_source(dvbdev->entity);
+			entity = dvbdev->entity;
+			if (entity && fe->dvb->mdev->disable_source)
+				fe->dvb->mdev->disable_source(entity);
 			mutex_unlock(&fe->dvb->mdev->graph_mutex);
 		}
 		mutex_unlock(&fe->dvb->mdev_lock);
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index d753d329502a..0b1ac40af164 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -208,10 +208,18 @@ static void dvb_media_device_free(struct dvb_device *dvbdev)
 {
 #if defined(CONFIG_MEDIA_CONTROLLER_DVB)
 	if (dvbdev->entity) {
-		media_device_unregister_entity(dvbdev->entity);
-		kfree(dvbdev->entity);
-		kfree(dvbdev->pads);
+		struct media_entity *entity = dvbdev->entity;
+
+		/*
+		 * Clear the pointer before the entity is unregistered and
+		 * freed.  The frontend release and open paths read it under
+		 * mdev->graph_mutex and pass it to the disable_source
+		 * handler, so a stale entity must not stay reachable.
+		 */
 		dvbdev->entity = NULL;
+		media_device_unregister_entity(entity);
+		kfree(entity);
+		kfree(dvbdev->pads);
 		dvbdev->pads = NULL;
 	}
 
-- 
2.34.1