[PATCH] media: dvb-core: prevent frontend thread resurrection after unregister

[YuanZhe Liu]

A use-after-free read in dvb_frontend_thread() was observed when a DVB USB
device was disconnected concurrently with userspace opening and closing the
frontend device node:

  BUG: KASAN: slab-use-after-free in dvb_frontend_thread+0x1380/0x1620
  Read of size 8 at addr ffff888008dd4320
  Comm: kdvb-ad-0-fe-0 Not tainted 7.1.2-dirty #81
  dvb_frontend_should_wakeup drivers/media/dvb-core/dvb_frontend.c:641
  dvb_frontend_thread drivers/media/dvb-core/dvb_frontend.c:682

  Allocated by the frontend attach path:
   cinergyt2_fe_attach+0x... (drivers/media/usb/dvb-usb/cinergyT2-fe.c:271)

  Freed by the final frontend release:
   dvb_frontend_put+0x... (drivers/media/dvb-core/dvb_frontend.c)
   dvb_frontend_release+0x... (drivers/media/dvb-core/dvb_frontend.c:2934)

On disconnect, dvb_unregister_frontend() stops the frontend kthread through
dvb_frontend_stop() and unregisters the dvb device, but it never marked the
frontend itself as removed: fe->exit was left at DVB_FE_NORMAL_EXIT, or at
DVB_FE_NO_EXIT when no thread was running.  dvb_frontend_open() does not
hold frontend_mutex, and dvb_frontend_start() did not check the removal
state at all, so an open racing with dvb_unregister_frontend() passed the
DVB_FE_DEVICE_REMOVED check in dvb_frontend_open(), unconditionally reset
fe->exit to DVB_FE_NO_EXIT and spawned a new kthread on a frontend that
was being removed.

dvb_unregister_frontend() and dvb_frontend_detach() then dropped both
registration references.  The resurrected kthread itself never holds a
reference on the frontend object, so once the last file descriptor was
closed, dvb_frontend_release() dropped the final reference and freed the
frontend state while the kthread was still running, resulting in the
use-after-free reported above.

Fix the lifetime break by making removal a permanent state:

- set fe->exit to DVB_FE_DEVICE_REMOVED in dvb_unregister_frontend()
  under frontend_mutex before stopping the thread; dvb_frontend_stop()
  already preserves this value and dvb_frontend_is_exiting() then makes
  a running thread exit immediately;
- check DVB_FE_DEVICE_REMOVED in dvb_frontend_start() after taking
  fepriv->sem and fail with -ENODEV; the thread's exit path writes
  fe->exit under the same semaphore and dvb_frontend_stop() returns only
  after kthread_stop() observed that write, so the check cannot miss a
  concurrent removal and the unconditional fe->exit = DVB_FE_NO_EXIT
  reset can no longer undo it;
- do not clear DVB_FE_DEVICE_REMOVED in dvb_frontend_thread() when the
  thread exits on its own, so the removal marker survives;
- initialize fe->exit to DVB_FE_NO_EXIT in dvb_register_frontend(), so
  a frontend object that is registered again after unregister remains
  usable.

em28xx already sets DVB_FE_DEVICE_REMOVED manually before unregistering
its frontends; the core now provides this protection for every dvb-core
user.

Fixes: 1f862a68df24 ("[media] dvb_frontend: move kref to struct dvb_frontend")
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>
---
 drivers/media/dvb-core/dvb_frontend.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index d082b6c57c76..d06f16b2ab84 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -796,7 +796,7 @@ static int dvb_frontend_thread(void *data)
 	fepriv->thread = NULL;
 	if (kthread_should_stop())
 		fe->exit = DVB_FE_DEVICE_REMOVED;
-	else
+	else if (fe->exit != DVB_FE_DEVICE_REMOVED)
 		fe->exit = DVB_FE_NO_EXIT;
 	mb();
 
@@ -873,6 +873,11 @@ static int dvb_frontend_start(struct dvb_frontend *fe)
 	if (down_interruptible(&fepriv->sem))
 		return -EINTR;
 
+	if (fe->exit == DVB_FE_DEVICE_REMOVED) {
+		up(&fepriv->sem);
+		return -ENODEV;
+	}
+
 	fepriv->state = FESTATE_IDLE;
 	fe->exit = DVB_FE_NO_EXIT;
 	fepriv->thread = NULL;
@@ -3042,6 +3047,7 @@ int dvb_register_frontend(struct dvb_adapter *dvb,
 	init_waitqueue_head(&fepriv->events.wait_queue);
 	mutex_init(&fepriv->events.mtx);
 	fe->dvb = dvb;
+	fe->exit = DVB_FE_NO_EXIT;
 	fepriv->inversion = INVERSION_OFF;
 
 	dev_info(fe->dvb->device,
@@ -3076,6 +3082,13 @@ int dvb_unregister_frontend(struct dvb_frontend *fe)
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
 	mutex_lock(&frontend_mutex);
+	/*
+	 * Mark the frontend as removed before stopping the thread, so that
+	 * a racing dvb_frontend_open() cannot restart the kthread via
+	 * dvb_frontend_start() and leave it running after the last
+	 * reference on the frontend object is dropped.
+	 */
+	fe->exit = DVB_FE_DEVICE_REMOVED;
 	dvb_frontend_stop(fe);
 	dvb_remove_device(fepriv->dvbdev);
 
-- 
2.34.1