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

[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@vger.kernel.org
Co-developed-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Bin Yu <byu@xidian.edu.cn>
Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>

---
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

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

[YuanZhe Liu]

Hi Sean,

The v2 below does not apply to linux-next/next-20261026 (f1ac44c32cba):
rc-main.c has been reworked by your series in the media tree, in
particular ec7cb49871e5 ("Fix race condition during rc_register_device()")
and 37efd09ba2c6 ("Ensure registered is cleared in error path").

37efd09ba2c6 already covers the lirc side of the problem, so the
out_lirc part of my patch is redundant.  However, the original uevent
use-after-free still looks possible in the current code:

	out_rx:
		rc_free_rx_device(dev);
	out_lirc:
		scoped_guard(mutex, &dev->lock)
			dev->registered = false;

rc_free_rx_device() calls ir_free_table(), which sets rc_map.name to
NULL under rc_map->lock and then kfree()s it.  rc_dev_uevent() takes
dev->lock, checks dev->registered (still true at this point) and reads
rc_map.name at rc-main.c:1660 without taking rc_map->lock.  A reader can
therefore take the pointer before it is cleared and dereference it after
the kfree(), which is the slab-use-after-free originally reported:

  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

This path is taken when ir_raw_event_register() fails, i.e. when
kthread_run() fails, which is the reported case.

Would it work to clear registered before freeing the keymap in out_rx?
Something like:

 out_rx:
+	scoped_guard(mutex, &dev->lock)
+		dev->registered = false;
+
 	rc_free_rx_device(dev);
 out_lirc:
 	scoped_guard(mutex, &dev->lock)
 		dev->registered = false;

The second clear is then redundant on the out_rx path, but it is still
needed for the failures that jump straight to out_lirc (lirc_register()
and rc_setup_rx_device()).  A separate label could avoid the duplicate
if you prefer.

I am happy to send a rebased patch against linux-next if that is easier
for you; otherwise feel free to fold this into your series.  If you do,
a Reported-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn> for the
analysis would be appreciated.

Thanks,
YuanZhe

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

[YuanZhe Liu]

Hi Mauro,

I am copying this address for visibility.

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

The remaining issue is a uevent racing with keymap cleanup while
registered is still true. The proposed approach is to clear registered
before freeing the keymap.

The follow-up linked below describes the remaining issue and the changes
needed for the current tree. I'd appreciate your feedback on the
proposed approach.

Discussion:
https://lore.kernel.org/r/20261027021718.14264-1-25031212351@stu.xidian.edu.cn

Original submission:
https://lore.kernel.org/r/20261027014144.9144-1-25031212351@stu.xidian.edu.cn

Thanks,
YuanZhe

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

[Sean Young]

On Fri, Oct 30, 2026 at 02:00:57PM +0800, YuanZhe Liu wrote:
> Hi Mauro,
> 
> I am copying this address for visibility.
> 
>   [PATCH v2] media: rc: fix use-after-free of rc_map.name on register
>   error path

This looks good on first pass, I need to review it.

> The remaining issue is a uevent racing with keymap cleanup while
> registered is still true. The proposed approach is to clear registered
> before freeing the keymap.

The problem with the current approach is that the remove uevent is
suppressed if it depends on registered. We need the uevent to always
be delivered, yet the uevent contains driver strings.

> The follow-up linked below describes the remaining issue and the changes
> needed for the current tree. I'd appreciate your feedback on the
> proposed approach.
> 
> Discussion:
> https://lore.kernel.org/r/20261027021718.14264-1-25031212351@stu.xidian.edu.cn
> 
> Original submission:
> https://lore.kernel.org/r/20261027014144.9144-1-25031212351@stu.xidian.edu.cn
> 
> Thanks,
> YuanZhe

Thanks,

Sean

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

[YuanZhe Liu]

Hi Sean,

You are right, and thanks - that is a good point. Once rc_dev_uevent()
returns -ENODEV, dev_uevent() fails and kobject_uevent_env() bails out,
so the KOBJ_REMOVE event is never sent at all. udev has seen the
KOBJ_ADD, so suppressing the remove leaves it with a stale device. That
is the wrong direction.

The strings themselves are not the problem at the point the remove
uevent is generated: rc_map.name, driver_name and device_name are all
still valid until the driver frees the rc_dev after rc_register_device()
returns, which is well after device_del(). What we need is that nothing
is freed before the uevent is sent, not that the uevent is suppressed.

So the error path should keep the device registered until device_del()
has sent the remove uevent: at out_rx only unregister the input device
(it is a child of dev->dev), keep the keymap for the uevent, and move
both the clearing of registered and the freeing of the keymap after
device_del(). device_del() drains the sysfs readers, so rc_dev_uevent()
cannot run afterwards and rc_map.name can be freed at that point.

Something like this against next-20261026:

 out_rx:
-	rc_free_rx_device(dev);
+	/* Keep the keymap until the remove uevent has been sent. */
+	if (dev->input_dev) {
+		input_unregister_device(dev->input_dev);
+		dev->input_dev = NULL;
+	}
 out_lirc:
-	scoped_guard(mutex, &dev->lock)
-		dev->registered = false;
-
 	lirc_unregister(dev);
 	device_del(&dev->dev);
-	/* registered already cleared above */
+
+	/*
+	 * The remove uevent has been sent and the sysfs readers have
+	 * been drained; lirc file descriptors that are still open must
+	 * not reach the driver any more.
+	 */
+	scoped_guard(mutex, &dev->lock)
+		dev->registered = false;
 	goto out_free_table;

out_free_table already frees the keymap after device_del(), so the
out_rx path gets the deferred free without a new label. rc_dev_uevent()
keeps its registered check: it is still what keeps open lirc descriptors
away from the driver, and it now runs only after the uevent has been
delivered.

The same ordering exists in rc_unregister_device() since 4f0835d6677d:
it clears registered and frees the input device and the keymap before
device_del(), so the remove uevent is suppressed there as well. I am
happy to send the error path patch only, or to extend it to
rc_unregister_device() as well - let me know which you prefer, and I
will rebase it on linux-next.

If you fold this into your series, a Reported-by: YuanZhe Liu
<25031212351@stu.xidian.edu.cn> for the analysis would be appreciated.

Thanks,
YuanZhe