linux-kernel - [PATCH] fs: Improve eventpoll logging to stop indicting timerfd

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-next>] [day] [month] [year] [list]

Message-Id: <20210302034928.3761098-1-varmam@google.com>
Date:   Mon,  1 Mar 2021 19:49:28 -0800
From:   Manish Varma <varmam@...gle.com>
To:     Alexander Viro <viro@...iv.linux.org.uk>,
        Thomas Gleixner <tglx@...utronix.de>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Manish Varma <varmam@...gle.com>,
        Kelly Rossmoyer <krossmo@...gle.com>
Subject: [PATCH] fs: Improve eventpoll logging to stop indicting timerfd

timerfd doesn't create any wakelocks, but eventpoll can.  When it does,
it names them after the underlying file descriptor, and since all
timerfd file descriptors are named "[timerfd]" (which saves memory on
systems like desktops with potentially many timerfd instances), all
wakesources created as a result of using the eventpoll-on-timerfd idiom
are called... "[timerfd]".

However, it becomes impossible to tell which "[timerfd]" wakesource is
affliated with which process and hence troubleshooting is difficult.

This change addresses this problem by changing the way eventpoll and
timerfd file descriptors (and thus the eventpoll-on-timerfd wakesources)
are named:

1) timerfd descriptors are now named "[timerfdN:P]", where N is a
monotonically increasing integer for each timerfd instance created and P
is the command string of the creating process.
2) the top-level per-process eventpoll wakesource is now named "epoll:P"
(instead of just "eventpoll"), where P, again, is the command string of
the creating process
3) individual per-underlying-filedescriptor eventpoll wakesources are
now named "epollitem:P.F", where P is the command string of the creating
process and F is the name of the underlying file descriptor.

All together, that will give us names like the following:

1) timerfd file descriptor: [timerfd14:system_server]
2) eventpoll top-level per-process wakesource: epoll:system_server
3) eventpoll-on-timerfd per-descriptor wakesource:
epollitem:system_server.[timerfd14:system_server]

Co-developed-by: Kelly Rossmoyer <krossmo@...gle.com>
Signed-off-by: Kelly Rossmoyer <krossmo@...gle.com>
Signed-off-by: Manish Varma <varmam@...gle.com>
---
 fs/eventpoll.c | 10 ++++++++--
 fs/timerfd.c   | 11 ++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 185252d8f4c7..af28be4285f8 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1451,15 +1451,21 @@ static int ep_create_wakeup_source(struct epitem *epi)
 {
 	struct name_snapshot n;
 	struct wakeup_source *ws;
+	char task_comm_buf[sizeof(current->comm)];
+	char buf[64];
+
+	get_task_comm(task_comm_buf, current);
 
 	if (!epi->ep->ws) {
-		epi->ep->ws = wakeup_source_register(NULL, "eventpoll");
+		snprintf(buf, sizeof(buf), "epoll:%s", task_comm_buf);
+		epi->ep->ws = wakeup_source_register(NULL, buf);
 		if (!epi->ep->ws)
 			return -ENOMEM;
 	}
 
 	take_dentry_name_snapshot(&n, epi->ffd.file->f_path.dentry);
-	ws = wakeup_source_register(NULL, n.name.name);
+	snprintf(buf, sizeof(buf), "epollitem:%s.%s", task_comm_buf, n.name.name);
+	ws = wakeup_source_register(NULL, buf);
 	release_dentry_name_snapshot(&n);
 
 	if (!ws)
diff --git a/fs/timerfd.c b/fs/timerfd.c
index c5509d2448e3..4249e8c9a38c 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -46,6 +46,8 @@ struct timerfd_ctx {
 	bool might_cancel;
 };
 
+static atomic_t instance_count = ATOMIC_INIT(0);
+
 static LIST_HEAD(cancel_list);
 static DEFINE_SPINLOCK(cancel_lock);
 
@@ -391,6 +393,9 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
 {
 	int ufd;
 	struct timerfd_ctx *ctx;
+	char task_comm_buf[sizeof(current->comm)];
+	char file_name_buf[32];
+	int instance;
 
 	/* Check the TFD_* constants for consistency.  */
 	BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
@@ -427,7 +432,11 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
 
 	ctx->moffs = ktime_mono_to_real(0);
 
-	ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
+	instance = atomic_inc_return(&instance_count);
+	get_task_comm(task_comm_buf, current);
+	snprintf(file_name_buf, sizeof(file_name_buf), "[timerfd%d:%s]",
+		 instance, task_comm_buf);
+	ufd = anon_inode_getfd(file_name_buf, &timerfd_fops, ctx,
 			       O_RDWR | (flags & TFD_SHARED_FCNTL_FLAGS));
 	if (ufd < 0)
 		kfree(ctx);
-- 
2.30.0.617.g56c4b15f3c-goog