On 06/14/2016 05:20 PM, Rusty Russell wrote: > Prarit Bhargava <prarit@xxxxxxxxxx> writes: >> Blacklisting a module in linux has long been a problem. The current >> procedure is to use rd.blacklist=module_name, however, that doesn't >> cover the case after the initramfs and before a boot prompt (where one >> is supposed to use /etc/modprobe.d/blacklist.conf to blacklist >> runtime loading). Using rd.shell to get an early prompt is hit-or-miss, >> and doesn't cover all situations AFAICT. >> >> This patch adds this functionality of permanently blacklisting a module >> by its name via the kernel parameter module_blacklist=module_name. >> >> [v2]: Rusty, use core_param() instead of __setup(), and drop struct which >> simplifies things. >> >> Signed-off-by: Prarit Bhargava <prarit@xxxxxxxxxx> >> Cc: Jonathan Corbet <corbet@xxxxxxx> >> Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx> >> Cc: linux-doc@xxxxxxxxxxxxxxx >> --- >> Documentation/kernel-parameters.txt | 3 +++ >> kernel/module.c | 25 +++++++++++++++++++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt >> index 82b42c958d1c..c720b96f2efc 100644 >> --- a/Documentation/kernel-parameters.txt >> +++ b/Documentation/kernel-parameters.txt >> @@ -2295,6 +2295,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted. >> Note that if CONFIG_MODULE_SIG_FORCE is set, that >> is always true, so this option does nothing. >> >> + module_blacklist= [KNL] Do not load a comma-separated list of >> + modules. Useful for debugging problem modules. >> + >> mousedev.tap_time= >> [MOUSE] Maximum time between finger touching and >> leaving touchpad surface for touch to be considered >> diff --git a/kernel/module.c b/kernel/module.c >> index 5f71aa63ed2a..5ff5287b19a8 100644 >> --- a/kernel/module.c >> +++ b/kernel/module.c >> @@ -3155,6 +3155,28 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr, >> return 0; >> } >> >> +/* module_blacklist is a comma-separated list of module names */ >> +static char *module_blacklist; >> +static bool blacklisted(char *module_name) >> +{ >> + char *str, *entry; >> + >> + if (!module_blacklist) >> + return false; >> + >> + str = module_blacklist; >> + do { >> + entry = strsep(&str, ","); >> + if (!strcmp(module_name, entry)) { >> + pr_info("module %s is blacklisted\n", module_name); >> + return true; >> + } > > strsep mangles the string; this will only work once :) > > This is untested, and a little ugly: > > len = strlen(module_name); > > while ((p = strstr(p, module_name)) != NULL) { > if ((p == module_blacklist || p[-1] == ',') && > (p[len] == ',' || p[len] == '\0')) > return true; > p += len; > } > return false; Hmm ... yeah, a bit ugly. I could also easily do: str = module_blacklist; do { if (str != module_blacklist) module_blacklist[strlen(str) - 1] = ','; entry = strsep(&str, ","); if (!strcmp(module_name, entry)) { pr_info("module %s is blacklisted\n", module_name); if (str != module_blacklist) module_blacklist[strlen(str) - 1] = ','; return true; } } while (str); which results in the correct behavior AFAICT with "module_blacklist=dummy_module,prarit_module": On boot: [ 23.737613] blacklist: comparing |dns_resolver| to |dummy_module| [ 23.743713] blacklist: comparing |dns_resolver| to |prarit_module| when attempting to load dummy_module: [ 43.938798] blacklist: comparing |dummy_module| to |dummy_module| [ 43.944915] module dummy_module is blacklisted and attempting to load another module after that (in this case ixgbe): [ 47.572557] blacklist: comparing |mdio| to |dummy_module| [ 47.577961] blacklist: comparing |mdio| to |prarit_module| [ 47.684713] blacklist: comparing |ixgbe| to |dummy_module| [ 47.690202] blacklist: comparing |ixgbe| to |prarit_module| Any objection to that version? Admittedly yours is shorter but I feel like mine might be "easier" to read... P. > > Cheers, > Rusty. > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html