[RFC PATCH] kmod: add whitelist option

I don't really agree with the "surprise enablement" use case presented.
There aren't really any negatives presented that explain what sort of
problems it avoids. Generally, it seems like a bit of an edge case --
one where the user might as well just be compiling their own kernel with
a particular subsystem (v4l, 802.11, bluetooth) removed to prevent this
sort of thing.

On the security concern, isn't this what signed modules, and in
particular, CONFIG_MODULE_SIG_FORCE is meant to provide?

I see downsides to this feature, because it's somewhat of a snake pit.
If you misuse it, you may render your machine unbootable. Modules can
(and have) changed names. I think the proper way to do this would be to
whitelist by modalias, but that isn't possible with this patch.

In the end, the decision isn't up to me, but this seems a bit strange.

I've left a few minor comments below in the code.

Cheers,
Dave

Post by Milan Broz
---
Makefile.am | 4 +-
libkmod/docs/libkmod-sections.txt | 1 +
libkmod/libkmod-config.c | 72 ++++++++++++-
libkmod/libkmod-module.c | 43 +++++++-
libkmod/libkmod-private.h | 3 +-
libkmod/libkmod.h | 3 +
libkmod/libkmod.sym | 5 +
man/modprobe.d.xml | 27 +++++
.../test-whitelist/etc/modprobe.d/modprobe.conf | 2 +
testsuite/test-whitelist.c | 114 ++++++++++++++++++++
tools/modprobe.c | 12 ++-
11 files changed, 275 insertions(+), 11 deletions(-)
create mode 100644 testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf
create mode 100644 testsuite/test-whitelist.c
diff --git a/Makefile.am b/Makefile.am
index e4c1a83..03aa421 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -176,7 +176,7 @@ testsuite_libtestsuite_la_LIBADD = -lrt
TESTSUITE = testsuite/test-init testsuite/test-testsuite testsuite/test-loaded \
testsuite/test-modinfo testsuite/test-alias testsuite/test-new-module \
- testsuite/test-modprobe testsuite/test-blacklist \
+ testsuite/test-modprobe testsuite/test-blacklist testsuite/test-whitelist \
testsuite/test-dependencies testsuite/test-depmod
check_PROGRAMS = $(TESTSUITE)
@@ -198,6 +198,8 @@ testsuite_test_modprobe_LDADD = $(TESTSUITE_LDADD)
testsuite_test_modprobe_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
testsuite_test_blacklist_LDADD = $(TESTSUITE_LDADD)
testsuite_test_blacklist_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
+testsuite_test_whitelist_LDADD = $(TESTSUITE_LDADD)
+testsuite_test_whitelist_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
testsuite_test_dependencies_LDADD = $(TESTSUITE_LDADD)
testsuite_test_dependencies_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
testsuite_test_depmod_LDADD = $(TESTSUITE_LDADD)
diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
index e59ab7a..bf7c0d9 100644
--- a/libkmod/docs/libkmod-sections.txt
+++ b/libkmod/docs/libkmod-sections.txt
@@ -31,6 +31,7 @@ kmod_list_prev
<FILE>libkmod-config</FILE>
kmod_config_iter
kmod_config_get_blacklists
+kmod_config_get_whitelists
kmod_config_get_install_commands
kmod_config_get_remove_commands
kmod_config_get_aliases
diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index 398468e..f1f1181 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -56,7 +56,7 @@ struct kmod_softdep {
unsigned int n_post;
};
-const char *kmod_blacklist_get_modname(const struct kmod_list *l)
+const char *kmod_list_get_modname(const struct kmod_list *l)
{
return l->data;
}
@@ -303,6 +303,38 @@ static void kmod_config_free_blacklist(struct kmod_config *config,
config->blacklists = kmod_list_remove(l);
}
+static int kmod_config_add_whitelist(struct kmod_config *config,
+ const char *modname)
+{
+ char *p;
+ struct kmod_list *list;
+
+ DBG(config->ctx, "modname=%s\n", modname);
+
+ p = strdup(modname);
+ if (!p)
+ goto oom_error_init;
+
+ list = kmod_list_append(config->whitelists, p);
+ if (!list)
+ goto oom_error;
+ config->whitelists = list;
+ return 0;
+
+ free(p);
+ ERR(config->ctx, "out-of-memory modname=%s\n", modname);
+ return -ENOMEM;
+}
+
+static void kmod_config_free_whitelist(struct kmod_config *config,
+ struct kmod_list *l)
+{
+ free(l->data);
+ config->whitelists = kmod_list_remove(l);
+}
+
static int kmod_config_add_softdep(struct kmod_config *config,
const char *modname,
const char *line)
@@ -634,6 +666,14 @@ static int kmod_config_parse(struct kmod_config *config, int fd,
kmod_config_add_blacklist(config,
underscores(ctx, modname));
+ } else if (streq(cmd, "whitelist")) {
+ char *modname = strtok_r(NULL, "\t ", &saveptr);
+
+ if (modname == NULL)
+ goto syntax_error;
+
+ kmod_config_add_whitelist(config,
+ underscores(ctx, modname));
} else if (streq(cmd, "options")) {
char *modname = strtok_r(NULL, "\t ", &saveptr);
char *options = strtok_r(NULL, "\0", &saveptr);
@@ -703,6 +743,9 @@ void kmod_config_free(struct kmod_config *config)
while (config->blacklists)
kmod_config_free_blacklist(config, config->blacklists);
+ while (config->whitelists)
+ kmod_config_free_whitelist(config, config->whitelists);
+
while (config->options)
kmod_config_free_options(config, config->options);
@@ -955,6 +998,7 @@ enum config_type {
CONFIG_TYPE_ALIAS,
CONFIG_TYPE_OPTION,
CONFIG_TYPE_SOFTDEP,
+ CONFIG_TYPE_WHITELIST,
};
struct kmod_config_iter {
@@ -987,7 +1031,11 @@ static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
switch (type) {
iter->list = config->blacklists;
- iter->get_key = kmod_blacklist_get_modname;
+ iter->get_key = kmod_list_get_modname;
+ break;
+ iter->list = config->whitelists;
+ iter->get_key = kmod_list_get_modname;
break;
iter->list = config->install_commands;
@@ -1046,6 +1094,26 @@ KMOD_EXPORT struct kmod_config_iter *kmod_config_get_blacklists(const struct kmo
}
/**
+ *
+ * Retrieve an iterator to deal with the whitelist maintained inside the
+ * library. See kmod_config_iter_get_key(), kmod_config_iter_get_value() and
+ * kmod_config_iter_next(). At least one call to kmod_config_iter_next() must
+ * be made to initialize the iterator and check if it's valid.
+ *
+ * Returns: a new iterator over the whitelists or NULL on failure. Free it
+ * with kmod_config_iter_free_iter().
+ */
+KMOD_EXPORT struct kmod_config_iter *kmod_config_get_whitelists(const struct kmod_ctx *ctx)
+{
+ if (ctx == NULL)
+ return NULL;
+
+ return kmod_config_iter_new(ctx, CONFIG_TYPE_WHITELIST);
+}
+
+/**
*
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 0d87ce1..cf68fae 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -850,7 +850,7 @@ static bool module_is_blacklisted(struct kmod_module *mod)
const struct kmod_list *l;
kmod_list_foreach(l, bl) {
- const char *modname = kmod_blacklist_get_modname(l);
+ const char *modname = kmod_list_get_modname(l);
if (streq(modname, mod->name))
return true;
@@ -859,6 +859,26 @@ static bool module_is_blacklisted(struct kmod_module *mod)
return false;
}
+static bool module_is_not_whitelisted(struct kmod_module *mod)

I strongly suggest that this be inverted and the call simply use
!module_is_whitelisted(module)

Post by Milan Broz
+{
+ struct kmod_ctx *ctx = mod->ctx;
+ const struct kmod_config *config = kmod_get_config(ctx);
+ const struct kmod_list *wl = config->whitelists;
+ const struct kmod_list *l;
+
+ if (!wl)
+ return false;
+
+ kmod_list_foreach(l, wl) {
+ const char *modname = kmod_list_get_modname(l);
+
+ if (streq(modname, mod->name))
+ return false;
+ }
+
+ return true;
+}
+
/**
* kmod_module_apply_filter
@@ -897,6 +917,10 @@ KMOD_EXPORT int kmod_module_apply_filter(const struct kmod_ctx *ctx,
if ((filter_type & KMOD_FILTER_BUILTIN) && mod->builtin)
continue;
+ if ((filter_type & KMOD_FILTER_WHITELIST) &&
+ module_is_not_whitelisted(mod))
+ continue;
+
node = kmod_list_append(*output, mod);
if (node == NULL)
goto fail;
@@ -1143,7 +1167,7 @@ static int kmod_module_get_probe_list(struct kmod_module *mod,
* output or in dry-run mode.
*
* Insert a module in Linux kernel resolving dependencies, soft dependencies,
- * install commands and applying blacklist.
+ * install commands and applying blacklist and whitelist.

Might be nice to generalize this and just say "applying filters" to make
it a bit more future proof.

Post by Milan Broz
*
@@ -1163,6 +1187,7 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod,
const char *options))
{
struct kmod_list *list = NULL, *l;
+ struct kmod_list *filtered = NULL;
struct probe_insert_cb cb;
int err;
@@ -1195,8 +1220,20 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod,
if (err < 0)
return err;
+ /* Removes modules NOT in whitelist */
+ err = kmod_module_apply_filter(mod->ctx,
+ KMOD_FILTER_WHITELIST, list, &filtered);
+ if (err < 0)
+ return err;
+
+ kmod_module_unref_list(list);
+ if (filtered == NULL)
+ return KMOD_PROBE_APPLY_WHITELIST;
+
+ list = filtered;
+ filtered = NULL;
+
if (flags & KMOD_PROBE_APPLY_BLACKLIST_ALL) {
- struct kmod_list *filtered = NULL;
err = kmod_module_apply_filter(mod->ctx,
KMOD_FILTER_BLACKLIST, list, &filtered);
diff --git a/libkmod/libkmod-private.h b/libkmod/libkmod-private.h
index 4760733..0651d52 100644
--- a/libkmod/libkmod-private.h
+++ b/libkmod/libkmod-private.h
@@ -102,6 +102,7 @@ struct kmod_config {
struct kmod_ctx *ctx;
struct kmod_list *aliases;
struct kmod_list *blacklists;
+ struct kmod_list *whitelists;
struct kmod_list *options;
struct kmod_list *remove_commands;
struct kmod_list *install_commands;
@@ -112,7 +113,7 @@ struct kmod_config {
int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **config, const char * const *config_paths) __attribute__((nonnull(1, 2,3)));
void kmod_config_free(struct kmod_config *config) __attribute__((nonnull(1)));
-const char *kmod_blacklist_get_modname(const struct kmod_list *l) __attribute__((nonnull(1)));
+const char *kmod_list_get_modname(const struct kmod_list *l) __attribute__((nonnull(1)));
const char *kmod_alias_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
const char *kmod_alias_get_modname(const struct kmod_list *l) __attribute__((nonnull(1)));
const char *kmod_option_get_options(const struct kmod_list *l) __attribute__((nonnull(1)));
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index d03ab19..18721f3 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -106,6 +106,7 @@ struct kmod_list *kmod_list_last(const struct kmod_list *list);
*/
struct kmod_config_iter;
struct kmod_config_iter *kmod_config_get_blacklists(const struct kmod_ctx *ctx);
+struct kmod_config_iter *kmod_config_get_whitelists(const struct kmod_ctx *ctx);
struct kmod_config_iter *kmod_config_get_install_commands(const struct kmod_ctx *ctx);
struct kmod_config_iter *kmod_config_get_remove_commands(const struct kmod_ctx *ctx);
struct kmod_config_iter *kmod_config_get_aliases(const struct kmod_ctx *ctx);
@@ -162,12 +163,14 @@ enum kmod_probe {
KMOD_PROBE_APPLY_BLACKLIST_ALL = 0x10000,
KMOD_PROBE_APPLY_BLACKLIST = 0x20000,
KMOD_PROBE_APPLY_BLACKLIST_ALIAS_ONLY = 0x40000,
+ KMOD_PROBE_APPLY_WHITELIST = 0x80000,
};
/* Flags to kmod_module_apply_filter() */
enum kmod_filter {
KMOD_FILTER_BLACKLIST = 0x00001,
KMOD_FILTER_BUILTIN = 0x00002,
+ KMOD_FILTER_WHITELIST = 0x00004,
};
int kmod_module_remove_module(struct kmod_module *mod, unsigned int flags);
diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
index 854d257..28c8462 100644
--- a/libkmod/libkmod.sym
+++ b/libkmod/libkmod.sym
@@ -86,3 +86,8 @@ LIBKMOD_6 {
kmod_module_apply_filter;
} LIBKMOD_5;
+
+LIBKMOD_11 {
+ kmod_config_get_whitelists;
+} LIBKMOD_6;
diff --git a/man/modprobe.d.xml b/man/modprobe.d.xml
index dc19b23..2140c9b 100644
--- a/man/modprobe.d.xml
+++ b/man/modprobe.d.xml
@@ -112,6 +112,33 @@
</listitem>
</varlistentry>
<varlistentry>
+ <term>whitelist <replaceable>modulename</replaceable>
+ </term>
+ <listitem>
+ <para>
+ If at least one <command>whitelist</command> command is specified,
+ module loading and scripts specified
+ using <command>install</command> are limited to module names
+ matching a glob <replaceable>pattern</replaceable> specified in one
+ of the <command>whitelist</command> commands.
+ </para>
+ <para>
+ You can prepare whitelist of currently loaded modules e.g. by this
+ </para>
+ <para>
+ lsmod | tail -n +2 | cut -d ' ' -f 1 | sed 's/^/whitelist /'
+ </para>
+ <para>
+ Note that the <command>whitelist</command> command is not a direct
+ opposite of the <command>blacklist</command> command.
+ The <command>blacklist</command> command affects the selection
+ of a module to be loaded or <command>install</command> script to
+ be performed.
+ </para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
<term>install <replaceable>modulename</replaceable> <replaceable>command...</replaceable>
</term>
<listitem>
diff --git a/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf b/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf
new file mode 100644
index 0000000..9dc0ced
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf
@@ -0,0 +1,2 @@
+whitelist floppy
+whitelist pcspkr
diff --git a/testsuite/test-whitelist.c b/testsuite/test-whitelist.c
new file mode 100644
index 0000000..49a3082
--- /dev/null
+++ b/testsuite/test-whitelist.c
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2011-2012 ProFUSION embedded systems
+ * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <errno.h>
+#include <unistd.h>
+#include <inttypes.h>
+#include <string.h>
+#include <libkmod.h>
+
+/* good luck bulding a kmod_list outside of the library... makes this blacklist
+ * function rather pointless */
+#include <libkmod-private.h>
+
+/* FIXME: hack, change name so we don't clash */
+#undef ERR
+#include "testsuite.h"
+
+static int whitelist_1(const struct test *t)
+{
+ struct kmod_ctx *ctx;
+ struct kmod_list *list = NULL, *l, *filtered;
+ struct kmod_module *mod;
+ int err;
+ size_t len = 0;
+
+ const char *names[] = { "pcspkr", "pcspkr2", "floppy", "ext4", NULL };
+ const char **name;
+
+ ctx = kmod_new(NULL, NULL);
+ if (ctx == NULL)
+ exit(EXIT_FAILURE);
+
+ for(name = names; *name; name++) {
+ err = kmod_module_new_from_name(ctx, *name, &mod);
+ if (err < 0)
+ goto fail_lookup;
+ list = kmod_list_append(list, mod);
+ }
+
+ err = kmod_module_apply_filter(ctx, KMOD_FILTER_WHITELIST, list,
+ &filtered);
+ if (err < 0) {
+ ERR("Could not filter: %s\n", strerror(-err));
+ goto fail;
+ }
+ if (filtered == NULL) {
+ ERR("All modules were filtered out!\n");
+ goto fail;
+ }
+
+ kmod_list_foreach(l, filtered) {
+ const char *modname;
+ mod = kmod_module_get_module(l);
+ modname = kmod_module_get_name(mod);
+ /* These are not on whitelist, must be rejected */
+ if (strcmp("pcspkr2", modname) == 0 || strcmp("ext4", modname) == 0)
+ goto fail;
+ /* These are on whitelist, must be in list */
+ if (strcmp("pcspkr", modname) && strcmp("floppy", modname))
+ goto fail;
+ len++;
+ kmod_module_unref(mod);
+ }
+
+ if (len != 2)
+ goto fail;
+
+ kmod_module_unref_list(filtered);
+ kmod_module_unref_list(list);
+ kmod_unref(ctx);
+
+ return EXIT_SUCCESS;
+
+ kmod_module_unref_list(list);
+ kmod_unref(ctx);
+ return EXIT_FAILURE;
+}
+static const struct test swhitelist_1 = {
+ .name = "whitelist_1",
+ .description = "check if modules are correctly whitelisted",
+ .func = whitelist_1,
+ .config = {
+ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-whitelist/",
+ },
+ .need_spawn = true,
+};
+
+static const struct test *tests[] = {
+ &swhitelist_1,
+ NULL,
+};
+
+TESTSUITE_MAIN(tests);
diff --git a/tools/modprobe.c b/tools/modprobe.c
index 58f6df9..81b8442 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -638,10 +638,14 @@ static int insmod(struct kmod_ctx *ctx, const char *alias,
extra_options, NULL, NULL, show);
}
- if (err >= 0)
- /* ignore flag return values such as a mod being blacklisted */
- err = 0;
- else {
+ if (err >= 0) {
+ if (err & KMOD_PROBE_APPLY_WHITELIST)
+ ERR("could not insert '%s': Module not on whitelist\n",
+ kmod_module_get_name(mod));
+ else
+ /* ignore flag return values such as a mod being blacklisted */
+ err = 0;
+ } else {
switch (err) {
ERR("could not insert '%s': Module already in kernel\n",
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

Milan Broz

2012-11-06 15:20:22 UTC

Hi,

Yes, I fully agree, in ideal world I will not compile modules into kernel
at all if I do not want them.

But the problem comes from enterprise RHEL world - you have stock kernel
with some set of modules (and you cannot or do not want to compile own,
the reason can be lost of support, certification etc).

And some customers want to harden system using several ways.

Here, at least as I understand it, it means "do not allow loading modules
which handle USB/speaker/microphone/... whatever you can imagine.

It obviously does not solve problem with modules compiled-in kernel.

I know that there are other ways (udev rules, selinux etc) but security is about
multiple barriers and despite this is not useful for most of the people,
I think it can be useful for others.

Post by Dave Reisner
On the security concern, isn't this what signed modules, and in
particular, CONFIG_MODULE_SIG_FORCE is meant to provide?

This is not about signature (all modules are signed), just about limiting
existing module use.

Post by Dave Reisner
I see downsides to this feature, because it's somewhat of a snake pit.
If you misuse it, you may render your machine unbootable.

That's true, but the same applies for other security barriers as well.
They need to prepare proper whitelist to use it properly.

Post by Dave Reisner
Modules can
(and have) changed names. I think the proper way to do this would be to
whitelist by modalias, but that isn't possible with this patch.

I can imagine one of the use case is masking some exact crypto modules
(like a hw accelerated ones) while keeping other implementation loadable,
so alias is not solution here. (Alias would mask all implementations.)

Post by Milan Broz
+static bool module_is_not_whitelisted(struct kmod_module *mod)

I strongly suggest that this be inverted and the call simply use
!module_is_whitelisted(module)

hehe, it was exactly this way and I changed it because this was more
clear to me :) np, I'll change it back.

Post by Milan Broz
@@ -1143,7 +1167,7 @@ static int kmod_module_get_probe_list(struct kmod_module *mod,
* output or in dry-run mode.
*
* Insert a module in Linux kernel resolving dependencies, soft dependencies,
- * install commands and applying blacklist.
+ * install commands and applying blacklist and whitelist.

Might be nice to generalize this and just say "applying filters" to make
it a bit more future proof.

ok.

Whatever, if the patch is acceptable, let me know, I'll resend fixed version.

Thanks for comments!
Milan

Lucas De Marchi

2012-11-06 17:24:46 UTC

Hi Milan,

Post by Milan Broz

Besides the ugly fact of trying to vendor-lock-in the user, what is a
whitelist in kmod really helping? User could just remove the whitelist
from the config.

Really, if this patch is all about vendor-lock-in (regardless if it's
an open source friendly company), you already got my nack.

Post by Milan Broz
And some customers want to harden system using several ways.

What's impeding them to remove the modules they don't ever want to
load from /lib/modules/ and rerun depmod? The whitelist would be much
more effective in a modules_install rule -> only install modules that
are in the whitelist.

I'll think about it, but in a first look I really don't see any benefit.

Lucas De Marchi

Josh Boyer

2012-11-06 17:44:44 UTC

On Tue, Nov 6, 2012 at 12:24 PM, Lucas De Marchi

Post by Lucas De Marchi
Hi Milan,

Post by Milan Broz

Besides the ugly fact of trying to vendor-lock-in the user, what is a
whitelist in kmod really helping? User could just remove the whitelist
from the config.
Really, if this patch is all about vendor-lock-in (regardless if it's
an open source friendly company), you already got my nack.

I won't comment on the mechanism itself, but I don't see how this is
implementing vendor-lock-in at all. It's a generic mechanism to
whitelist which modules can be loaded on a system regardless of which
kernel vendor those modules come from.

Post by Lucas De Marchi
From what I understand, the patch has nothing to do with vendor-lock-in.

It seems to be about giving admins more control over which modules can
be loaded, particularly in the presence of hotplug mechanisms that can
auto-load drivers for various devices when they're plugged in.

(For full disclosure, I'm posting from a gmail address but I am a
Red Hat employee. I do not speak for Red Hat and my views are my own.)

josh

Lucas De Marchi

2012-11-06 17:50:10 UTC

Post by Josh Boyer
On Tue, Nov 6, 2012 at 12:24 PM, Lucas De Marchi

Post by Lucas De Marchi
Hi Milan,

Post by Milan Broz

Besides the ugly fact of trying to vendor-lock-in the user, what is a
whitelist in kmod really helping? User could just remove the whitelist
from the config.
Really, if this patch is all about vendor-lock-in (regardless if it's
an open source friendly company), you already got my nack.

I was answering his comment about what I understood be an argument
saying this was about a vendor-lock-in mechanism. I replied saying
that even if this was the case, it would still fail, because the
whitelist does not prevent the user .

Post by Josh Boyer
From what I understand, the patch has nothing to do with vendor-lock-in.

I hope so, but his comment triggered my reaction ;-)

Post by Josh Boyer
It seems to be about giving admins more control over which modules can
be loaded, particularly in the presence of hotplug mechanisms that can
auto-load drivers for various devices when they're plugged in.

then how is this any better than a whitelist in which modules get installed?

Lucas De Marchi

Josh Boyer

2012-11-06 17:59:08 UTC

On Tue, Nov 6, 2012 at 12:50 PM, Lucas De Marchi

then how is this any better than a whitelist in which modules get installed?

Well, I didn't say it was. However, from a distribution perspective
you can do one of the following with a common distribution kernel RPM:

1) Install kernel RPM; remove modules from /lib/modules after the fact.
2) Code something into the RPM %post script that looks at a whitelist
on the machine and automatically removes modules not in that list
after the install (really a refinement of option 1).
3) Blacklist every module except those you want to allow to be loaded
(opposite of the proposed solution).
4) Add a whitelist to kmod.
5) Something I haven't thought of.

Option 1 is kind of ugly to have to do by hand. Option 2 is slightly
more feasible. Both of them would then cause RPM's idea of what a
package provides to be somewhat broken though (e.g. rpm -V would fail).

Option 3 is fairly tedious to do by hand for a sysadmin.

Option 4 is what has been proposed. It has positives and negatives
and I'm sure that is what Milan is looking to discuss so I'll leave
that be.

Option 5 is clearly beyond my capacity of thinking at the moment ;).

So, there are multiple ways to solve this. I just wanted to make sure
the proposed option wasn't being viewed as a lock-in mechanism.

josh

Lucas De Marchi

2012-11-06 18:43:54 UTC

CC'ing Jon Masters

Post by Josh Boyer
On Tue, Nov 6, 2012 at 12:50 PM, Lucas De Marchi

then how is this any better than a whitelist in which modules get installed?

Well, I didn't say it was. However, from a distribution perspective
1) Install kernel RPM; remove modules from /lib/modules after the fact.
2) Code something into the RPM %post script that looks at a whitelist
on the machine and automatically removes modules not in that list
after the install (really a refinement of option 1).
3) Blacklist every module except those you want to allow to be loaded
(opposite of the proposed solution).
4) Add a whitelist to kmod.
5) Something I haven't thought of.
Option 1 is kind of ugly to have to do by hand. Option 2 is slightly
more feasible. Both of them would then cause RPM's idea of what a
package provides to be somewhat broken though (e.g. rpm -V would fail).
Option 3 is fairly tedious to do by hand for a sysadmin.
Option 4 is what has been proposed. It has positives and negatives
and I'm sure that is what Milan is looking to discuss so I'll leave
that be.
Option 5 is clearly beyond my capacity of thinking at the moment ;).
So, there are multiple ways to solve this. I just wanted to make sure
the proposed option wasn't being viewed as a lock-in mechanism.

Ok. I kind of see the point here. I was worried about the arguments
for it though.

Jon, it seems it was a feature request for module-init-tools ~ 2.5
years ago. Any reason why it was not accepted back then?

Lucas De Marchi

Josh Boyer

2012-12-11 19:51:16 UTC

On Tue, Nov 6, 2012 at 1:43 PM, Lucas De Marchi

Post by Lucas De Marchi
CC'ing Jon Masters

Post by Josh Boyer
On Tue, Nov 6, 2012 at 12:50 PM, Lucas De Marchi

then how is this any better than a whitelist in which modules get installed?

Ok. I kind of see the point here. I was worried about the arguments
for it though.
Jon, it seems it was a feature request for module-init-tools ~ 2.5
years ago. Any reason why it was not accepted back then?

I think you're on your own as the kmod maintainer on this one, Lucas.
Jon is rather busy and hasn't commented on this in 2.5 years. I don't
think waiting around for him to do so now is going to serve anyone's
best interests at this point.

I'm sure Milan would be happy to discuss further if you have more
questions.

josh

Jon Masters

2012-12-11 20:14:09 UTC

Post by Josh Boyer
On Tue, Nov 6, 2012 at 1:43 PM, Lucas De Marchi

Post by Lucas De Marchi
CC'ing Jon Masters

Post by Josh Boyer
On Tue, Nov 6, 2012 at 12:50 PM, Lucas De Marchi

then how is this any better than a whitelist in which modules get installed?

Ok. I kind of see the point here. I was worried about the arguments
for it though.
Jon, it seems it was a feature request for module-init-tools ~ 2.5
years ago. Any reason why it was not accepted back then?

All true points :) But specifically on why whitelisting didn't happen,
because it was contentious and the approach of just having a whitelist
file wasn't going to scale. That would mean that a new kernel changing
the name of a driver or adding a new driver, or even buying some new
hardware would require updates to the whitelist files. I understand this
is something that some very security sensitive folks want to do, but I
don't personally feel a whitelist is the way to achieve it.

However, I'll let you guys solve this one :)

Jon.

Josh Boyer

2013-03-21 13:21:31 UTC

On Tue, Nov 6, 2012 at 1:43 PM, Lucas De Marchi

Post by Lucas De Marchi
CC'ing Jon Masters

Post by Josh Boyer
On Tue, Nov 6, 2012 at 12:50 PM, Lucas De Marchi

then how is this any better than a whitelist in which modules get installed?

Ok. I kind of see the point here. I was worried about the arguments
for it though.

Reviving an old thread here...

Lucas, did you decide on whether or not this functionality was acceptable
upstream? If so, I can repost the patch against latest git. There are
security minded people that would like to see this available, and given
that it is entirely optional I don't see much in the way of burden. I
can make a note that people will need to be diligent in maintaining their
whitelist if they use one.

Let me know how you want to proceed.

josh

Lucas De Marchi

2013-03-22 14:33:29 UTC

Hi Josh,

Post by Josh Boyer
On Tue, Nov 6, 2012 at 1:43 PM, Lucas De Marchi

Post by Lucas De Marchi
CC'ing Jon Masters

Post by Josh Boyer
On Tue, Nov 6, 2012 at 12:50 PM, Lucas De Marchi

then how is this any better than a whitelist in which modules get installed?

Ok. I kind of see the point here. I was worried about the arguments
for it though.

Reviving an old thread here...
Lucas, did you decide on whether or not this functionality was acceptable
upstream? If so, I can repost the patch against latest git. There are
security minded people that would like to see this available, and given
that it is entirely optional I don't see much in the way of burden. I
can make a note that people will need to be diligent in maintaining their
whitelist if they use one.
Let me know how you want to proceed.

I'm no huge fan of this feature. Indeed I think it's a pretty broken
feature (just not as broken as the install rules we need to carry for
compatibility reasons). I also think that for people that needs this a
custom kernel with things compiled-in would be way better.

That said, I will give you guys the opportunity to shoot yourselves on
the foot. Please rebase the patch. I'm sending another email
commenting on somethings I'd like to see in this patch before
applying.

Lucas De Marchi

Tom Gundersen

2013-03-22 16:18:34 UTC

On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

Post by Lucas De Marchi
I'm no huge fan of this feature. Indeed I think it's a pretty broken
feature (just not as broken as the install rules we need to carry for
compatibility reasons). I also think that for people that needs this a
custom kernel with things compiled-in would be way better.

Just to add my two cents to the 'this is a bad idea'-choir: This
feature seems to be at the wrong level of the stack. There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).

Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.

Of course, if Lucas wants to maintain this feature that is his call :-)

Cheers,

Tom

Lucas De Marchi

2013-03-22 16:49:39 UTC

Post by Tom Gundersen
On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

Just to add my two cents to the 'this is a bad idea'-choir: This
feature seems to be at the wrong level of the stack. There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).
Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.
Of course, if Lucas wants to maintain this feature that is his call :-)

So, after actually reviewing the code and not only the idea behind I
came to conclusion this is actually worse than I expected.

But I don't think having this in kernel and maintaining a list of
whitelisted module is good - otherwise it would be easier to just have
modules compiled statically. And if the kernel is going to call
userspace to allow/deny module loading, how this is supposed to be?
Another helper?

Josh/Milan, could you come up with an alternative solution to what you
are trying to accomplish? It seems to be a very limited use-case
(hardened systems) that may hurt others that don't want this,
rendering their machines unbootable.

thanks.
Lucas De Marchi

Daniel Dadap

2013-03-22 18:46:25 UTC

Post by Tom Gundersen
On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

Just to add my two cents to the 'this is a bad idea'-choir: This
feature seems to be at the wrong level of the stack. There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).
Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.
Of course, if Lucas wants to maintain this feature that is his call :-)

So, after actually reviewing the code and not only the idea behind I
came to conclusion this is actually worse than I expected.
But I don't think having this in kernel and maintaining a list of
whitelisted module is good - otherwise it would be easier to just have
modules compiled statically. And if the kernel is going to call
userspace to allow/deny module loading, how this is supposed to be?
Another helper?
Josh/Milan, could you come up with an alternative solution to what you
are trying to accomplish? It seems to be a very limited use-case
(hardened systems) that may hurt others that don't want this,
rendering their machines unbootable.

If the goal is to harden a system by allowing only certain modules to
load, then how about using CONFIG_MODULE_SIG_FORCE, and only signing
modules on the whitelist, and keeping kmod out of it completely? That
seems like exactly the kind of use case that CONFIG_MODULE_SIG_FORCE
would be good for.

Post by Lucas De Marchi
thanks.
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

Josh Boyer

2013-03-22 19:02:15 UTC

Post by Tom Gundersen
On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

Just to add my two cents to the 'this is a bad idea'-choir: This
feature seems to be at the wrong level of the stack. There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).
Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.
Of course, if Lucas wants to maintain this feature that is his call :-)

So, after actually reviewing the code and not only the idea behind I
came to conclusion this is actually worse than I expected.
But I don't think having this in kernel and maintaining a list of
whitelisted module is good - otherwise it would be easier to just have
modules compiled statically. And if the kernel is going to call
userspace to allow/deny module loading, how this is supposed to be?
Another helper?
Josh/Milan, could you come up with an alternative solution to what you
are trying to accomplish? It seems to be a very limited use-case
(hardened systems) that may hurt others that don't want this,
rendering their machines unbootable.

If the goal is to harden a system by allowing only certain modules to load,
then how about using CONFIG_MODULE_SIG_FORCE, and only signing modules on
the whitelist, and keeping kmod out of it completely? That seems like
exactly the kind of use case that CONFIG_MODULE_SIG_FORCE would be good for.

That's a fine suggestion for individuals building their own kernel. One
I would indeed recommend. It does not work for people using a distro
provided kernel but wanting to limit the modules loaded on their
individual systems.

Now, there is something almost the opposite they could do. If the distro
modules are signed, they could pass the enforcing parameter to the kernel
and strip the signature off of any modules they don't want loaded. Might
work.

josh

Lucas De Marchi

2013-03-22 19:01:52 UTC

Post by Tom Gundersen
On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

Just to add my two cents to the 'this is a bad idea'-choir: This
feature seems to be at the wrong level of the stack. There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).
Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.
Of course, if Lucas wants to maintain this feature that is his call :-)

So, after actually reviewing the code and not only the idea behind I
came to conclusion this is actually worse than I expected.
But I don't think having this in kernel and maintaining a list of
whitelisted module is good - otherwise it would be easier to just have
modules compiled statically. And if the kernel is going to call
userspace to allow/deny module loading, how this is supposed to be?
Another helper?
Josh/Milan, could you come up with an alternative solution to what you
are trying to accomplish? It seems to be a very limited use-case
(hardened systems) that may hurt others that don't want this,
rendering their machines unbootable.

If the goal is to harden a system by allowing only certain modules to load,
then how about using CONFIG_MODULE_SIG_FORCE, and only signing modules on
the whitelist, and keeping kmod out of it completely? That seems like
exactly the kind of use case that CONFIG_MODULE_SIG_FORCE would be good for.

Sure it's a better solution and I think the real solution is in this
direction. Dave even proposed this back last year. However Josh and
Milan raised the question that in a distro kernel, it's the distro
that's going to sign the modules. And all modules will had already be
signed upon package installation.

Removing the modules from /lib/modules will make the package manager yells.

But... the admin could just remove the signature from modules he
doesn't want (we can provide the tooling). And then apply the
CONFIG_MODULE_SIG_FORCE as you and Dave propose. Josh/Milan, what do
you think?

Lucas De Marchi

Josh Boyer

2013-03-22 19:04:58 UTC

On Fri, Mar 22, 2013 at 3:01 PM, Lucas De Marchi

Post by Tom Gundersen
On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

Just to add my two cents to the 'this is a bad idea'-choir: This
feature seems to be at the wrong level of the stack. There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).
Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.
Of course, if Lucas wants to maintain this feature that is his call :-)

So, after actually reviewing the code and not only the idea behind I
came to conclusion this is actually worse than I expected.
But I don't think having this in kernel and maintaining a list of
whitelisted module is good - otherwise it would be easier to just have
modules compiled statically. And if the kernel is going to call
userspace to allow/deny module loading, how this is supposed to be?
Another helper?
Josh/Milan, could you come up with an alternative solution to what you
are trying to accomplish? It seems to be a very limited use-case
(hardened systems) that may hurt others that don't want this,
rendering their machines unbootable.

If the goal is to harden a system by allowing only certain modules to load,
then how about using CONFIG_MODULE_SIG_FORCE, and only signing modules on
the whitelist, and keeping kmod out of it completely? That seems like
exactly the kind of use case that CONFIG_MODULE_SIG_FORCE would be good for.

Sure it's a better solution and I think the real solution is in this
direction. Dave even proposed this back last year. However Josh and
Milan raised the question that in a distro kernel, it's the distro
that's going to sign the modules. And all modules will had already be
signed upon package installation.
Removing the modules from /lib/modules will make the package manager yells.
But... the admin could just remove the signature from modules he
doesn't want (we can provide the tooling). And then apply the
CONFIG_MODULE_SIG_FORCE as you and Dave propose. Josh/Milan, what do
you think?

Great minds think alike. I just replied with the same thing.

It does seem feasible, but it depends on signed modules whereas the
current whitelist proposal works regardless of kernel options. It is
something to think about anyway.

josh

Josh Boyer

2013-03-22 19:07:08 UTC

Post by Tom Gundersen
On Fri, Mar 22, 2013 at 3:33 PM, Lucas De Marchi

You are not incorrect, however rebuilding the kernel isn't always an
option.

Post by Tom Gundersen
Could you do something similar to what was done with finit_module()
and the kernel_module_from_file hook? With the right security module
it seems like you should be able to catch all modules and verify that
they conform to whatever criterion you have.

That is also something to look into.

josh

Tom Gundersen

2013-03-22 19:10:24 UTC

Post by Josh Boyer

Post by Tom Gundersen
There is nothing
forcing you to use libkmod to load modules, so there would be no
guarantee that only the modules on the white-list can be loaded (i.e.,
adding this feature would not have the same guarantee as rebuilding
the kernel with only the whitelisted modules enabled, contrary to what
I guess one would expect?).

You are not incorrect, however rebuilding the kernel isn't always an
option.

Yes, so my understanding was that you want this white-listing feature
in order to get the same behavior as if you had rebuilt the kernel,
without actually having to do so. Is that correct? If so, then my
point was that you don't get the same behavior, as there are other
ways to load modules, so people might get a false sense of security...

-t

Josh Boyer

2013-03-22 19:17:50 UTC

Post by Tom Gundersen

Post by Josh Boyer

You are not incorrect, however rebuilding the kernel isn't always an
option.

That is arugably true. I would imagine some additional mechanisms are
necessary to ensure that the kmod tools are the only ones permitted to
call finit_module and init_module. Perhaps via SELinux.

josh

Milan Broz

2012-11-06 18:11:45 UTC

Post by Lucas De Marchi
Besides the ugly fact of trying to vendor-lock-in the user, what is a
whitelist in kmod really helping? User could just remove the whitelist
from the config.

This has nothing to do with vendor locking. The user cannot
modify whitelist because he has no privilege to do it.
(whitelist can be part of security policy and managed centrally for example.)

I mentioned RHEL, because I just explained where the request comes from.

Post by Lucas De Marchi
Really, if this patch is all about vendor-lock-in (regardless if it's
an open source friendly company), you already got my nack.

Sorry, but this was quite unfair. I really do not understand how this
can implement vendor lock in.

It is just another security mechanism, nothing more, nothing less.

Post by Milan Broz
And some customers want to harden system using several ways.

What's impeding them to remove the modules they don't ever want to
load from /lib/modules/ and rerun depmod?

Package verification will scream about missing and changed files
for example.

Post by Lucas De Marchi
The whitelist would be much
more effective in a modules_install rule -> only install modules that
are in the whitelist.

But sure, if I can manipulate with module_install, I can also change my
kernel config to not include these subsystems at all. But this is not
use case we are trying to solve here.

Milan

Milan Broz

2012-11-13 15:58:57 UTC

The whitelist option allows setup of hardened systems, only
modules on whitelist are allowed to load.

This can be useful when users cannot compile own kernel
(using distro compiled kernel) or cannot remove modules from
installation afterwards (because e.g. package verification issues).

This patch adds "whitelist" option and definition to libkmod
allowing to work with whitelist (similar to blacklist, except
whitelist, once defined, is unconditional).

Without defined whitelist, there is no change.

If at least one whitelist command is specified, module loading
is limited to module matching loaded whitelist.

For more context see bug
https://bugzilla.redhat.com/show_bug.cgi?id=560084
---
Makefile.am | 4 +-
libkmod/docs/libkmod-sections.txt | 1 +
libkmod/libkmod-config.c | 72 ++++++++++++-
libkmod/libkmod-module.c | 44 +++++++-
libkmod/libkmod-private.h | 3 +-
libkmod/libkmod.h | 3 +
libkmod/libkmod.sym | 5 +
man/modprobe.d.xml | 27 +++++
.../test-whitelist/etc/modprobe.d/modprobe.conf | 2 +
testsuite/test-whitelist.c | 114 ++++++++++++++++++++
tools/modprobe.c | 12 ++-
11 files changed, 276 insertions(+), 11 deletions(-)
create mode 100644 testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf
create mode 100644 testsuite/test-whitelist.c

diff --git a/Makefile.am b/Makefile.am
index 4865e52..7d100f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -176,7 +176,7 @@ testsuite_libtestsuite_la_LIBADD = -lrt

TESTSUITE = testsuite/test-init testsuite/test-testsuite testsuite/test-loaded \
testsuite/test-modinfo testsuite/test-alias testsuite/test-new-module \
- testsuite/test-modprobe testsuite/test-blacklist \
+ testsuite/test-modprobe testsuite/test-blacklist testsuite/test-whitelist \
testsuite/test-dependencies testsuite/test-depmod

check_PROGRAMS = $(TESTSUITE)
@@ -198,6 +198,8 @@ testsuite_test_modprobe_LDADD = $(TESTSUITE_LDADD)
testsuite_test_modprobe_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
testsuite_test_blacklist_LDADD = $(TESTSUITE_LDADD)
testsuite_test_blacklist_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
+testsuite_test_whitelist_LDADD = $(TESTSUITE_LDADD)
+testsuite_test_whitelist_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
testsuite_test_dependencies_LDADD = $(TESTSUITE_LDADD)
testsuite_test_dependencies_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
testsuite_test_depmod_LDADD = $(TESTSUITE_LDADD)
diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
index e59ab7a..bf7c0d9 100644
--- a/libkmod/docs/libkmod-sections.txt
+++ b/libkmod/docs/libkmod-sections.txt
@@ -31,6 +31,7 @@ kmod_list_prev
<FILE>libkmod-config</FILE>
kmod_config_iter
kmod_config_get_blacklists
+kmod_config_get_whitelists
kmod_config_get_install_commands
kmod_config_get_remove_commands
kmod_config_get_aliases
diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index 398468e..f1f1181 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -56,7 +56,7 @@ struct kmod_softdep {
unsigned int n_post;
};

-const char *kmod_blacklist_get_modname(const struct kmod_list *l)
+const char *kmod_list_get_modname(const struct kmod_list *l)
{
return l->data;
}
@@ -303,6 +303,38 @@ static void kmod_config_free_blacklist(struct kmod_config *config,
config->blacklists = kmod_list_remove(l);
}

+static int kmod_config_add_whitelist(struct kmod_config *config,
+ const char *modname)
+{
+ char *p;
+ struct kmod_list *list;
+
+ DBG(config->ctx, "modname=%s\n", modname);
+
+ p = strdup(modname);
+ if (!p)
+ goto oom_error_init;
+
+ list = kmod_list_append(config->whitelists, p);
+ if (!list)
+ goto oom_error;
+ config->whitelists = list;
+ return 0;
+
+oom_error:
+ free(p);
+oom_error_init:
+ ERR(config->ctx, "out-of-memory modname=%s\n", modname);
+ return -ENOMEM;
+}
+
+static void kmod_config_free_whitelist(struct kmod_config *config,
+ struct kmod_list *l)
+{
+ free(l->data);
+ config->whitelists = kmod_list_remove(l);
+}
+
static int kmod_config_add_softdep(struct kmod_config *config,
const char *modname,
const char *line)
@@ -634,6 +666,14 @@ static int kmod_config_parse(struct kmod_config *config, int fd,

kmod_config_add_blacklist(config,
underscores(ctx, modname));
+ } else if (streq(cmd, "whitelist")) {
+ char *modname = strtok_r(NULL, "\t ", &saveptr);
+
+ if (modname == NULL)
+ goto syntax_error;
+
+ kmod_config_add_whitelist(config,
+ underscores(ctx, modname));
} else if (streq(cmd, "options")) {
char *modname = strtok_r(NULL, "\t ", &saveptr);
char *options = strtok_r(NULL, "\0", &saveptr);
@@ -703,6 +743,9 @@ void kmod_config_free(struct kmod_config *config)
while (config->blacklists)
kmod_config_free_blacklist(config, config->blacklists);

+ while (config->whitelists)
+ kmod_config_free_whitelist(config, config->whitelists);
+
while (config->options)
kmod_config_free_options(config, config->options);

@@ -955,6 +998,7 @@ enum config_type {
CONFIG_TYPE_ALIAS,
CONFIG_TYPE_OPTION,
CONFIG_TYPE_SOFTDEP,
+ CONFIG_TYPE_WHITELIST,
};

struct kmod_config_iter {
@@ -987,7 +1031,11 @@ static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
switch (type) {
case CONFIG_TYPE_BLACKLIST:
iter->list = config->blacklists;
- iter->get_key = kmod_blacklist_get_modname;
+ iter->get_key = kmod_list_get_modname;
+ break;
+ case CONFIG_TYPE_WHITELIST:
+ iter->list = config->whitelists;
+ iter->get_key = kmod_list_get_modname;
break;
case CONFIG_TYPE_INSTALL:
iter->list = config->install_commands;
@@ -1046,6 +1094,26 @@ KMOD_EXPORT struct kmod_config_iter *kmod_config_get_blacklists(const struct kmo
}

/**
+ * kmod_config_get_whitelists:
+ * @ctx: kmod library context
+ *
+ * Retrieve an iterator to deal with the whitelist maintained inside the
+ * library. See kmod_config_iter_get_key(), kmod_config_iter_get_value() and
+ * kmod_config_iter_next(). At least one call to kmod_config_iter_next() must
+ * be made to initialize the iterator and check if it's valid.
+ *
+ * Returns: a new iterator over the whitelists or NULL on failure. Free it
+ * with kmod_config_iter_free_iter().
+ */
+KMOD_EXPORT struct kmod_config_iter *kmod_config_get_whitelists(const struct kmod_ctx *ctx)
+{
+ if (ctx == NULL)
+ return NULL;
+
+ return kmod_config_iter_new(ctx, CONFIG_TYPE_WHITELIST);
+}
+
+/**
* kmod_config_get_install_commands:
* @ctx: kmod library context
*
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 0d87ce1..ec33d31 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -850,7 +850,28 @@ static bool module_is_blacklisted(struct kmod_module *mod)
const struct kmod_list *l;

kmod_list_foreach(l, bl) {
- const char *modname = kmod_blacklist_get_modname(l);
+ const char *modname = kmod_list_get_modname(l);
+
+ if (streq(modname, mod->name))
+ return true;
+ }
+
+ return false;
+}
+
+static bool module_is_whitelisted(struct kmod_module *mod)
+{
+ struct kmod_ctx *ctx = mod->ctx;
+ const struct kmod_config *config = kmod_get_config(ctx);
+ const struct kmod_list *wl = config->whitelists;
+ const struct kmod_list *l;
+
+ /* No whitelist means all items are whitelisted */
+ if (!wl)
+ return true;
+
+ kmod_list_foreach(l, wl) {
+ const char *modname = kmod_list_get_modname(l);

if (streq(modname, mod->name))
return true;
@@ -897,6 +918,10 @@ KMOD_EXPORT int kmod_module_apply_filter(const struct kmod_ctx *ctx,
if ((filter_type & KMOD_FILTER_BUILTIN) && mod->builtin)
continue;

+ if ((filter_type & KMOD_FILTER_WHITELIST) &&
+ !module_is_whitelisted(mod))
+ continue;
+
node = kmod_list_append(*output, mod);
if (node == NULL)
goto fail;
@@ -1143,7 +1168,7 @@ static int kmod_module_get_probe_list(struct kmod_module *mod,
* output or in dry-run mode.
*
* Insert a module in Linux kernel resolving dependencies, soft dependencies,
- * install commands and applying blacklist.
+ * install commands and applying filters.
*
* If @run_install is NULL, this function will fork and exec by calling
* system(3). Don't pass a NULL argument in @run_install if your binary is
@@ -1163,6 +1188,7 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod,
const char *options))
{
struct kmod_list *list = NULL, *l;
+ struct kmod_list *filtered = NULL;
struct probe_insert_cb cb;
int err;

@@ -1195,8 +1221,20 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod,
if (err < 0)
return err;

+ /* Removes modules NOT in whitelist */
+ err = kmod_module_apply_filter(mod->ctx,
+ KMOD_FILTER_WHITELIST, list, &filtered);
+ if (err < 0)
+ return err;
+
+ kmod_module_unref_list(list);
+ if (filtered == NULL)
+ return KMOD_PROBE_APPLY_WHITELIST;
+
+ list = filtered;
+ filtered = NULL;
+
if (flags & KMOD_PROBE_APPLY_BLACKLIST_ALL) {
- struct kmod_list *filtered = NULL;

err = kmod_module_apply_filter(mod->ctx,
KMOD_FILTER_BLACKLIST, list, &filtered);
diff --git a/libkmod/libkmod-private.h b/libkmod/libkmod-private.h
index 4760733..0651d52 100644
--- a/libkmod/libkmod-private.h
+++ b/libkmod/libkmod-private.h
@@ -102,6 +102,7 @@ struct kmod_config {
struct kmod_ctx *ctx;
struct kmod_list *aliases;
struct kmod_list *blacklists;
+ struct kmod_list *whitelists;
struct kmod_list *options;
struct kmod_list *remove_commands;
struct kmod_list *install_commands;
@@ -112,7 +113,7 @@ struct kmod_config {

int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **config, const char * const *config_paths) __attribute__((nonnull(1, 2,3)));
void kmod_config_free(struct kmod_config *config) __attribute__((nonnull(1)));
-const char *kmod_blacklist_get_modname(const struct kmod_list *l) __attribute__((nonnull(1)));
+const char *kmod_list_get_modname(const struct kmod_list *l) __attribute__((nonnull(1)));
const char *kmod_alias_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
const char *kmod_alias_get_modname(const struct kmod_list *l) __attribute__((nonnull(1)));
const char *kmod_option_get_options(const struct kmod_list *l) __attribute__((nonnull(1)));
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index d03ab19..18721f3 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -106,6 +106,7 @@ struct kmod_list *kmod_list_last(const struct kmod_list *list);
*/
struct kmod_config_iter;
struct kmod_config_iter *kmod_config_get_blacklists(const struct kmod_ctx *ctx);
+struct kmod_config_iter *kmod_config_get_whitelists(const struct kmod_ctx *ctx);
struct kmod_config_iter *kmod_config_get_install_commands(const struct kmod_ctx *ctx);
struct kmod_config_iter *kmod_config_get_remove_commands(const struct kmod_ctx *ctx);
struct kmod_config_iter *kmod_config_get_aliases(const struct kmod_ctx *ctx);
@@ -162,12 +163,14 @@ enum kmod_probe {
KMOD_PROBE_APPLY_BLACKLIST_ALL = 0x10000,
KMOD_PROBE_APPLY_BLACKLIST = 0x20000,
KMOD_PROBE_APPLY_BLACKLIST_ALIAS_ONLY = 0x40000,
+ KMOD_PROBE_APPLY_WHITELIST = 0x80000,
};

/* Flags to kmod_module_apply_filter() */
enum kmod_filter {
KMOD_FILTER_BLACKLIST = 0x00001,
KMOD_FILTER_BUILTIN = 0x00002,
+ KMOD_FILTER_WHITELIST = 0x00004,
};

int kmod_module_remove_module(struct kmod_module *mod, unsigned int flags);
diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
index 854d257..6895d61 100644
--- a/libkmod/libkmod.sym
+++ b/libkmod/libkmod.sym
@@ -86,3 +86,8 @@ LIBKMOD_6 {
global:
kmod_module_apply_filter;
} LIBKMOD_5;
+
+LIBKMOD_12 {
+global:
+ kmod_config_get_whitelists;
+} LIBKMOD_6;
diff --git a/man/modprobe.d.xml b/man/modprobe.d.xml
index dc19b23..2140c9b 100644
--- a/man/modprobe.d.xml
+++ b/man/modprobe.d.xml
@@ -112,6 +112,33 @@
</listitem>
</varlistentry>
<varlistentry>
+ <term>whitelist <replaceable>modulename</replaceable>
+ </term>
+ <listitem>
+ <para>
+ If at least one <command>whitelist</command> command is specified,
+ module loading and scripts specified
+ using <command>install</command> are limited to module names
+ matching a glob <replaceable>pattern</replaceable> specified in one
+ of the <command>whitelist</command> commands.
+ </para>
+ <para>
+ You can prepare whitelist of currently loaded modules e.g. by this
+ command:
+ </para>
+ <para>
+ lsmod | tail -n +2 | cut -d ' ' -f 1 | sed 's/^/whitelist /'
+ </para>
+ <para>
+ Note that the <command>whitelist</command> command is not a direct
+ opposite of the <command>blacklist</command> command.
+ The <command>blacklist</command> command affects the selection
+ of a module to be loaded or <command>install</command> script to
+ be performed.
+ </para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
<term>install <replaceable>modulename</replaceable> <replaceable>command...</replaceable>
</term>
<listitem>
diff --git a/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf b/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf
new file mode 100644
index 0000000..9dc0ced
--- /dev/null
+++ b/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf
@@ -0,0 +1,2 @@
+whitelist floppy
+whitelist pcspkr
diff --git a/testsuite/test-whitelist.c b/testsuite/test-whitelist.c
new file mode 100644
index 0000000..49a3082
--- /dev/null
+++ b/testsuite/test-whitelist.c
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2011-2012 ProFUSION embedded systems
+ * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <errno.h>
+#include <unistd.h>
+#include <inttypes.h>
+#include <string.h>
+#include <libkmod.h>
+
+/* good luck bulding a kmod_list outside of the library... makes this blacklist
+ * function rather pointless */
+#include <libkmod-private.h>
+
+/* FIXME: hack, change name so we don't clash */
+#undef ERR
+#include "testsuite.h"
+
+static int whitelist_1(const struct test *t)
+{
+ struct kmod_ctx *ctx;
+ struct kmod_list *list = NULL, *l, *filtered;
+ struct kmod_module *mod;
+ int err;
+ size_t len = 0;
+
+ const char *names[] = { "pcspkr", "pcspkr2", "floppy", "ext4", NULL };
+ const char **name;
+
+ ctx = kmod_new(NULL, NULL);
+ if (ctx == NULL)
+ exit(EXIT_FAILURE);
+
+ for(name = names; *name; name++) {
+ err = kmod_module_new_from_name(ctx, *name, &mod);
+ if (err < 0)
+ goto fail_lookup;
+ list = kmod_list_append(list, mod);
+ }
+
+ err = kmod_module_apply_filter(ctx, KMOD_FILTER_WHITELIST, list,
+ &filtered);
+ if (err < 0) {
+ ERR("Could not filter: %s\n", strerror(-err));
+ goto fail;
+ }
+ if (filtered == NULL) {
+ ERR("All modules were filtered out!\n");
+ goto fail;
+ }
+
+ kmod_list_foreach(l, filtered) {
+ const char *modname;
+ mod = kmod_module_get_module(l);
+ modname = kmod_module_get_name(mod);
+ /* These are not on whitelist, must be rejected */
+ if (strcmp("pcspkr2", modname) == 0 || strcmp("ext4", modname) == 0)
+ goto fail;
+ /* These are on whitelist, must be in list */
+ if (strcmp("pcspkr", modname) && strcmp("floppy", modname))
+ goto fail;
+ len++;
+ kmod_module_unref(mod);
+ }
+
+ if (len != 2)
+ goto fail;
+
+ kmod_module_unref_list(filtered);
+ kmod_module_unref_list(list);
+ kmod_unref(ctx);
+
+ return EXIT_SUCCESS;
+
+fail:
+ kmod_module_unref_list(list);
+fail_lookup:
+ kmod_unref(ctx);
+ return EXIT_FAILURE;
+}
+static const struct test swhitelist_1 = {
+ .name = "whitelist_1",
+ .description = "check if modules are correctly whitelisted",
+ .func = whitelist_1,
+ .config = {
+ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-whitelist/",
+ },
+ .need_spawn = true,
+};
+
+static const struct test *tests[] = {
+ &swhitelist_1,
+ NULL,
+};
+
+TESTSUITE_MAIN(tests);
diff --git a/tools/modprobe.c b/tools/modprobe.c
index 437dea3..13bf379 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -547,10 +547,14 @@ static int insmod(struct kmod_ctx *ctx, const char *alias,
extra_options, NULL, NULL, show);
}

- if (err >= 0)
- /* ignore flag return values such as a mod being blacklisted */
- err = 0;
- else {
+ if (err >= 0) {
+ if (err & KMOD_PROBE_APPLY_WHITELIST)
+ ERR("could not insert '%s': Module not on whitelist\n",
+ kmod_module_get_name(mod));
+ else
+ /* ignore flag return values such as a mod being blacklisted */
+ err = 0;
+ } else {
switch (err) {
case -EEXIST:
ERR("could not insert '%s': Module already in kernel\n",

--
1.7.10.4

Lucas De Marchi

2013-03-22 15:26:28 UTC