Messages in this thread | | | From | Tzung-Bi Shih <> | Date | Tue, 19 Oct 2021 13:24:28 +0800 | Subject | Re: [RESEND PATCH] platform/chrome: cros_ec_debugfs: detach log reader wq from devm |
| |
On Mon, Oct 18, 2021 at 4:59 PM Guenter Roeck <groeck@google.com> wrote: > > On Mon, Oct 18, 2021 at 2:03 AM Tzung-Bi Shih <tzungbi@google.com> wrote: > > > > Debugfs console_log uses devm memory (see struct cros_ec_debugfs in > > cros_ec_console_log fops). However, lifecycles of device and debugfs > > are independent. An use-after-free issue is observed if userland > > program operates the debugfs after the memory has been freed. > > > > It would help to see the backtrace. Without it, it is difficult to > determine where the UAF is observed. Also, most if not all of the > touched functions access struct cros_ec_debugfs all over the place, > not only for the wait queue, so I am not sure if moving the wait queue > out of the data structure is the correct fix. It might instead be > necessary to disconnect memory allocations from the ec device.
A trimmed backtrace is in the commit message, but the more verbose one: [ 426.174308] Call trace: [ 426.174314] dump_backtrace+0x0/0x3ec [ 426.174318] show_stack+0x20/0x2c [ 426.174324] dump_stack+0x11c/0x1ac [ 426.174329] print_address_description+0x7c/0x510 [ 426.174333] kasan_report+0x134/0x174 [ 426.174337] __asan_report_load4_noabort+0x44/0x50 [ 426.174341] do_raw_spin_lock+0x214/0x308 [ 426.174345] _raw_spin_lock_irqsave+0x68/0xf0 [ 426.174350] remove_wait_queue+0x3c/0x10c [ 426.174355] ep_unregister_pollwait+0x120/0x170 [ 426.174358] ep_remove+0x60/0x2a0 [ 426.174362] do_epoll_ctl+0x590/0x7f4
I guess only the wait queue in the struct cros_ec_debugfs has deep-coupled to console_log debugfs. There are 2 more file operation scenarios appended after the "--".
> > An userland program example in Python code: > > ... import select > > ... p = select.epoll() > > ... f = open('/sys/kernel/debug/cros_scp/console_log') > > ... p.register(f, select.POLLIN) > > ... p.poll(1) > > [(4, 1)] # 4=fd, 1=select.POLLIN > > > > [ shutdown cros_scp at the point ] > > > > ... p.poll(1) > > [(4, 16)] # 4=fd, 16=select.POLLHUP > > ... p.unregister(f) > > > > An use-after-free issue raises here. It called epoll_ctl with > > EPOLL_CTL_DEL which in turn to use the workqueue in the devm (i.e. > > log_wq). > > > > The calling stack: > > do_raw_spin_lock > > _raw_spin_lock_irqsave > > remove_wait_queue > > ep_unregister_pollwait > > ep_remove > > do_epoll_ctl
Here is the trimmed backtrace.
[...] > > --- > > As for 2 other cases: > > > > Case 1. userland program opens the debugfs after the device has been removed > > > > ENOENT. cros_ec_debugfs_remove() calls debugfs_remove_recursive(). > > > > Case 2. userland program is reading when the device is removing > > > > If the userland program is waiting in cros_ec_console_log_read(), the device > > removal will wait for it. > > > > See the calling stack for the case: > > wait_for_completion > > __debugfs_file_removed > > remove_one > > simple_recursive_removal > > debugfs_remove > > cros_ec_debugfs_remove > > platform_drv_remove > > device_release_driver_internal > > device_release_driver > > bus_remove_device > > device_del > > platform_device_del > > platform_device_unregister
Here are the 2 other cases.
| |