* [PATCH v5 1/3] module: Make module_flags_taint() accept a module's taints bitmap and usable outside core code
2022-05-02 20:51 [PATCH v5 0/3] module: Introduce module unload taint tracking Aaron Tomlin
@ 2022-05-02 20:51 ` Aaron Tomlin
2022-05-02 20:51 ` [PATCH v5 2/3] module: Move module_assert_mutex_or_preempt() to internal.h Aaron Tomlin
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Aaron Tomlin @ 2022-05-02 20:51 UTC (permalink / raw)
To: mcgrof
Cc: cl, pmladek, mbenes, christophe.leroy, akpm, linux-kernel,
linux-modules, atomlin, ghalat, oleksandr, neelx
No functional change.
The purpose of this patch is to modify module_flags_taint() to accept
a module's taints bitmap as a parameter and modifies all users
accordingly. Furthermore, it is now possible to access a given
module's taint flags data outside of non-essential code yet does
remain for internal use only.
This is in preparation for module unload taint tracking support.
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
kernel/module/internal.h | 1 +
kernel/module/main.c | 8 ++++----
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 3e23bef5884d..abbd1c5ef264 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -100,6 +100,7 @@ int cmp_name(const void *name, const void *sym);
long module_get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr,
unsigned int section);
char *module_flags(struct module *mod, char *buf);
+size_t module_flags_taint(unsigned long taints, char *buf);
static inline unsigned long kernel_symbol_value(const struct kernel_symbol *sym)
{
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 05a42d8fcd7a..7dbdd098b995 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -890,13 +890,13 @@ static inline int module_unload_init(struct module *mod)
}
#endif /* CONFIG_MODULE_UNLOAD */
-static size_t module_flags_taint(struct module *mod, char *buf)
+size_t module_flags_taint(unsigned long taints, char *buf)
{
size_t l = 0;
int i;
for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
- if (taint_flags[i].module && test_bit(i, &mod->taints))
+ if (taint_flags[i].module && test_bit(i, &taints))
buf[l++] = taint_flags[i].c_true;
}
@@ -974,7 +974,7 @@ static ssize_t show_taint(struct module_attribute *mattr,
{
size_t l;
- l = module_flags_taint(mk->mod, buffer);
+ l = module_flags_taint(mk->mod->taints, buffer);
buffer[l++] = '\n';
return l;
}
@@ -2993,7 +2993,7 @@ char *module_flags(struct module *mod, char *buf)
mod->state == MODULE_STATE_GOING ||
mod->state == MODULE_STATE_COMING) {
buf[bx++] = '(';
- bx += module_flags_taint(mod, buf + bx);
+ bx += module_flags_taint(mod->taints, buf + bx);
/* Show a - for module-is-being-unloaded */
if (mod->state == MODULE_STATE_GOING)
buf[bx++] = '-';
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v5 2/3] module: Move module_assert_mutex_or_preempt() to internal.h
2022-05-02 20:51 [PATCH v5 0/3] module: Introduce module unload taint tracking Aaron Tomlin
2022-05-02 20:51 ` [PATCH v5 1/3] module: Make module_flags_taint() accept a module's taints bitmap and usable outside core code Aaron Tomlin
@ 2022-05-02 20:51 ` Aaron Tomlin
2022-05-02 20:52 ` [PATCH v5 3/3] module: Introduce module unload taint tracking Aaron Tomlin
2022-05-03 20:12 ` [PATCH v5 0/3] " Luis Chamberlain
3 siblings, 0 replies; 5+ messages in thread
From: Aaron Tomlin @ 2022-05-02 20:51 UTC (permalink / raw)
To: mcgrof
Cc: cl, pmladek, mbenes, christophe.leroy, akpm, linux-kernel,
linux-modules, atomlin, ghalat, oleksandr, neelx
No functional change.
This patch migrates module_assert_mutex_or_preempt() to internal.h.
So, the aforementiond function can be used outside of main/or core
module code yet will remain restricted for internal use only.
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
kernel/module/internal.h | 12 ++++++++++++
kernel/module/main.c | 11 -----------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index abbd1c5ef264..0bdf64c9dfb5 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/rculist.h>
+#include <linux/rcupdate.h>
#ifndef ARCH_SHF_SMALL
#define ARCH_SHF_SMALL 0
@@ -102,6 +103,17 @@ long module_get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr,
char *module_flags(struct module *mod, char *buf);
size_t module_flags_taint(unsigned long taints, char *buf);
+static inline void module_assert_mutex_or_preempt(void)
+{
+#ifdef CONFIG_LOCKDEP
+ if (unlikely(!debug_locks))
+ return;
+
+ WARN_ON_ONCE(!rcu_read_lock_sched_held() &&
+ !lockdep_is_held(&module_mutex));
+#endif
+}
+
static inline unsigned long kernel_symbol_value(const struct kernel_symbol *sym)
{
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 7dbdd098b995..7a0484900320 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -118,17 +118,6 @@ static void mod_update_bounds(struct module *mod)
#endif
}
-static void module_assert_mutex_or_preempt(void)
-{
-#ifdef CONFIG_LOCKDEP
- if (unlikely(!debug_locks))
- return;
-
- WARN_ON_ONCE(!rcu_read_lock_sched_held() &&
- !lockdep_is_held(&module_mutex));
-#endif
-}
-
/* Block module loading/unloading? */
int modules_disabled = 0;
core_param(nomodule, modules_disabled, bint, 0);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v5 3/3] module: Introduce module unload taint tracking
2022-05-02 20:51 [PATCH v5 0/3] module: Introduce module unload taint tracking Aaron Tomlin
2022-05-02 20:51 ` [PATCH v5 1/3] module: Make module_flags_taint() accept a module's taints bitmap and usable outside core code Aaron Tomlin
2022-05-02 20:51 ` [PATCH v5 2/3] module: Move module_assert_mutex_or_preempt() to internal.h Aaron Tomlin
@ 2022-05-02 20:52 ` Aaron Tomlin
2022-05-03 20:12 ` [PATCH v5 0/3] " Luis Chamberlain
3 siblings, 0 replies; 5+ messages in thread
From: Aaron Tomlin @ 2022-05-02 20:52 UTC (permalink / raw)
To: mcgrof
Cc: cl, pmladek, mbenes, christophe.leroy, akpm, linux-kernel,
linux-modules, atomlin, ghalat, oleksandr, neelx
Currently, only the initial module that tainted the kernel is
recorded e.g. when an out-of-tree module is loaded.
The purpose of this patch is to allow the kernel to maintain a record of
each unloaded module that taints the kernel. So, in addition to
displaying a list of linked modules (see print_modules()) e.g. in the
event of a detected bad page, unloaded modules that carried a taint/or
taints are displayed too. A tainted module unload count is maintained.
The number of tracked modules is not fixed. This feature is disabled by
default.
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
init/Kconfig | 11 ++++++++
kernel/module/Makefile | 1 +
kernel/module/internal.h | 21 ++++++++++++++
kernel/module/main.c | 5 ++++
kernel/module/tracking.c | 61 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 99 insertions(+)
create mode 100644 kernel/module/tracking.c
diff --git a/init/Kconfig b/init/Kconfig
index ddcbefe535e9..6b30210f787d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2118,6 +2118,17 @@ config MODULE_FORCE_UNLOAD
rmmod). This is mainly for kernel developers and desperate users.
If unsure, say N.
+config MODULE_UNLOAD_TAINT_TRACKING
+ bool "Tainted module unload tracking"
+ depends on MODULE_UNLOAD
+ default n
+ help
+ This option allows you to maintain a record of each unloaded
+ module that tainted the kernel. In addition to displaying a
+ list of linked (or loaded) modules e.g. on detection of a bad
+ page (see bad_page()), the aforementioned details are also
+ shown. If unsure, say N.
+
config MODVERSIONS
bool "Module versioning support"
help
diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index d1ca799c12e2..948efea81e85 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_PROC_FS) += procfs.o
obj-$(CONFIG_SYSFS) += sysfs.o
obj-$(CONFIG_KGDB_KDB) += kdb.o
obj-$(CONFIG_MODVERSIONS) += version.o
+obj-$(CONFIG_MODULE_UNLOAD_TAINT_TRACKING) += tracking.o
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 0bdf64c9dfb5..bc5507ab8450 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -145,6 +145,27 @@ static inline bool set_livepatch_module(struct module *mod)
#endif
}
+#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
+struct mod_unload_taint {
+ struct list_head list;
+ char name[MODULE_NAME_LEN];
+ unsigned long taints;
+ u64 count;
+};
+
+int try_add_tainted_module(struct module *mod);
+void print_unloaded_tainted_modules(void);
+#else /* !CONFIG_MODULE_UNLOAD_TAINT_TRACKING */
+static inline int try_add_tainted_module(struct module *mod)
+{
+ return 0;
+}
+
+static inline void print_unloaded_tainted_modules(void)
+{
+}
+#endif /* CONFIG_MODULE_UNLOAD_TAINT_TRACKING */
+
#ifdef CONFIG_MODULE_DECOMPRESS
int module_decompress(struct load_info *info, const void *buf, size_t size);
void module_decompress_cleanup(struct load_info *info);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 7a0484900320..6c3b4a846645 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1190,6 +1190,9 @@ static void free_module(struct module *mod)
module_bug_cleanup(mod);
/* Wait for RCU-sched synchronizing before releasing mod->list and buglist. */
synchronize_rcu();
+ if (try_add_tainted_module(mod))
+ pr_err("%s: adding tainted module to the unloaded tainted modules list failed.\n",
+ mod->name);
mutex_unlock(&module_mutex);
/* Clean up CFI for the module. */
@@ -3125,6 +3128,8 @@ void print_modules(void)
continue;
pr_cont(" %s%s", mod->name, module_flags(mod, buf));
}
+
+ print_unloaded_tainted_modules();
preempt_enable();
if (last_unloaded_module[0])
pr_cont(" [last unloaded: %s]", last_unloaded_module);
diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
new file mode 100644
index 000000000000..7f8133044d09
--- /dev/null
+++ b/kernel/module/tracking.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module taint unload tracking support
+ *
+ * Copyright (C) 2022 Aaron Tomlin
+ */
+
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include <linux/rculist.h>
+#include "internal.h"
+
+static LIST_HEAD(unloaded_tainted_modules);
+
+int try_add_tainted_module(struct module *mod)
+{
+ struct mod_unload_taint *mod_taint;
+
+ module_assert_mutex_or_preempt();
+
+ list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, list,
+ lockdep_is_held(&module_mutex)) {
+ if (!strcmp(mod_taint->name, mod->name) &&
+ mod_taint->taints & mod->taints) {
+ mod_taint->count++;
+ goto out;
+ }
+ }
+
+ mod_taint = kmalloc(sizeof(*mod_taint), GFP_KERNEL);
+ if (unlikely(!mod_taint))
+ return -ENOMEM;
+ strscpy(mod_taint->name, mod->name, MODULE_NAME_LEN);
+ mod_taint->taints = mod->taints;
+ list_add_rcu(&mod_taint->list, &unloaded_tainted_modules);
+ mod_taint->count = 1;
+out:
+ return 0;
+}
+
+void print_unloaded_tainted_modules(void)
+{
+ struct mod_unload_taint *mod_taint;
+ char buf[MODULE_FLAGS_BUF_SIZE];
+
+ if (!list_empty(&unloaded_tainted_modules)) {
+ printk(KERN_DEFAULT "Unloaded tainted modules:");
+ list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules,
+ list) {
+ size_t l;
+
+ l = module_flags_taint(mod_taint->taints, buf);
+ buf[l++] = '\0';
+ pr_cont(" %s(%s):%llu", mod_taint->name, buf,
+ mod_taint->count);
+ }
+ }
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v5 0/3] module: Introduce module unload taint tracking
2022-05-02 20:51 [PATCH v5 0/3] module: Introduce module unload taint tracking Aaron Tomlin
` (2 preceding siblings ...)
2022-05-02 20:52 ` [PATCH v5 3/3] module: Introduce module unload taint tracking Aaron Tomlin
@ 2022-05-03 20:12 ` Luis Chamberlain
3 siblings, 0 replies; 5+ messages in thread
From: Luis Chamberlain @ 2022-05-03 20:12 UTC (permalink / raw)
To: Aaron Tomlin
Cc: cl, pmladek, mbenes, christophe.leroy, akpm, linux-kernel,
linux-modules, atomlin, ghalat, oleksandr, neelx
On Mon, May 02, 2022 at 09:51:02PM +0100, Aaron Tomlin wrote:
> Hi Luis,
>
> This iteration is still based on the latest mcgrof/modules-next branch.
>
> I have decided still to use RCU even though no entry is ever removed from
> the unloaded tainted modules list. That being said, if I understand
> correctly, it is not safe in some instances to use 'module_mutex' in
> print_modules(). So instead we disable preemption to ensure list traversal
> with concurrent list manipulation e.g. list_add_rcu(), is safe too.
>
> Changes since v4 [1]
> - Moved code to kernel/module/tracking.c
> (Luis Chamberlain)
> - Used only strcmp() in try_add_tainted_module()
> (Christophe Leroy)
>
> Changes since v3 [2]
> - Fixed kernel build error reported by kernel test robot i.e. moved
> '#endif' outside 'if (!list_empty(&unloaded_tainted_modules))'
> statement in the context of print_modules()
> - Used strncmp() instead of memcmp()
> (Oleksandr Natalenko)
> - Removed the additional strlen()
> (Christoph Lameter)
>
> Changes since v2 [3]
> - Dropped RFC from subject
> - Removed the newline i.e. "\n" in printk()
> - Always include the tainted module's unload count
> - Unconditionally display each unloaded tainted module
>
> Please let me know your thoughts.
Thanks! Queued onto modules-testing. If no issues are found I'll push to
modules-next soon after.
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread