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@xxxxxxxxxxxxxxxxx> for the
analysis would be appreciated.

Thanks,
YuanZhe