All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Tzung-Bi Shih <tzungbi@google.com>,
	Guenter Roeck <groeck@google.com>,
	Benson Leung <bleung@chromium.org>,
	Sasha Levin <sashal@kernel.org>,
	chrome-platform@lists.linux.dev
Subject: [PATCH AUTOSEL 5.4 14/17] platform/chrome: cros_ec_debugfs: detach log reader wq from devm
Date: Wed,  6 Apr 2022 21:15:18 -0400	[thread overview]
Message-ID: <20220407011521.115014-14-sashal@kernel.org> (raw)
In-Reply-To: <20220407011521.115014-1-sashal@kernel.org>

From: Tzung-Bi Shih <tzungbi@google.com>

[ Upstream commit 0e8eb5e8acbad19ac2e1856b2fb2320184299b33 ]

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>
Reviewed-by: Guenter Roeck <groeck@google.com>
Link: https://lore.kernel.org/r/20220209051130.386175-1-tzungbi@google.com
Signed-off-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 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 6ae484989d1f..c4b57e1df192 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -26,6 +26,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.
  *
@@ -34,7 +37,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
  */
@@ -45,7 +47,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;
@@ -108,7 +109,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);
@@ -142,7 +143,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;
@@ -174,7 +175,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,
@@ -359,7 +360,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.1


  parent reply	other threads:[~2022-04-07  1:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07  1:15 [PATCH AUTOSEL 5.4 01/17] gfs2: assign rgrp glock before compute_bitstructs Sasha Levin
2022-04-07  1:15 ` [Cluster-devel] " Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 02/17] rtc: fix use-after-free on device removal Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 03/17] um: Cleanup syscall_handler_t definition/cast, fix warning Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 04/17] um: port_user: Improve error handling when port-helper is not found Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 05/17] Input: add bounds checking to input_set_capability() Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 06/17] Input: stmfts - fix reference leak in stmfts_input_open Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 07/17] crypto: stm32 - fix reference leak in stm32_crc_remove Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 08/17] crypto: x86/chacha20 - Avoid spurious jumps to other functions Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 09/17] ALSA: hda/realtek: Enable headset mic on Lenovo P360 Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 10/17] nvme-multipath: fix hang when disk goes live over reconnect Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 11/17] rtc: mc146818-lib: Fix the AltCentury for AMD platforms Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 12/17] MIPS: lantiq: check the return value of kzalloc() Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 13/17] drbd: remove usage of list iterator variable after loop Sasha Levin
2022-04-07  1:15 ` Sasha Levin [this message]
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 15/17] ARM: 9191/1: arm/stacktrace, kasan: Silence KASAN warnings in unwind_frame() Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 16/17] nilfs2: fix lockdep warnings in page operations for btree nodes Sasha Levin
2022-04-07  1:15   ` Sasha Levin
2022-04-07  1:15 ` [PATCH AUTOSEL 5.4 17/17] nilfs2: fix lockdep warnings during disk space reclamation Sasha Levin
2022-04-07  1:15   ` Sasha Levin

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=20220407011521.115014-14-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=groeck@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tzungbi@google.com \
    /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.