Skip to content

Commit

Permalink
dyndbg: allow ddebug_add_module to fail a modprobe
Browse files Browse the repository at this point in the history
This upgrades the callchain around ddebug_add_module() so it can fail
a modprobe.  This will allow a class-id reservation conflict to fail
safe, and so be caught during development, rather than have a subtly
broken dyndbg facility.

Return int from ddebug_attach_*module_classes(), and return -EINVAL
from them if they encounter a class-id range overlap in the multiple
classmaps they're given.

In ddebug_add_module(), declare a reserved_ids bitmap, and pass it
by-ref into both ddebug_attach*(); it accumulates the class-id
reservations of each attached class, allowing detection of conflicts.

In ddebug_module_notify(), catch the -EINVAL from ddebug_add_module(),
and don't WARN about it, since failure is expected.  Since we're
removing the module, call ddebug_remove_module() before finishing.
This takes it out of the ddebug-tables list, which fixes BUGs on
subsequent 'cat /proc/dynamic_debug/control's.

Here it is, failing modprobe test_dynamic_debug_submod, due to the bad
classmap DEFINE added there.

[root@v6 b0-dd]# modprobe test_dynamic_debug_submod dyndbg=+p
 dyndbg: classes[0]: module:test_dynamic_debug_submod base:1 len:1 type:DISJOINT_BITS
 dyndbg: module:test_dynamic_debug_submod attached 1 classes
 dyndbg: loaded class: module:test_dynamic_debug_submod base:1 len:1 type:DISJOINT_BITS
 dyndbg: class_ref[0] test_dynamic_debug_submod -> \
 	 test_dynamic_debug module:test_dynamic_debug base:14 len:8 type:LEVEL_NUM
 dyndbg: class_ref[1] test_dynamic_debug_submod -> \
 	 test_dynamic_debug module:test_dynamic_debug base:0 len:10 type:DISJOINT_BITS
 dyndbg: Overlapping range: [0..9] on D2_CORE
modprobe: ERROR: could not insert 'test_dynamic_debug_submod': Invalid argument

The submod attaches its own bad class 1st; base: 1 len: 1, then finds
the 2 classes in the parent-module, and fails when it sees the range
overlap.  It reports the failure on the last class attached, which due
to LIFO linking, is the 2nd classmap DEFINEd in the parent module.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
squashed this in:
dyndbg: properly unwind on ddebug_add_module failure

 A recent change to ddebug_add_module() allowed it to fail when
 multiple classmaps didnt properly share the 0..62 class-id space (per
 module).  This error flowed to the notify-handler, which avoided the
 WARN, and thought all was good. But testing hit a page-fault:

 [    2.159815] dyndbg: Overlapping range: [0..9] on D2_CORE
 modprobe: ERROR: could not insert 'test_dynamic_debug_submod': Invalid argument
 [    2.160133] dyndbg: bad class reservations
 [    2.192181] BUG: unable to handle page fault for address: ffffffffc038b53b
 [    2.192872] #PF: supervisor read access in kernel mode
 [    2.193179] #PF: error_code(0x0000) - not-present page
 [    2.193489] PGD 3ca43067 P4D 3ca43067 PUD 3ca45067 PMD 1aa4067 PTE 0
 [    2.193885] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
 [    2.194182] CPU: 1 UID: 0 PID: 414 Comm: grep Not tainted 6.11.0-dd-00062-g27917198ef23 torvalds#17
 [    2.194678] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
 [    2.195036] RIP: 0010:ddebug_proc_show+0x38/0x220

 Fix this (apparently) by removing the just-added module, which takes
 it of the ddebug-tables list, so proc-show doesn't traverse it.
  • Loading branch information
jimc committed Sep 20, 2024
1 parent 8d85fa9 commit 79008ca
Showing 1 changed file with 26 additions and 14 deletions.
40 changes: 26 additions & 14 deletions lib/dynamic_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -1262,9 +1262,9 @@ static int ddebug_class_check_ranges(struct ddebug_class_map *cm,
* modular classmap vector/section. Save the start and length of the
* subrange at its edges.
*/
static void ddebug_attach_module_classes(struct ddebug_table *dt,
const struct _ddebug_info *di,
u64 *reserved_ids)
static int ddebug_attach_module_classes(struct ddebug_table *dt,
const struct _ddebug_info *di,
u64 *reserved_ids)
{
struct ddebug_class_map *cm;
int i, nc = 0;
Expand All @@ -1276,24 +1276,25 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt,
dt->classes = cm;
}
if (ddebug_class_check_ranges(cm, reserved_ids))
return;
return -EINVAL;
}
if (!nc)
return;
return 0;

vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
dt->num_classes = nc;

for (i = 0, cm = dt->classes; i < dt->num_classes; i++, cm++)
ddebug_apply_params(cm, cm->mod_name);
return 0;
}

/*
* propagates class-params thru their classmaps to class-users. this
* means a query against the dt/module, which means it must be on the
* list to be seen by ddebug_change.
*/
static void ddebug_attach_user_module_classes(struct ddebug_table *dt,
static int ddebug_attach_user_module_classes(struct ddebug_table *dt,
const struct _ddebug_info *di,
u64 *reserved_ids)
{
Expand All @@ -1317,10 +1318,10 @@ static void ddebug_attach_user_module_classes(struct ddebug_table *dt,
dt->class_users = cli;
}
if (ddebug_class_check_ranges(cli->map, reserved_ids))
return;
return -EINVAL;
}
if (!nc)
return;
return 0;

dt->num_class_users = nc;

Expand All @@ -1329,6 +1330,7 @@ static void ddebug_attach_user_module_classes(struct ddebug_table *dt,
ddebug_apply_params(cli->map, cli->user_mod_name);

vpr_dt_info(dt, "attach-client-module: ");
return 0;
}

/*
Expand All @@ -1339,6 +1341,7 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
struct ddebug_table *dt;
u64 reserved_ids;
int rc;

if (!di->num_descs)
return 0;
Expand All @@ -1362,16 +1365,20 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)

INIT_LIST_HEAD(&dt->link);

if (di->num_classes)
ddebug_attach_module_classes(dt, di, &reserved_ids);

if (di->num_classes) {
rc = ddebug_attach_module_classes(dt, di, &reserved_ids);
if (rc)
return rc;
}
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);

if (di->num_class_users)
ddebug_attach_user_module_classes(dt, di, &reserved_ids);

if (di->num_class_users) {
rc = ddebug_attach_user_module_classes(dt, di, &reserved_ids);
if (rc)
return rc;
}
vpr_info("%3u debug prints in module %s\n", di->num_descs, modname);
return 0;
}
Expand Down Expand Up @@ -1456,6 +1463,11 @@ static int ddebug_module_notify(struct notifier_block *self, unsigned long val,
switch (val) {
case MODULE_STATE_COMING:
ret = ddebug_add_module(&mod->dyndbg_info, mod->name);
if (ret == -EINVAL) {
pr_err("conflicting dyndbg-classmap reservations\n");
ddebug_remove_module(mod->name);
break;
}
if (ret)
WARN(1, "Failed to allocate memory: dyndbg may not work properly.\n");
break;
Expand Down

0 comments on commit 79008ca

Please sign in to comment.