linux-kernel - Re: [PATCH v2] platform/chrome: cros_ec_debugfs: detach log reader wq from devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+Px+wWWCY2mVoaH8T+vXpzunYxUzeuyQALV0UMhmO2_W5CPxQ@mail.gmail.com>
Date:   Tue, 9 Nov 2021 14:49:34 +0800
From:   Tzung-Bi Shih <tzungbi@...gle.com>
To:     bleung@...omium.org, groeck@...omium.org, bleung@...gle.com,
        groeck@...gle.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [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@...gle.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@...gle.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?