lkml.org 
[lkml]   [2021]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2] platform/chrome: cros_ec_debugfs: detach log reader wq from devm
    Hi Benson and Guenter,

    On Mon, Nov 1, 2021 at 4:36 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
    >
    > Debugfs console_log uses devm memory (e.g. debug_info in
    > cros_ec_console_log_poll()). 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.
    >
    > The call trace:
    > do_raw_spin_lock
    > _raw_spin_lock_irqsave
    > remove_wait_queue
    > ep_unregister_pollwait
    > ep_remove
    > do_epoll_ctl
    >
    > A Python example to reproduce the issue:
    > ... 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).
    >
    > Detaches log reader's workqueue from devm to make sure it is persistent
    > even if the device has been removed.
    >
    > Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
    > ---
    > As for 2 other related cases I could image:
    >
    > 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

    Would you mind to share your thoughts on the patch?

    \
     
     \ /
      Last update: 2021-11-09 07:50    [W:7.238 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site