linux-kernel - [PATCH 38/63] dyndbg: drop "protection" of class'd pr

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250125064619.8305-39-jim.cromie@gmail.com>
Date: Fri, 24 Jan 2025 23:45:52 -0700
From: Jim Cromie <jim.cromie@...il.com>
To: linux-kernel@...r.kernel.org,
	jbaron@...mai.com,
	gregkh@...uxfoundation.org,
	ukaszb@...omium.org
Cc: intel-gfx-trybot@...ts.freedesktop.org,
	dri-devel@...ts.freedesktop.org,
	amd-gfx@...ts.freedesktop.org,
	intel-gvt-dev@...ts.freedesktop.org,
	intel-gfx@...ts.freedesktop.org,
	daniel.vetter@...ll.ch,
	tvrtko.ursulin@...ux.intel.com,
	jani.nikula@...el.com,
	ville.syrjala@...ux.intel.com,
	Jim Cromie <jim.cromie@...il.com>
Subject: [PATCH 38/63] dyndbg: drop "protection" of class'd pr_debugs from legacy queries

Current classmap code protects class'd pr_debugs from unintended
changes by "legacy" unclassed queries:

  # this doesn't disable all of DRM_UT_* categories
  echo "-p" > /proc/dynamic_debug/control

  # name the class to change it - protective but tedious
  echo "class DRM_UT_CORE +p" > /proc/dynamic_debug/control

  # or do it the subsystem way
  echo 1 > /sys/module/drm/parameters/debug

This "name the class to change it" behavior gave a modicum of
protection to classmap users (ie DRM) so their debug settings aren't
trivially and unintentionally altered underneath them.  Afterall,
__drm_debug is authoritative w/o dyndbg under it.

But this made the class keyword special in some sense; the other
keywords follow the rule: no "keyword value" given means no skipping
on that criterion.

Jason Baron didn't like this special case when I 1st proposed it;
I argued 2 points:
- "protection gives stable-debug, improving utility"
- "class _DFLT" assumed in query is not that special

I thought I'd convinced him back then, (and the patchset got merged),
but he noted it again when he reviewed this series.  So this commit
undoes the special case, indirectly.

The difference in behavior comes down to a choice on how to handle
class-mismatches; ddebug_client_module_protects_classes() names this
choice (and defines it false).  On class-mismatches of this kind, it
is "called" before skipping the legacy cmd on a class'd site.

So this makes it a policy-choice, and reverts to original policy.

Later, if any user/module wants to protect its classes, we could add a
flag to ddebug_table, a means to set it from CLASSMAP_DEFINE, and
check it when applying a classless query/cmd.  Further, runtime mods
to the protection are possible via an exported fn, or a grammar
addition to >control.

NOTES

In ddebug_change(), the 2-part class code is mostly unchanged:

1. query->class_str is validated once per module, and maps good ones
to valid_class, now renamed to slctd_class to convey non-boolean-ness.

2. in the inner per-site loop, site mismatches are now 2 sub-cases:

a. a class_str given explicitly, and site doesn't match it.
   skip/continue.
b. no class_str given, _CLASS_DFLT is inferred
   skip if ddebug_client_module_protects_classes()
   else fall thru to site-change

CC: jbaron@...mai.com
Signed-off-by: Jim Cromie <jim.cromie@...il.com>
---
 lib/dynamic_debug.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 68b5a77c2d79..9a278c7ca365 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -193,6 +193,17 @@ static int ddebug_find_valid_class(struct ddebug_table const *dt, const char *cl
 	return -ENOENT;
 }
 
+/*
+ * classmaps-v1 protected classes from changes by legacy commands
+ * (those selecting _DPRINTK_CLASS_DFLT by omission), v2 undoes that
+ * special treatment.  State so explicitly.  Later we could give
+ * modules the choice to protect their classes or to keep v2 behavior.
+ */
+static inline bool ddebug_client_module_protects_classes(const struct ddebug_table *dt)
+{
+	return false;
+}
+
 /*
  * Search the tables for _ddebug's which match the given `query' and
  * apply the `flags' and `mask' to them.  Returns number of matching
@@ -206,7 +217,7 @@ static int ddebug_change(const struct ddebug_query *query, struct flag_settings
 	unsigned int newflags;
 	unsigned int nfound = 0;
 	struct flagsbuf fbuf, nbuf;
-	int valid_class;
+	int slctd_class;
 
 	/* search for matching ddebugs */
 	mutex_lock(&ddebug_lock);
@@ -218,21 +229,26 @@ static int ddebug_change(const struct ddebug_query *query, struct flag_settings
 			continue;
 
 		if (query->class_string) {
-			valid_class = ddebug_find_valid_class(dt, query->class_string);
-			if (valid_class < 0)
+			slctd_class = ddebug_find_valid_class(dt, query->class_string);
+			if (slctd_class < 0)
+				/* skip/reject classes unknown by module */
 				continue;
 		} else {
-			/* constrain query, do not touch class'd callsites */
-			valid_class = _DPRINTK_CLASS_DFLT;
+			slctd_class = _DPRINTK_CLASS_DFLT;
 		}
 
 		for (i = 0; i < dt->info.descs.len; i++) {
 			struct _ddebug *dp = &dt->info.descs.start[i];
 
-			/* match site against query-class */
-			if (dp->class_id != valid_class)
-				continue;
-
+			if (dp->class_id != slctd_class) {
+				if (query->class_string)
+					/* site.class != given class */
+					continue;
+				/* legacy query, class'd site */
+				else if (ddebug_client_module_protects_classes(dt))
+					continue;
+				/* allow change on class'd pr_debug */
+			}
 			/* match against the source filename */
 			if (query->filename &&
 			    !match_wildcard(query->filename, dp->filename) &&
-- 
2.48.1