From nobody Tue Dec 2 02:41:30 2025 Received: from mail-il1-f178.google.com (mail-il1-f178.google.com [209.85.166.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A0B033A5E62 for ; Tue, 18 Nov 2025 20:19:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763497179; cv=none; b=TTYletthOg4FUekQGDuzK5KeGC8kvZgnS/JHDG+JVopUlVjC6iy5P9KUoKl+OWvsfSLw2bYF0ftBIo517+tH9TEtgCETkL50DgsIqlVk3LI6l1vVOD4DjIVE3D2EuUeLUQhFanM63PmipHcIiBUD7MHx0EsNJgfLWqCI0UhvrI4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763497179; c=relaxed/simple; bh=VMrdZA7s31F2GEdrtj8PHSNZKtDjrq0E2iTShqTqv5M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=oZRqtQHjrz9aHkzHGLFBOe9AWjKh21GlWzrOxZ0WNXz28Z6WmmzQKFq9MIEFP1gMhn14LeyTV2ruwDiUL5AofMN4iUuKLrlmy/EKAsNU8cMz0z8xRBCFbvDXDY9O+Xfv/IUOLE6PDg/p0zx3yTEidkayBuiHDbaGeB3ErJmBG+Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=cVcJQ/VU; arc=none smtp.client-ip=209.85.166.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="cVcJQ/VU" Received: by mail-il1-f178.google.com with SMTP id e9e14a558f8ab-4332ae0635fso34810655ab.2 for ; Tue, 18 Nov 2025 12:19:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763497173; x=1764101973; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=C1toahUY738pvkwlfUtafPSZY20olgovUh6swweGApM=; b=cVcJQ/VUxSTuBMpZwOhtzm8AE5Qs5G6AehZD39HtZTnVHSKdfBuo7v2x+n0sYqV0PP xxxvpGeKwENBYMXJLMG0Diywo2/z5ZAqCXqDq+u4igErrnRh7qgj64vku7MwikY5UIE1 PGhZkOUX1HKilHbUiuj6cJicuLfB/mVu8sjHHZNVOMEcvmmK+pyWRSYViBk5NUpTPmYy /fvhoGkUUTN8jq/c29e42+rhCf66qwT7pTyd2hVv/7v2E/f5v3oSEFb3EaE6nMxq0SIR iccMyJ7lkvW4yo4XnOZSnrqfXH/o1CW1dJIUsyyCj5VJaLdXgOTfW1HRRHbZUJXwxORe 1wfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763497173; x=1764101973; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=C1toahUY738pvkwlfUtafPSZY20olgovUh6swweGApM=; b=DCONJkIGdl9PZn4ovLPMoGYtx8AphqwB4IU3boMefKDN/XYuHH2rKYhl76Cpea/H0/ PWifwuU9uMD6at7PzDY8sPL+Mv8sGrNA0qE8flK2LHbIn2UnTeBIkSTDZUhRwD6hkxRz W/yCMlHoZRj93kLNB+lpcXdRETFVyiu2CLh6hQO9ZZqCUz1g81zmKfKJtlz7IHaY/vJw fob0yoOJ/4GMQVCLbAPTSQ682dcSYN22YkJ17HFcJlu+nX3yinRpA0n1s8Vsv9h57ouo Kh1uhQgTphbjzSCqtqLhQNSGoAUEL3AZgad+VTw/JqfxhgrJO7cmZ0CJDD1zoNwUoNlF QbYg== X-Gm-Message-State: AOJu0YzoYZCSzNdCRrSTHbSSYjdxG8gp1K4EYPW4WybalyfvVVmTWt+c 2MZ7VLsz8T026nZYerU8xAhQj4jBVYnKQQ5bJLZ8zZi8hIybJhivKIoOvaA9MGQn X-Gm-Gg: ASbGncvezNUqE+cyfRX9uEydLvna6hV4zgX9XmIzCuiLQlHf5V+8Fx16Gni8eNmUDet ExMOAQi9+YGtc7vMBjgwt+U7gHQqDhmy7Fo6UcluevSif22cuY3+DrYLfYi4aptBd7dnqxJTZTX eYCc3zQ+prGQkyJbpQQA3GnffuXqCZ9Toq533+Du32HHKTR+AHGSQIvPA72cyjREBAZ5iFUke7f XH2nFxyfwyR6xbpjl4vToZphPyF98gvKI7Fl2+UMXHWLTVYrmozDeTbnxtzjmTP3hADkDawHIYH OhG2kMAwsPGJYB19GWqYPnzN/oh9F9rHg4TVssrBNo3crjWtudMZMQqzql61H8wYZDs1Tg1JdL8 e6zHNzlqpnFybKp3IQipd/3ZzWUaghgR9JGDJJbGLQhNHX//ILzD6hTQgZfOBYrTh6fHCLKeBS5 GgUPTqvYuOmUzIPOPw8iWnAAtKkFvybxvMZe6iJccRRgyUzC9ed0FEoOIqfsyXfLc0f+Q= X-Google-Smtp-Source: AGHT+IEP/UZHqJYFr/A1jMUV1bsPYuwts+iE/3zNuDLcQ/zAkDIdEP2KDusPSuCu3YA1VHfgKSswRw== X-Received: by 2002:a05:6e02:1fea:b0:433:2b02:af4d with SMTP id e9e14a558f8ab-4348c867cfbmr207286605ab.12.1763497173341; Tue, 18 Nov 2025 12:19:33 -0800 (PST) Received: from godzilla.raven-morpho.ts.net (c-98-38-17-99.hsd1.co.comcast.net. [98.38.17.99]) by smtp.googlemail.com with ESMTPSA id ca18e2360f4ac-948fd4c273bsm419823939f.18.2025.11.18.12.19.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Nov 2025 12:19:32 -0800 (PST) From: Jim Cromie To: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, gregkh@linuxfoundation.org, jbaron@akamai.com Cc: ukaszb@chromium.org, louis.chauvet@bootlin.com, Jim Cromie Subject: [PATCH v6 29/31] dyndbg: resolve "protection" of class'd pr_debug Date: Tue, 18 Nov 2025 13:18:39 -0700 Message-ID: <20251118201842.1447666-30-jim.cromie@gmail.com> X-Mailer: git-send-email 2.51.1 In-Reply-To: <20251118201842.1447666-1-jim.cromie@gmail.com> References: <20251118201842.1447666-1-jim.cromie@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" classmap-v1 code protected class'd pr_debugs from unintended changes by unclassed/_DFLT queries: # - to declutter examples: alias ddcmd=3D'echo $* > /proc/dynamic_debug/control' # IOW, this should NOT alter drm.debug settings ddcmd -p # Instead, you must name the class to change it. # Protective but tedious ddcmd class DRM_UT_CORE +p # Or do it the (old school) subsystem way # This is ABI !! echo 1 > /sys/module/drm/parameters/debug Since the debug sysfs-node is ABI, if dyndbg is going to implement it, it must also honor its settings; it must at least protect against accidental changes to its classes from legacy queries. The protection allows all previously conceived queries to work the way they always have; ie select the same set of pr_debugs, despite the inclusion of whole new classes of pr_debugs. But that choice has 2 downsides: 1. "name the class to change it" makes a tedious long-winded interface, needing many commands to set DRM_UT_* one at a time. 2. It makes the class keyword special in some sense; the other keywords skip only on query mismatch, otherwise the code falls thru to adjust the pr-debug site. Jason Baron didn't like v1 on point 2. Louis Chauvet didn't like recent rev on point 1 tedium. But that said: /sys/ is ABI, so this must be reliable: #> echo 0x1f > /sys/module/drm/parameters/debug It 'just works' without dyndbg underneath; we must deliver that same stability. Convenience is secondary. The new resolution: If ABI is the blocking issue, then no ABI means no blocking issue. IOW, if the classmap has no presence under /sys/*, ie no PARAM, there is no ABI to guard, and no reason to enforce a tedious interface. In the future, if DRM wants to alter this protection, that is practical, but I think default-on is the correct mode. So atm classes without a PARAM are unprotected at >control, allowing admins their shortcuts. I think this could satisfy all viewpoints. That said, theres also a possibility of wildcard classes: #> ddcmd class '*' +p Currently, the query-class is exact-matched against each module's classmaps.names. This gives precise behavior, a good basis. But class wildcards are possible, they just did'nt appear useful for DRM, whose classmap names are a flat DRM_UT_* namespace. IOW, theres no useful selectivity there: #> ddcmd class "DRM_*" +p # these enable every DRM_* class #> ddcmd class "DRM_UT_*" +p #> ddcmd class "DRM_UT_V*" +p # finally select just 1: DRM_UT_VBL #> ddcmd class "DRM_UT_D*" +p # but this gets 3 #> ddcmd class "D*V*" +p # here be dragons But there is debatable utility in the feature. #> ddcmd class __DEFAULT__ -p # what about this ? #> ddcmd -p # thats what this does. automatically Anyway, this patch does: 1. adds link field from _ddebug_class_map to the .controlling_param 2. sets it in ddebug_match_apply_kparam(), during modprobe/init, when options like drm.debug=3DVAL are handled. 3. ddebug_class_has_param() now checks .controlling_param 4. ddebug_class_wants_protection() macro renames 3. this frames it as a separable policy decision 5. ddebug_match_desc() gets the most attention: a. move classmap consideration to the bottom this insures all other constraints act 1st. allows simpler 'final' decisions. b. split class choices cleanly on query: class FOO vs none, and class'd vs _DPRINTK_CLASS_DFLT site. c. calls 4 when applying a class-less query to a class'd pr_debug here we need a new fn to find the classmap with this .class_id d. calls new ddebug_find_classmap_by_class_id(). when class-less query looks at a class'd pr_debug. finds classmap, which can then decide, currently by PARAM existence. NOTES: protection is only against class-less queries, explicit "class FOO" adjustments are allowed (that is the mechanism). The drm.debug sysfs-node heavily under-specifies the class'd pr_debugs it controls; none of the +mfls prefixing flags have any effect, and each callsite remains individually controllable. drm.debug just toggles the +p flag for all the modules' class'd pr_debugs. Signed-off-by: Jim Cromie --- history -v0 - original, before classmaps: no special case keywords -v1 - "class DEFAULT" is assumed if not mentioned. this protects classes from class-less queries -v2.pre-this-patch - protection macro'd to false -v2.with-this-patch - sysfs knob decides -v2.speculative - module decides wrt classmap protection seems unneeded now, TBD v3 - new patch v4 - drop fn-scope map var, with 2 local vars, renamed to purpose - fix for NULL ptr case. - Add loop-var to reduce many "&dt->info." exprs to "di->" - add 1-liner postcondition comments fixus --- include/linux/dynamic_debug.h | 14 ++-- lib/dynamic_debug.c | 127 +++++++++++++++++++++++++++------- 2 files changed, 110 insertions(+), 31 deletions(-) diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index b1d11d946780..b22da40e2583 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -75,6 +75,7 @@ enum ddebug_class_map_type { * map @class_names 0..N to consecutive constants starting at @base. */ struct _ddebug_class_map { + struct _ddebug_class_param *controlling_param; const struct module *mod; /* NULL for builtins */ const char *mod_name; const char **class_names; @@ -259,7 +260,12 @@ struct _ddebug_class_param { * * Creates a sysfs-param to control the classes defined by the * exported classmap, with bits 0..N-1 mapped to the classes named. - * This version keeps class-state in a private long int. + * + * Since sysfs-params are ABI, this also protects the classmap'd + * pr_debugs from un-class'd `echo -p > /proc/dynamic_debug/control` + * changes. + * + * This keeps class-state in a private long int. */ #define DYNAMIC_DEBUG_CLASSMAP_PARAM(_name, _var, _flags) \ static unsigned long _name##_bvec; \ @@ -272,10 +278,8 @@ struct _ddebug_class_param { * @_var: name of the (exported) classmap var defining the classes/bits * @_flags: flags to be toggled, typically just 'p' * - * Creates a sysfs-param to control the classes defined by the - * exported clasmap, with bits 0..N-1 mapped to the classes named. - * This version keeps class-state in user @_bits. This lets drm check - * __drm_debug elsewhere too. + * Like DYNAMIC_DEBUG_CLASSMAP_PARAM, but maintains param-state in + * extern @_bits. This lets DRM check __drm_debug elsewhere too. */ #define DYNAMIC_DEBUG_CLASSMAP_PARAM_REF(_name, _bits, _var, _flags) \ __DYNAMIC_DEBUG_CLASSMAP_PARAM(_name, _bits, _var, _flags) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 636a6b5741f7..1082e0273f0e 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -206,6 +206,50 @@ ddebug_find_valid_class(struct _ddebug_info const *di,= const char *query_class, return NULL; } =20 +static bool ddebug_class_in_range(const int class_id, const struct _ddebug= _class_map *map) +{ + return (class_id >=3D map->base && + class_id < map->base + map->length); +} + +static struct _ddebug_class_map * +ddebug_find_map_by_class_id(struct _ddebug_info *di, int class_id) +{ + struct _ddebug_class_map *map; + struct _ddebug_class_user *cli; + int i; + + for_subvec(i, map, di, maps) + if (ddebug_class_in_range(class_id, map)) + return map; + + for_subvec(i, cli, di, users) + if (ddebug_class_in_range(class_id, cli->map)) + return cli->map; + + return NULL; +} + +/* + * classmaps-V1 protected classes from changes by legacy commands + * (those selecting _DPRINTK_CLASS_DFLT by omission). This had the + * downside that saying "class FOO" for every change can get tedious. + * + * V2 is smarter, it protects class-maps if the defining module also + * calls DYNAMIC_DEBUG_CLASSMAP_PARAM to create a sysfs parameter. + * Since the author wants the knob, we should assume they intend to + * use it (in preference to "class FOO +p" >control), and want to + * trust its settings. This gives protection when its useful, and not + * when its just tedious. + */ +static inline bool ddebug_class_has_param(const struct _ddebug_class_map *= map) +{ + return !!(map->controlling_param); +} + +/* re-framed as a policy choice */ +#define ddebug_class_wants_protection(map) (ddebug_class_has_param(map)) + /* * Search the tables for _ddebug's which match the given `query' and * apply the `flags' and `mask' to them. Returns number of matching @@ -214,11 +258,10 @@ ddebug_find_valid_class(struct _ddebug_info const *di= , const char *query_class, */ static bool ddebug_match_desc(const struct ddebug_query *query, struct _ddebug *dp, - int valid_class) + struct _ddebug_info *di, + int selected_class) { - /* match site against query-class */ - if (dp->class_id !=3D valid_class) - return false; + struct _ddebug_class_map *site_map; =20 /* match against the source filename */ if (query->filename && @@ -255,7 +298,28 @@ static bool ddebug_match_desc(const struct ddebug_quer= y *query, dp->lineno > query->last_lineno) return false; =20 - return true; + /* + * above are all satisfied, so we can make final decisions: + * 1- class FOO or implied class __DEFAULT__ + * 2- site.is_classed or not + */ + if (query->class_string) { + /* class FOO given, exact match required */ + return (dp->class_id =3D=3D selected_class); + } + /* query class __DEFAULT__ by omission. */ + if (dp->class_id =3D=3D _DPRINTK_CLASS_DFLT) { + /* un-classed site */ + return true; + } + /* site is class'd */ + site_map =3D ddebug_find_map_by_class_id(di, dp->class_id); + if (!site_map) { + /* _UNKNOWN_ class_id. XXX: Allow changes here ? */ + return false; + } + /* module(-param) decides protection */ + return !ddebug_class_wants_protection(site_map); } =20 static int ddebug_change(const struct ddebug_query *query, struct flag_set= tings *modifiers) @@ -265,33 +329,31 @@ static int ddebug_change(const struct ddebug_query *q= uery, struct flag_settings unsigned int newflags; unsigned int nfound =3D 0; struct flagsbuf fbuf, nbuf; - struct _ddebug_class_map *map =3D NULL; - int valid_class; + int selected_class; =20 /* search for matching ddebugs */ mutex_lock(&ddebug_lock); list_for_each_entry(dt, &ddebug_tables, link) { struct _ddebug_info *di =3D &dt->info; + struct _ddebug_class_map *mods_map; =20 /* match against the module name */ if (query->module && !match_wildcard(query->module, di->mod_name)) continue; =20 + selected_class =3D _DPRINTK_CLASS_DFLT; if (query->class_string) { - map =3D ddebug_find_valid_class(&dt->info, query->class_string, - &valid_class); - if (!map) + mods_map =3D ddebug_find_valid_class(di, query->class_string, + &selected_class); + if (!mods_map) continue; - } else { - /* constrain query, do not touch class'd callsites */ - valid_class =3D _DPRINTK_CLASS_DFLT; } =20 for (i =3D 0; i < di->descs.len; i++) { struct _ddebug *dp =3D &di->descs.start[i]; =20 - if (!ddebug_match_desc(query, dp, valid_class)) + if (!ddebug_match_desc(query, dp, di, selected_class)) continue; =20 nfound++; @@ -656,6 +718,7 @@ static int ddebug_exec_queries(char *query, const char = *modname) } =20 /* apply a new class-param setting */ + static int ddebug_apply_class_bitmap(const struct _ddebug_class_param *dcp, const unsigned long *new_bits, const unsigned long old_bits, @@ -1107,12 +1170,6 @@ static void *ddebug_proc_next(struct seq_file *m, vo= id *p, loff_t *pos) return dp; } =20 -static bool ddebug_class_in_range(const int class_id, const struct _ddebug= _class_map *map) -{ - return (class_id >=3D map->base && - class_id < map->base + map->length); -} - static const char *ddebug_class_name(struct _ddebug_info *di, struct _ddeb= ug *dp) { struct _ddebug_class_map *map; @@ -1237,25 +1294,36 @@ static void ddebug_sync_classbits(const struct kern= el_param *kp, const char *mod } } =20 -static void ddebug_match_apply_kparam(const struct kernel_param *kp, - const struct _ddebug_class_map *map, - const char *mod_name) +static struct _ddebug_class_param * +ddebug_get_classmap_kparam(const struct kernel_param *kp, + const struct _ddebug_class_map *map) { struct _ddebug_class_param *dcp; =20 if (kp->ops !=3D ¶m_ops_dyndbg_classes) - return; + return false; =20 dcp =3D (struct _ddebug_class_param *)kp->arg; =20 - if (map =3D=3D dcp->map) { + return (map =3D=3D dcp->map) + ? dcp : (struct _ddebug_class_param *)NULL; +} + +static void ddebug_match_apply_kparam(const struct kernel_param *kp, + struct _ddebug_class_map *map, + const char *mod_name) +{ + struct _ddebug_class_param *dcp =3D ddebug_get_classmap_kparam(kp, map); + + if (dcp) { + map->controlling_param =3D dcp; v2pr_info(" kp:%s.%s =3D0x%lx", mod_name, kp->name, *dcp->bits); vpr_cm_info(map, " %s maps ", mod_name); ddebug_sync_classbits(kp, mod_name); } } =20 -static void ddebug_apply_params(const struct _ddebug_class_map *cm, const = char *mod_name) +static void ddebug_apply_params(struct _ddebug_class_map *cm, const char *= mod_name) { const struct kernel_param *kp; #if IS_ENABLED(CONFIG_MODULES) @@ -1275,6 +1343,13 @@ static void ddebug_apply_params(const struct _ddebug= _class_map *cm, const char * } } =20 +/* + * called from add_module, ie early. it can find controlling kparams, + * which can/does? enable protection of this classmap from class-less + * queries, on the grounds that the user created the kparam, means to + * use it, and expects it to reflect reality. We should oblige him, + * and protect those classmaps from classless "-p" changes. + */ static void ddebug_apply_class_maps(const struct _ddebug_info *di) { struct _ddebug_class_map *cm; --=20 2.51.1