linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Add kernel parameter to blacklist modules
@ 2016-06-13 12:32 Prarit Bhargava
  2016-06-13 21:23 ` Rusty Russell
  2016-06-14 17:17 ` [PATCH] " Christoph Hellwig
  0 siblings, 2 replies; 9+ messages in thread
From: Prarit Bhargava @ 2016-06-13 12:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: Prarit Bhargava, Jonathan Corbet, Rusty Russell, linux-doc

Blacklisting a module in linux has long been a problem.  The process of
blacklisting a module has changed over time, and it seems that every OS
does it slightly differently and depends on the age of the init system
used on that OS.

The current Fedora/systemd 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.  Explaining all of this
to an end user isn't trivial.  This becomes especially difficult when
attempting to blacklist a module during system install from RO media.

This patch adds this functionality of blacklisting a module by its name
via the kernel parameter module_blacklist=module_name.  The parameter
can also accept comma separated values.

Usage example:

[root@intel-mohonpeak-02 ~]# insmod /home/dummy-module/dummy-module.ko
insmod: ERROR: could not insert module /home/rhel7/redhat/debug/dummy-module/dummy-module.ko: Operation not permitted
[root@intel-mohonpeak-02 ~]# dmesg | grep black
[    0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.7.0-rc2+ root=/dev/mapper/rhel_intel--mohonpeak--02-root ro crashkernel=auto rd.lvm.lv=rhel_intel-mohonpeak-02/root rd.lvm.lv=rhel_intel-mohonpeak-02/swap console=ttyS0,115200 LANG=en_US.UTF-8 module_blacklist=dummy_module
[    0.000000] Kernel command line: BOOT_IMAGE=/vmlinuz-4.7.0-rc2+ root=/dev/mapper/rhel_intel--mohonpeak--02-root ro crashkernel=auto rd.lvm.lv=rhel_intel-mohonpeak-02/root rd.lvm.lv=rhel_intel-mohonpeak-02/swap console=ttyS0,115200 LANG=en_US.UTF-8 module_blacklist=dummy_module
[    0.000000] blacklisting module dummy_module
[   85.127433] module dummy_module has been blacklisted.  This module will not load.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: linux-doc@vger.kernel.org
---
 Documentation/kernel-parameters.txt |    3 +++
 kernel/module.c                     |   37 +++++++++++++++++++++++++++++++++++
 2 files changed, 40 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..ed3076d98a32 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -46,6 +46,7 @@
 #include <linux/string.h>
 #include <linux/mutex.h>
 #include <linux/rculist.h>
+#include <linux/bootmem.h>
 #include <asm/uaccess.h>
 #include <asm/cacheflush.h>
 #include <asm/mmu_context.h>
@@ -3155,16 +3156,52 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
 	return 0;
 }
 
+struct mod_blacklist_entry {
+	struct list_head next;
+	char *buf;
+};
+
+static LIST_HEAD(blacklisted_modules);
+static int __init module_blacklist(char *str)
+{
+	char *str_entry;
+	struct mod_blacklist_entry *entry;
+
+	/* str argument is a comma-separated list of module names */
+	do {
+		str_entry = strsep(&str, ",");
+		if (str_entry) {
+			pr_debug("blacklisting module %s\n", str_entry);
+			entry = alloc_bootmem(sizeof(*entry));
+			entry->buf = alloc_bootmem(strlen(str_entry) + 1);
+			strcpy(entry->buf, str_entry);
+			list_add(&entry->next, &blacklisted_modules);
+		}
+	} while (str_entry);
+
+	return 0;
+}
+__setup("module_blacklist=", module_blacklist);
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
 	/* Module within temporary copy. */
 	struct module *mod;
 	int err;
+	struct mod_blacklist_entry *entry;
 
 	mod = setup_load_info(info, flags);
 	if (IS_ERR(mod))
 		return mod;
 
+	/* Has this module been blacklisted by the user? */
+	list_for_each_entry(entry, &blacklisted_modules, next) {
+		if (!strcmp(mod->name, entry->buf)) {
+			pr_debug("module %s blacklisted\n", mod->name);
+			return ERR_PTR(-EPERM);
+		}
+	}
+
 	err = check_modinfo(mod, info, flags);
 	if (err)
 		return ERR_PTR(err);
-- 
1.7.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add kernel parameter to blacklist modules
  2016-06-13 12:32 [PATCH] Add kernel parameter to blacklist modules Prarit Bhargava
@ 2016-06-13 21:23 ` Rusty Russell
  2016-06-14 17:15   ` [PATCH v2] " Prarit Bhargava
  2016-06-14 17:17 ` [PATCH] " Christoph Hellwig
  1 sibling, 1 reply; 9+ messages in thread
From: Rusty Russell @ 2016-06-13 21:23 UTC (permalink / raw)
  To: Prarit Bhargava, linux-kernel; +Cc: Prarit Bhargava, Jonathan Corbet, linux-doc

Prarit Bhargava <prarit@redhat.com> writes:
> Blacklisting a module in linux has long been a problem.  The process of
> blacklisting a module has changed over time, and it seems that every OS
> does it slightly differently and depends on the age of the init system
> used on that OS.

Hi Prarit,

        I don't mind the idea, but we can refine the implementation a
little I think.

1) Use core_param(module_blacklist, module_blacklist, charp, 0400)
   instead of __setup().  I'd like to kill __setup(), really.
2) Simply search the blacklist string at load time, no new datastructures.

Not completely sure about #2, but I think it'll be fairly neat.

Thanks,
Rusty.

> The current Fedora/systemd 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.  Explaining all of this
> to an end user isn't trivial.  This becomes especially difficult when
> attempting to blacklist a module during system install from RO media.
>
> This patch adds this functionality of blacklisting a module by its name
> via the kernel parameter module_blacklist=module_name.  The parameter
> can also accept comma separated values.
>
> Usage example:
>
> [root@intel-mohonpeak-02 ~]# insmod /home/dummy-module/dummy-module.ko
> insmod: ERROR: could not insert module /home/rhel7/redhat/debug/dummy-module/dummy-module.ko: Operation not permitted
> [root@intel-mohonpeak-02 ~]# dmesg | grep black
> [    0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.7.0-rc2+ root=/dev/mapper/rhel_intel--mohonpeak--02-root ro crashkernel=auto rd.lvm.lv=rhel_intel-mohonpeak-02/root rd.lvm.lv=rhel_intel-mohonpeak-02/swap console=ttyS0,115200 LANG=en_US.UTF-8 module_blacklist=dummy_module
> [    0.000000] Kernel command line: BOOT_IMAGE=/vmlinuz-4.7.0-rc2+ root=/dev/mapper/rhel_intel--mohonpeak--02-root ro crashkernel=auto rd.lvm.lv=rhel_intel-mohonpeak-02/root rd.lvm.lv=rhel_intel-mohonpeak-02/swap console=ttyS0,115200 LANG=en_US.UTF-8 module_blacklist=dummy_module
> [    0.000000] blacklisting module dummy_module
> [   85.127433] module dummy_module has been blacklisted.  This module will not load.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: linux-doc@vger.kernel.org
> ---
>  Documentation/kernel-parameters.txt |    3 +++
>  kernel/module.c                     |   37 +++++++++++++++++++++++++++++++++++
>  2 files changed, 40 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..ed3076d98a32 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -46,6 +46,7 @@
>  #include <linux/string.h>
>  #include <linux/mutex.h>
>  #include <linux/rculist.h>
> +#include <linux/bootmem.h>
>  #include <asm/uaccess.h>
>  #include <asm/cacheflush.h>
>  #include <asm/mmu_context.h>
> @@ -3155,16 +3156,52 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
>  	return 0;
>  }
>  
> +struct mod_blacklist_entry {
> +	struct list_head next;
> +	char *buf;
> +};
> +
> +static LIST_HEAD(blacklisted_modules);
> +static int __init module_blacklist(char *str)
> +{
> +	char *str_entry;
> +	struct mod_blacklist_entry *entry;
> +
> +	/* str argument is a comma-separated list of module names */
> +	do {
> +		str_entry = strsep(&str, ",");
> +		if (str_entry) {
> +			pr_debug("blacklisting module %s\n", str_entry);
> +			entry = alloc_bootmem(sizeof(*entry));
> +			entry->buf = alloc_bootmem(strlen(str_entry) + 1);
> +			strcpy(entry->buf, str_entry);
> +			list_add(&entry->next, &blacklisted_modules);
> +		}
> +	} while (str_entry);
> +
> +	return 0;
> +}
> +__setup("module_blacklist=", module_blacklist);
> +
>  static struct module *layout_and_allocate(struct load_info *info, int flags)
>  {
>  	/* Module within temporary copy. */
>  	struct module *mod;
>  	int err;
> +	struct mod_blacklist_entry *entry;
>  
>  	mod = setup_load_info(info, flags);
>  	if (IS_ERR(mod))
>  		return mod;
>  
> +	/* Has this module been blacklisted by the user? */
> +	list_for_each_entry(entry, &blacklisted_modules, next) {
> +		if (!strcmp(mod->name, entry->buf)) {
> +			pr_debug("module %s blacklisted\n", mod->name);
> +			return ERR_PTR(-EPERM);
> +		}
> +	}
> +
>  	err = check_modinfo(mod, info, flags);
>  	if (err)
>  		return ERR_PTR(err);
> -- 
> 1.7.9.3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Add kernel parameter to blacklist modules
  2016-06-13 21:23 ` Rusty Russell
@ 2016-06-14 17:15   ` Prarit Bhargava
  2016-06-14 21:20     ` Rusty Russell
  0 siblings, 1 reply; 9+ messages in thread
From: Prarit Bhargava @ 2016-06-14 17:15 UTC (permalink / raw)
  To: linux-kernel; +Cc: Prarit Bhargava, Jonathan Corbet, Rusty Russell, linux-doc

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@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: linux-doc@vger.kernel.org
---
 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;
+		}
+	} while (str);
+
+	return false;
+}
+core_param(module_blacklist, module_blacklist, charp, 0400);
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
 	/* Module within temporary copy. */
@@ -3165,6 +3187,9 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
 	if (IS_ERR(mod))
 		return mod;
 
+	if (blacklisted(mod->name))
+		return ERR_PTR(-EPERM);
+
 	err = check_modinfo(mod, info, flags);
 	if (err)
 		return ERR_PTR(err);
-- 
1.7.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add kernel parameter to blacklist modules
  2016-06-13 12:32 [PATCH] Add kernel parameter to blacklist modules Prarit Bhargava
  2016-06-13 21:23 ` Rusty Russell
@ 2016-06-14 17:17 ` Christoph Hellwig
  2016-06-14 17:55   ` Prarit Bhargava
  2016-06-14 20:51   ` Henrique de Moraes Holschuh
  1 sibling, 2 replies; 9+ messages in thread
From: Christoph Hellwig @ 2016-06-14 17:17 UTC (permalink / raw)
  To: Prarit Bhargava; +Cc: linux-kernel, Jonathan Corbet, Rusty Russell, linux-doc

On Mon, Jun 13, 2016 at 08:32:41AM -0400, Prarit Bhargava wrote:
> Blacklisting a module in linux has long been a problem.  The process of
> blacklisting a module has changed over time, and it seems that every OS
> does it slightly differently and depends on the age of the init system
> used on that OS.

And why would we care about blacklisting a module?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add kernel parameter to blacklist modules
  2016-06-14 17:17 ` [PATCH] " Christoph Hellwig
@ 2016-06-14 17:55   ` Prarit Bhargava
  2016-06-14 20:51   ` Henrique de Moraes Holschuh
  1 sibling, 0 replies; 9+ messages in thread
From: Prarit Bhargava @ 2016-06-14 17:55 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Jonathan Corbet, Rusty Russell, linux-doc



On 06/14/2016 01:17 PM, Christoph Hellwig wrote:
> On Mon, Jun 13, 2016 at 08:32:41AM -0400, Prarit Bhargava wrote:
>> Blacklisting a module in linux has long been a problem.  The process of
>> blacklisting a module has changed over time, and it seems that every OS
>> does it slightly differently and depends on the age of the init system
>> used on that OS.
> 
> And why would we care about blacklisting a module?

<rant>
I have a new system that I want to install that a customer could not install any
Linux OS on.  It's sitting on my desk right now because I finally had to get
them to send me the system.  And I *cannot* get the bloody thing to install
anything recent because the damned nouveau driver keeps blowing up in the latest
Fedora rawhide (which is pretty close to upstream), or install anyting older
because the storage isn't detected on older releases.  I have *no* way of
stopping the driver from loading so that I can at least start debugging, or
provide valuable debug information in a kernel.org bugzilla (or appropriate
mailing list).

So what I'm stuck with now is a very expensive (albeit pretty) paperweight.
</rant>

Sorry about the rant ... but I had to let it all out Christoph ... :)

Now, I am smart enough to install to a USB stick, remove the nouveau driver from
both the install image and the initramfs, play around with the myriad and random
BIOS settings on this laptop (I will note that these are different for EVERY
piece of hardware) to figure out how to get the BIOS or EFI and Secure Boot
settings to detect the USB stick, cross my fingers and stand on one leg while I
pray to $deity that the system boots.

I do NOT expect random user to follow those steps in order to install Fedora or
another other upstream-following Linux OS.  They won't, and users turn to other
OSes that "just work".  That does nothing for the community.  It does nothing to
improve bug reporting.  It does nothing to improve the kernel.

I'd much rather tell those users to add "module_blacklist=nouveau" and send me
some debug information so I can find out what the issue might be, and find
hardware similar to theirs to help them out.  I still might have to tell that
user to send me their hardware but that's always a last resort.

P.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add kernel parameter to blacklist modules
  2016-06-14 17:17 ` [PATCH] " Christoph Hellwig
  2016-06-14 17:55   ` Prarit Bhargava
@ 2016-06-14 20:51   ` Henrique de Moraes Holschuh
  2016-06-14 22:51     ` Prarit Bhargava
  1 sibling, 1 reply; 9+ messages in thread
From: Henrique de Moraes Holschuh @ 2016-06-14 20:51 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Prarit Bhargava, linux-kernel, Jonathan Corbet, Rusty Russell, linux-doc

On Tue, 14 Jun 2016, Christoph Hellwig wrote:
> On Mon, Jun 13, 2016 at 08:32:41AM -0400, Prarit Bhargava wrote:
> > Blacklisting a module in linux has long been a problem.  The process of
> > blacklisting a module has changed over time, and it seems that every OS
> > does it slightly differently and depends on the age of the init system
> > used on that OS.
> 
> And why would we care about blacklisting a module?

Because the "Current Best Practice" way to help users blacklist modules
that won't drag you to nasty places trying to ensure they did it right
(i.e. "mv" the .ko file away then trigger an initramfs update and
reboot) just covers situations where the system actually boots/installs
mostly fine in the first place.

So, yes, such a feature looks like it would be rather useful, to the
point that I'd even advocate for it to be backported (once it has been
in a released mainline kernel for a while to remove any risk of
regressions, of course).

And if such a module blacklist feature ends up being invoked by the
"nuke_module_from_orbit=<modules list>" parameter, I will pay the author
(and the subsystem maintainer that manages to get that merged) a couple
beers should we ever meet in real life :-)

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Add kernel parameter to blacklist modules
  2016-06-14 17:15   ` [PATCH v2] " Prarit Bhargava
@ 2016-06-14 21:20     ` Rusty Russell
  2016-06-15 13:38       ` Prarit Bhargava
  0 siblings, 1 reply; 9+ messages in thread
From: Rusty Russell @ 2016-06-14 21:20 UTC (permalink / raw)
  To: Prarit Bhargava, linux-kernel; +Cc: Prarit Bhargava, Jonathan Corbet, linux-doc

Prarit Bhargava <prarit@redhat.com> 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@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: linux-doc@vger.kernel.org
> ---
>  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;

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add kernel parameter to blacklist modules
  2016-06-14 20:51   ` Henrique de Moraes Holschuh
@ 2016-06-14 22:51     ` Prarit Bhargava
  0 siblings, 0 replies; 9+ messages in thread
From: Prarit Bhargava @ 2016-06-14 22:51 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh, Christoph Hellwig
  Cc: linux-kernel, Jonathan Corbet, Rusty Russell, linux-doc



On 06/14/2016 04:51 PM, Henrique de Moraes Holschuh wrote:

> 
> And if such a module blacklist feature ends up being invoked by the
> "nuke_module_from_orbit=<modules list>" parameter, I will pay the author
> (and the subsystem maintainer that manages to get that merged) a couple
> beers should we ever meet in real life :-)

v2 here: http://marc.info/?l=linux-kernel&m=146592456120073&w=2

P.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Add kernel parameter to blacklist modules
  2016-06-14 21:20     ` Rusty Russell
@ 2016-06-15 13:38       ` Prarit Bhargava
  0 siblings, 0 replies; 9+ messages in thread
From: Prarit Bhargava @ 2016-06-15 13:38 UTC (permalink / raw)
  To: Rusty Russell, linux-kernel; +Cc: Jonathan Corbet, linux-doc



On 06/14/2016 05:20 PM, Rusty Russell wrote:
> Prarit Bhargava <prarit@redhat.com> 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@redhat.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: linux-doc@vger.kernel.org
>> ---
>>  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.
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-15 13:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 12:32 [PATCH] Add kernel parameter to blacklist modules Prarit Bhargava
2016-06-13 21:23 ` Rusty Russell
2016-06-14 17:15   ` [PATCH v2] " Prarit Bhargava
2016-06-14 21:20     ` Rusty Russell
2016-06-15 13:38       ` Prarit Bhargava
2016-06-14 17:17 ` [PATCH] " Christoph Hellwig
2016-06-14 17:55   ` Prarit Bhargava
2016-06-14 20:51   ` Henrique de Moraes Holschuh
2016-06-14 22:51     ` Prarit Bhargava

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).