[PATCH v2] media: rc: fix use-after-free of rc_map.name on register error path

[YuanZhe Liu]

If rc_register_device() fails after device_add() - for example when
ir_raw_event_register() cannot create its kthread - the unwind path
frees the keymap via rc_free_rx_device() while the device is still
visible in sysfs, and never clears dev->registered.

A userspace reader of the uevent attribute then enters
rc_dev_uevent(), passes the registered check under dev->lock and
dereferences the already freed dev->rc_map.name:

  BUG: KASAN: slab-use-after-free in string+0x483/0x4c0
  lib/vsprintf.c:737
  Read of size 1 at addr ffff888007ca9d80
   rc_dev_uevent+0xd7/0x1d0 drivers/media/rc/rc-main.c:1628
   dev_uevent+0x28d/0x570 drivers/base/core.c:2709
   uevent_show+0x1af/0x380 drivers/base/core.c:2756

Fix it the same way rc_unregister_device() does: take dev->lock,
close the input device if it is still open and clear
dev->registered before releasing any resources. rc_dev_uevent()
then observes the cleared flag and returns -ENODEV instead of
reading rc_map.name. The same sequence is also needed when
rc_setup_rx_device() fails: lirc has been registered by then, so
an open /dev/lircX file descriptor would otherwise keep calling
driver callbacks that access the torn down device.

Fixes: f56928abaa6d ("[media] rc-core: cleanup rc_register_device")
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>

---
v2:
 - Clear dev->registered on the rc_setup_rx_device() error path too;
   lirc is already registered there and open file descriptors can
   still reach driver callbacks.
 - Factor the lock/close/clear sequence into rc_unregister_device_state()
   and share it with rc_unregister_device().
---
 drivers/media/rc/rc-main.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index dda3479ea3ad..3ac5d532c6dc 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1888,6 +1888,24 @@ static void rc_free_rx_device(struct rc_dev *dev)
 	ir_free_table(&dev->rc_map);
 }
 
+/*
+ * Prevent userspace from accessing a device that is being taken down.
+ *
+ * The uevent attribute, the lirc character device and the input device
+ * all check dev->registered under dev->lock before using driver data.
+ * Clear it while still holding that lock, and call dev->close() for an
+ * input device that is still open, exactly as rc_unregister_device()
+ * does, before the resources they use are freed.
+ */
+static void rc_unregister_device_state(struct rc_dev *dev)
+{
+	mutex_lock(&dev->lock);
+	if (dev->users && dev->close)
+		dev->close(dev);
+	dev->registered = false;
+	mutex_unlock(&dev->lock);
+}
+
 int rc_register_device(struct rc_dev *dev)
 {
 	const char *path;
@@ -1954,8 +1972,10 @@ int rc_register_device(struct rc_dev *dev)
 
 	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
 		rc = rc_setup_rx_device(dev);
-		if (rc)
+		if (rc) {
+			rc_unregister_device_state(dev);
 			goto out_lirc;
+		}
 	}
 
 	if (dev->driver_type == RC_DRIVER_IR_RAW) {
@@ -1970,6 +1990,7 @@ int rc_register_device(struct rc_dev *dev)
 	return 0;
 
 out_rx:
+	rc_unregister_device_state(dev);
 	rc_free_rx_device(dev);
 out_lirc:
 	if (dev->allowed_protocols != RC_PROTO_BIT_CEC)
@@ -2024,11 +2045,7 @@ void rc_unregister_device(struct rc_dev *dev)
 	timer_delete_sync(&dev->timer_keyup);
 	timer_delete_sync(&dev->timer_repeat);
 
-	mutex_lock(&dev->lock);
-	if (dev->users && dev->close)
-		dev->close(dev);
-	dev->registered = false;
-	mutex_unlock(&dev->lock);
+	rc_unregister_device_state(dev);
 
 	rc_free_rx_device(dev);
 
-- 
2.34.1