[PATCH] media: dvb-core: fix UAF on failed device registration

[PATCH] media: dvb-core: fix UAF on failed device registration

[Shuangpeng Bai]

dvb_register_device() stores a dvb_device pointer in dvb_minors[] and
takes a reference for that slot before calling dvb_register_media_device()
and device_create().

If either later step fails, the error paths free the dvb_device directly
without clearing the published minor slot or dropping the minor-table
reference. A subsequent open of the same character device minor can fetch
the stale pointer from dvb_minors[] and dereference freed memory in
dvb_device_open().

This can be reproduced with the in-tree vidtv driver by using failslab to
fail the media_devnode_create() allocation in dvb_register_media_device().

Add a helper to undo the minor-table publication and use it in both the
normal remove path and the post-publication registration failure paths.
Drop the initial registration reference with dvb_device_put() instead of
bypassing the kref with kfree().

Fixes: 0fc044b2b5e2 ("media: dvbdev: adopts refcnt to avoid UAF")
Cc: stable@vger.kernel.org
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
 drivers/media/dvb-core/dvbdev.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index d753d329502a..64c68581b649 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -448,6 +448,14 @@ static int dvb_register_media_device(struct dvb_device *dvbdev,
 	return 0;
 }
 
+static void dvb_device_remove_minor(struct dvb_device *dvbdev)
+{
+	down_write(&minor_rwsem);
+	dvb_minors[dvbdev->minor] = NULL;
+	dvb_device_put(dvbdev);
+	up_write(&minor_rwsem);
+}
+
 int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 			const struct dvb_device *template, void *priv,
 			enum dvb_device_type type, int demux_sink_pads)
@@ -551,6 +559,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	if (ret) {
 		pr_err("%s: dvb_register_media_device failed to create the mediagraph\n",
 		       __func__);
+		dvb_device_remove_minor(dvbdev);
 		if (new_node) {
 			list_del(&new_node->list_head);
 			kfree(dvbdevfops);
@@ -558,8 +567,8 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 		}
 		dvb_media_device_free(dvbdev);
 		list_del(&dvbdev->list_head);
-		kfree(dvbdev);
 		*pdvbdev = NULL;
+		dvb_device_put(dvbdev);
 		mutex_unlock(&dvbdev_register_lock);
 		return ret;
 	}
@@ -570,6 +579,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	if (IS_ERR(clsdev)) {
 		pr_err("%s: failed to create device dvb%d.%s%d (%pe)\n",
 		       __func__, adap->num, dnames[type], id, clsdev);
+		dvb_device_remove_minor(dvbdev);
 		if (new_node) {
 			list_del(&new_node->list_head);
 			kfree(dvbdevfops);
@@ -577,8 +587,8 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 		}
 		dvb_media_device_free(dvbdev);
 		list_del(&dvbdev->list_head);
-		kfree(dvbdev);
 		*pdvbdev = NULL;
+		dvb_device_put(dvbdev);
 		mutex_unlock(&dvbdev_register_lock);
 		return PTR_ERR(clsdev);
 	}
@@ -596,10 +606,7 @@ void dvb_remove_device(struct dvb_device *dvbdev)
 	if (!dvbdev)
 		return;
 
-	down_write(&minor_rwsem);
-	dvb_minors[dvbdev->minor] = NULL;
-	dvb_device_put(dvbdev);
-	up_write(&minor_rwsem);
+	dvb_device_remove_minor(dvbdev);
 
 	dvb_media_device_free(dvbdev);
 
-- 
2.43.0