[PATCH 23/63] dyndbg: fail modprobe on ddebug_class_range_overlap()

Jim Cromie jim.cromie at gmail.com
Mon Nov 4 21:37:44 UTC 2024


1. All classes used by a module (declared DYNDBG_CLASSMAP_{DEFINE,USE}
by module code) must share 0..62 class-id space; ie their respective
base,+length reservations shouldn't overlap.  Overlaps would lead to
unintended changes in ddebug enablements.

Detecting these class-id range overlaps at compile-time would be ideal
but is not obvious how; failing at modprobe at least insures that the
developer sees and fixes the conflict.

ddebug_class_range_overlap() implements the range check, accumulating
the reserved-ids as it examines each class.  It probably should use
bitmaps.

A previous commit reworked the modprobe callchain to allow failure,
now call ddebug_class_range_overlap() to check when classid conflicts
happen, and signal that failure.

NB: this can only happen when a module defines+uses several classmaps,

TBD: failing modprobe is kinda harsh, maybe warn and proceed ?

test_dynamic_debug*.ko:

If built with -DFORCE_CLASSID_CONFLICT_MODPROBE, the modules get 2 bad
DYNDBG_CLASS_DEFINE declarations, into parent and the _submod.  These
conflict with one of the good ones in the parent (D2_CORE..etc),
causing the modprobe(s) to fail.  TODO: do in submod only, since fail
of parent prevents submod from ever trying.

Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
---
---
 lib/dynamic_debug.c      | 30 ++++++++++++++++++++++++------
 lib/test_dynamic_debug.c | 11 ++++++++++-
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8afcd4111531..8e1e087e07c3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1211,6 +1211,21 @@ static void ddebug_apply_params(const struct ddebug_class_map *cm, const char *m
 	}
 }
 
+static int ddebug_class_range_overlap(struct ddebug_class_map *cm,
+				      u64 *reserved_ids)
+{
+	u64 range = (((1ULL << cm->length) - 1) << cm->base);
+
+	if (range & *reserved_ids) {
+		pr_err("[%d..%d] on %s conflicts with %llx\n", cm->base,
+		       cm->base + cm->length - 1, cm->class_names[0],
+		       *reserved_ids);
+		return -EINVAL;
+	}
+	*reserved_ids |= range;
+	return 0;
+}
+
 /*
  * scan the named array: @_vec, ref'd from inside @_box, for the
  * start,len of the sub-array of elements matching on ->mod_name;
@@ -1242,9 +1257,11 @@ static int ddebug_module_apply_class_maps(struct ddebug_table *dt,
 	struct ddebug_class_map *cm;
 	int i;
 
-	for_subvec(i, cm, &dt->info, maps)
+	for_subvec(i, cm, &dt->info, maps) {
+		if (ddebug_class_range_overlap(cm, reserved_ids))
+			return -EINVAL;
 		ddebug_apply_params(cm, cm->mod_name);
-
+	}
 	vpr_info("module:%s attached %d classmaps\n", dt->mod_name, dt->info.maps.len);
 	return 0;
 }
@@ -1255,10 +1272,11 @@ static int ddebug_module_apply_class_users(struct ddebug_table *dt,
 	struct ddebug_class_user *cli;
 	int i;
 
-	/* now iterate dt */
-	for_subvec(i, cli, &dt->info, users)
+	for_subvec(i, cli, &dt->info, users) {
+		if (ddebug_class_range_overlap(cli->map, reserved_ids))
+			return -EINVAL;
 		ddebug_apply_params(cli->map, cli->mod_name);
-
+	}
 	vpr_info("module:%s attached %d classmap uses\n", dt->mod_name, dt->info.users.len);
 	return 0;
 }
@@ -1311,11 +1329,11 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
 			return rc;
 		}
 	}
+
 	mutex_lock(&ddebug_lock);
 	list_add_tail(&dt->link, &ddebug_tables);
 	mutex_unlock(&ddebug_lock);
 
-
 	if (dt->info.users.len) {
 		rc = ddebug_module_apply_class_users(dt, &reserved_ids);
 		if (rc)
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index b1555b0a2bb1..74b98adc4ed0 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -81,7 +81,7 @@ enum cat_disjoint_bits {
 	D2_DRMRES };
 
 /* numeric verbosity, V2 > V1 related */
-enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
+enum cat_level_num { V0 = 16, V1, V2, V3, V4, V5, V6, V7 };
 
 /* recapitulate DRM's parent(drm.ko) <-- _submod(drivers,helpers) */
 #if !defined(TEST_DYNAMIC_DEBUG_SUBMOD)
@@ -90,6 +90,7 @@ enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
  * classmaps on the client enums above, and then declares the PARAMS
  * ref'g the classmaps.  Each is exported.
  */
+
 DYNDBG_CLASSMAP_DEFINE(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS,
 		       D2_CORE,
 		       "D2_CORE",
@@ -113,6 +114,14 @@ DYNDBG_CLASSMAP_DEFINE(map_level_num, DD_CLASS_TYPE_LEVEL_NUM,
 DYNDBG_CLASSMAP_PARAM(disjoint_bits, p);
 DYNDBG_CLASSMAP_PARAM(level_num, p);
 
+#ifdef FORCE_CLASSID_CONFLICT_MODPROBE
+/*
+ * Enable with -Dflag on compile to test overlapping class-id range
+ * detection.  This should break on modprobe.
+ */
+DYNDBG_CLASSMAP_DEFINE(classid_range_conflict, 0, D2_CORE + 1, "D3_CORE");
+#endif
+
 #else /* TEST_DYNAMIC_DEBUG_SUBMOD */
 
 /*
-- 
2.47.0



More information about the Intel-gfx-trybot mailing list