Thread (39 messages) 39 messages, 2 authors, 4d ago

Re: [PATCH v7 11/29] dyndbg: factor ddebug_match_desc out from ddebug_change

From: sashiko-bot@kernel.org
Date: 2026-07-21 21:05:27
Also in: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added NULL format check uses an un-ratelimited, context-free `pr_info()` inside a globally locked inner loop.
--
--- Patch [11]: [PATCH v7 11/29] dyndbg: factor ddebug_match_desc out from ddebug_change ---
commit 47cab4604933a3e3a136a1d101a11ae69fe36598
Author: Jim Cromie [off-list ref]
Subject: dyndbg: factor ddebug_match_desc out from ddebug_change

This commit factors out the per-descriptor filtering logic from the inner loop
of ddebug_change() into a separate boolean function, ddebug_match_desc(). It
also introduces a check to prevent operating on debug descriptors with a NULL
format string.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 18a71a9108d3e..577a079160727 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -172,6 +172,56 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
  * callsites, normally the same as number of changes.  If verbose,
  * logs the changes.  Takes ddebug_lock.
  */
+static bool ddebug_match_desc(const struct ddebug_query *query,
+			      struct _ddebug *dp,
+			      int valid_class)
+{
[ ... ]
+	/* match against the format */
+	if (query->format) {
+		if (!dp->format) {
+			pr_info("encountered a NULL format\n");
[Severity: Medium]
Could this un-ratelimited pr_info() spam the kernel log? 

Since ddebug_match_desc() is executed inside a nested loop holding
ddebug_lock, if there is a malformed descriptor in the system, querying the
format via debugfs could trigger this message repeatedly on every format
query.

Additionally, because the message lacks context, it doesn't provide clues
to help identify which callsite is missing the format string. Would it be
better to include details like dp->filename, dp->function, or dp->lineno
here, and perhaps use pr_info_once() or pr_warn_ratelimited() to avoid
log flooding?
+			return false;
+		}
+		if (*query->format == '^') {
+			char *p;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dd-maint-2-v7-0-010fbe73b311@gmail.com?part=11
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help