linux-kernel - Re: [PATCH] rv: Fix wrong type cast in enabled_monitors

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

Message-ID: <87tt0t4u19.fsf@yellow.woof>
Date: Tue, 23 Sep 2025 07:28:18 +0200
From: Nam Cao <namcao@...utronix.de>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Gabriele Monaco <gmonaco@...hat.com>, linux-trace-kernel@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rv: Fix wrong type cast in enabled_monitors_next()

Hi Nathan,

Nathan Chancellor <nathan@...nel.org> writes:
> I am seeing a crash when reading from /sys/kernel/tracing/rv/enabled_monitors
> on a couple of my arm64 boxes running Fedora after this change, which
> landed in mainline in 6.17-rc7. I can reproduce this in QEMU pretty
> easily.
...
> With this change reverted, there is no crash. As this change seems to
> have proper justification, is there some other latent bug here?

Thanks for the report.

Yes, this patch is broken, because argument 'p' of
enabled_monitors_next() *is* a pointer to struct rv_monitor. I'm not
sure how did I even test this patch... Steven is right, we really need
something in kselftest for RV, another thing in my RV TODO list.

But reverting is not the real fix, because monitors_show() still expects
a pointer to list_head. Changing monitors_show() is not an option,
because it is shared with the 'available_monitors' interface.

So the real fix is completely changing the iterator to be list_head
instead of rv_monitor.

Best regards,
Nam

diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index 48338520376f..43e9ea473cda 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -501,7 +501,7 @@ static void *enabled_monitors_next(struct seq_file *m, void *p, loff_t *pos)
 
 	list_for_each_entry_continue(mon, &rv_monitors_list, list) {
 		if (mon->enabled)
-			return mon;
+			return &mon->list;
 	}
 
 	return NULL;
@@ -509,7 +509,7 @@ static void *enabled_monitors_next(struct seq_file *m, void *p, loff_t *pos)
 
 static void *enabled_monitors_start(struct seq_file *m, loff_t *pos)
 {
-	struct rv_monitor *mon;
+	struct list_head *head;
 	loff_t l;
 
 	mutex_lock(&rv_interface_lock);
@@ -517,15 +517,15 @@ static void *enabled_monitors_start(struct seq_file *m, loff_t *pos)
 	if (list_empty(&rv_monitors_list))
 		return NULL;
 
-	mon = list_entry(&rv_monitors_list, struct rv_monitor, list);
+	head = &rv_monitors_list;
 
 	for (l = 0; l <= *pos; ) {
-		mon = enabled_monitors_next(m, mon, &l);
-		if (!mon)
+		head = enabled_monitors_next(m, head, &l);
+		if (!head)
 			break;
 	}
 
-	return mon;
+	return head;
 }
 
 /*