From: Tzung-Bi Shih <tzungbi@google.com>
To: bleung@chromium.org, groeck@chromium.org
Cc: chrome-platform@lists.linux.dev, tzungbi@google.com
Subject: [RESEND PATCH v2] platform/chrome: cros_ec_debugfs: detach log reader wq from devm
Date: Wed, 9 Feb 2022 13:11:30 +0800 [thread overview]
Message-ID: <20220209051130.386175-1-tzungbi@google.com> (raw)
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
Changes from v1:
- rebase to next-20211029.
- change the commit messages.
drivers/platform/chrome/cros_ec_debugfs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 272c89837d74..0dbceee87a4b 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -25,6 +25,9 @@
#define CIRC_ADD(idx, size, value) (((idx) + (value)) & ((size) - 1))
+/* waitqueue for log readers */
+static DECLARE_WAIT_QUEUE_HEAD(cros_ec_debugfs_log_wq);
+
/**
* struct cros_ec_debugfs - EC debugging information.
*
@@ -33,7 +36,6 @@
* @log_buffer: circular buffer for console log information
* @read_msg: preallocated EC command and buffer to read console log
* @log_mutex: mutex to protect circular buffer
- * @log_wq: waitqueue for log readers
* @log_poll_work: recurring task to poll EC for new console log data
* @panicinfo_blob: panicinfo debugfs blob
*/
@@ -44,7 +46,6 @@ struct cros_ec_debugfs {
struct circ_buf log_buffer;
struct cros_ec_command *read_msg;
struct mutex log_mutex;
- wait_queue_head_t log_wq;
struct delayed_work log_poll_work;
/* EC panicinfo */
struct debugfs_blob_wrapper panicinfo_blob;
@@ -107,7 +108,7 @@ static void cros_ec_console_log_work(struct work_struct *__work)
buf_space--;
}
- wake_up(&debug_info->log_wq);
+ wake_up(&cros_ec_debugfs_log_wq);
}
mutex_unlock(&debug_info->log_mutex);
@@ -141,7 +142,7 @@ static ssize_t cros_ec_console_log_read(struct file *file, char __user *buf,
mutex_unlock(&debug_info->log_mutex);
- ret = wait_event_interruptible(debug_info->log_wq,
+ ret = wait_event_interruptible(cros_ec_debugfs_log_wq,
CIRC_CNT(cb->head, cb->tail, LOG_SIZE));
if (ret < 0)
return ret;
@@ -173,7 +174,7 @@ static __poll_t cros_ec_console_log_poll(struct file *file,
struct cros_ec_debugfs *debug_info = file->private_data;
__poll_t mask = 0;
- poll_wait(file, &debug_info->log_wq, wait);
+ poll_wait(file, &cros_ec_debugfs_log_wq, wait);
mutex_lock(&debug_info->log_mutex);
if (CIRC_CNT(debug_info->log_buffer.head,
@@ -377,7 +378,6 @@ static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info)
debug_info->log_buffer.tail = 0;
mutex_init(&debug_info->log_mutex);
- init_waitqueue_head(&debug_info->log_wq);
debugfs_create_file("console_log", S_IFREG | 0444, debug_info->dir,
debug_info, &cros_ec_console_log_fops);
--
2.35.0.263.gb82422642f-goog
next reply other threads:[~2022-02-09 5:11 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-09 5:11 Tzung-Bi Shih [this message]
2022-02-09 16:38 ` [RESEND PATCH v2] platform/chrome: cros_ec_debugfs: detach log reader wq from devm Guenter Roeck
2022-02-09 21:30 ` Prashant Malani
2022-02-10 3:11 ` Tzung-Bi Shih
2022-02-18 19:28 ` Benson Leung
2022-02-18 19:30 ` patchwork-bot+chrome-platform
2022-03-31 16:50 ` patchwork-bot+chrome-platform
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220209051130.386175-1-tzungbi@google.com \
--to=tzungbi@google.com \
--cc=bleung@chromium.org \
--cc=chrome-platform@lists.linux.dev \
--cc=groeck@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.