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 <[email protected]>
---
init/Kconfig | 11 ++++++++
kernel/module/main.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
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/main.c b/kernel/module/main.c
index ea78cec316dd..d7cc64172dd1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -68,6 +68,16 @@
*/
DEFINE_MUTEX(module_mutex);
LIST_HEAD(modules);
+#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
+static LIST_HEAD(unloaded_tainted_modules);
+
+struct mod_unload_taint {
+ struct list_head list;
+ char name[MODULE_NAME_LEN];
+ unsigned long taints;
+ u64 count;
+};
+#endif
/* Work queue for freeing init sections in success case */
static void do_free_init(struct work_struct *w);
@@ -150,6 +160,41 @@ int unregister_module_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_module_notifier);
+#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
+static 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)) {
+ size_t len = strlen(mod_taint->name);
+
+ if (len == strlen(mod->name) && !memcmp(mod_taint->name, mod->name, len) &&
+ 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;
+}
+#else /* MODULE_UNLOAD_TAINT_TRACKING */
+static int try_add_tainted_module(struct module *mod)
+{
+ return 0;
+}
+#endif
+
/*
* We require a truly strong try_module_get(): 0 means success.
* Otherwise an error is returned due to ongoing or failed
@@ -1201,6 +1246,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. */
@@ -3126,6 +3174,9 @@ struct module *__module_text_address(unsigned long addr)
void print_modules(void)
{
struct module *mod;
+#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
+ struct mod_unload_taint *mod_taint;
+#endif
char buf[MODULE_FLAGS_BUF_SIZE];
printk(KERN_DEFAULT "Modules linked in:");
@@ -3136,6 +3187,20 @@ void print_modules(void)
continue;
pr_cont(" %s%s", mod->name, module_flags(mod, buf));
}
+#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
+ 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);
+ }
+#endif
+ }
preempt_enable();
if (last_unloaded_module[0])
pr_cont(" [last unloaded: %s]", last_unloaded_module);
--
2.34.1
On Thu 2022-04-21 16:28 +0200, Oleksandr Natalenko wrote:
> Hello.
Hi Oleksandr,
> Thanks for this submission. Please see one comment inline.
Thanks for your feedback!
> > @@ -150,6 +160,41 @@ int unregister_module_notifier(struct notifier_block *nb)
> > }
> > EXPORT_SYMBOL(unregister_module_notifier);
> >
> > +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
> > +static 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)) {
> > + size_t len = strlen(mod_taint->name);
> > +
> > + if (len == strlen(mod->name) && !memcmp(mod_taint->name, mod->name, len) &&
>
> Here, two strings are compared, so I'd expect to see `strncmp()` instead of `memcmp()`.
Good point. There are other examples of this throughout
kernel/module/main.c; albeit, I will use strncmp() here.
Kind regards,
--
Aaron Tomlin
Hi Aaron,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on eeaec7801c421e17edda6e45a32d4a5596b633da]
url: https://github.com/intel-lab-lkp/linux/commits/Aaron-Tomlin/module-Introduce-module-unload-taint-tracking/20220420-195459
base: eeaec7801c421e17edda6e45a32d4a5596b633da
config: hexagon-randconfig-r045-20220420 (https://download.01.org/0day-ci/archive/20220421/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bac6cd5bf85669e3376610cfc4c4f9ca015e7b9b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/523ff1f52de42049b9cc5a4db0495a5f21a7ee7c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Aaron-Tomlin/module-Introduce-module-unload-taint-tracking/20220420-195459
git checkout 523ff1f52de42049b9cc5a4db0495a5f21a7ee7c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> kernel/module/main.c:3204:2: error: meaningless 'volatile' on asm outside function
preempt_enable();
^
include/linux/preempt.h:277:28: note: expanded from macro 'preempt_enable'
#define preempt_enable() barrier()
^
include/linux/compiler.h:86:28: note: expanded from macro 'barrier'
# define barrier() __asm__ __volatile__("": : :"memory")
^
kernel/module/main.c:3204:2: error: expected ')'
include/linux/preempt.h:277:28: note: expanded from macro 'preempt_enable'
#define preempt_enable() barrier()
^
include/linux/compiler.h:86:43: note: expanded from macro 'barrier'
# define barrier() __asm__ __volatile__("": : :"memory")
^
kernel/module/main.c:3204:2: note: to match this '('
include/linux/preempt.h:277:28: note: expanded from macro 'preempt_enable'
#define preempt_enable() barrier()
^
include/linux/compiler.h:86:40: note: expanded from macro 'barrier'
# define barrier() __asm__ __volatile__("": : :"memory")
^
kernel/module/main.c:3205:2: error: expected identifier or '('
if (last_unloaded_module[0])
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
kernel/module/main.c:3207:2: error: expected identifier or '('
pr_cont("\n");
^
include/linux/printk.h:531:2: note: expanded from macro 'pr_cont'
printk(KERN_CONT fmt, ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^
include/linux/printk.h:416:3: note: expanded from macro 'printk_index_wrap'
({ \
^
kernel/module/main.c:3207:2: error: expected ')'
include/linux/printk.h:531:2: note: expanded from macro 'pr_cont'
printk(KERN_CONT fmt, ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^
include/linux/printk.h:416:3: note: expanded from macro 'printk_index_wrap'
({ \
^
kernel/module/main.c:3207:2: note: to match this '('
include/linux/printk.h:531:2: note: expanded from macro 'pr_cont'
printk(KERN_CONT fmt, ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^
include/linux/printk.h:416:2: note: expanded from macro 'printk_index_wrap'
({ \
^
kernel/module/main.c:3208:1: error: extraneous closing brace ('}')
}
^
6 errors generated.
vim +/volatile +3204 kernel/module/main.c
e610499e2656e61 kernel/module.c Rusty Russell 2009-03-31 3172
^1da177e4c3f415 kernel/module.c Linus Torvalds 2005-04-16 3173 /* Don't grab lock, we're oopsing. */
^1da177e4c3f415 kernel/module.c Linus Torvalds 2005-04-16 3174 void print_modules(void)
^1da177e4c3f415 kernel/module.c Linus Torvalds 2005-04-16 3175 {
^1da177e4c3f415 kernel/module.c Linus Torvalds 2005-04-16 3176 struct module *mod;
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3177 #ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3178 struct mod_unload_taint *mod_taint;
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3179 #endif
7fd8329ba502ef7 kernel/module.c Petr Mladek 2016-09-21 3180 char buf[MODULE_FLAGS_BUF_SIZE];
^1da177e4c3f415 kernel/module.c Linus Torvalds 2005-04-16 3181
b231125af7811a2 kernel/module.c Linus Torvalds 2009-06-16 3182 printk(KERN_DEFAULT "Modules linked in:");
d72b37513cdfbd3 kernel/module.c Andi Kleen 2008-08-30 3183 /* Most callers should already have preempt disabled, but make sure */
d72b37513cdfbd3 kernel/module.c Andi Kleen 2008-08-30 3184 preempt_disable();
0d21b0e3477395e kernel/module.c Rusty Russell 2013-01-12 3185 list_for_each_entry_rcu(mod, &modules, list) {
0d21b0e3477395e kernel/module.c Rusty Russell 2013-01-12 3186 if (mod->state == MODULE_STATE_UNFORMED)
0d21b0e3477395e kernel/module.c Rusty Russell 2013-01-12 3187 continue;
27bba4d6bb3779a kernel/module.c Jiri Slaby 2014-02-03 3188 pr_cont(" %s%s", mod->name, module_flags(mod, buf));
0d21b0e3477395e kernel/module.c Rusty Russell 2013-01-12 3189 }
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3190 #ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3191 if (!list_empty(&unloaded_tainted_modules)) {
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3192 printk(KERN_DEFAULT "Unloaded tainted modules:");
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3193 list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules,
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3194 list) {
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3195 size_t l;
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3196
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3197 l = module_flags_taint(mod_taint->taints, buf);
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3198 buf[l++] = '\0';
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3199 pr_cont(" %s(%s):%llu", mod_taint->name, buf,
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3200 mod_taint->count);
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3201 }
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3202 #endif
523ff1f52de4204 kernel/module/main.c Aaron Tomlin 2022-04-20 3203 }
d72b37513cdfbd3 kernel/module.c Andi Kleen 2008-08-30 @3204 preempt_enable();
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Fri 2022-04-22 10:11 +0200, Christoph Lameter wrote:
> On Thu, 21 Apr 2022, Aaron Tomlin wrote:
>
> > > > + if (len == strlen(mod->name) && !memcmp(mod_taint->name, mod->name, len) &&
> > >
> > > Here, two strings are compared, so I'd expect to see `strncmp()` instead of `memcmp()`.
> >
> > Good point. There are other examples of this throughout
> > kernel/module/main.c; albeit, I will use strncmp() here.
>
> Comparing the length first may be an attempt to avoid the expensive
> memcmp. But here we need to first execute strlen() to obtain the string
> length. This is already accessing all characters so this
> check is wasteful and a straight str[n]cmp is better.
Hi Christoph,
Agreed - we can skip the extra strlen().
Thanks,
--
Aaron Tomlin
On Thu, 21 Apr 2022, Aaron Tomlin wrote:
> > > + if (len == strlen(mod->name) && !memcmp(mod_taint->name, mod->name, len) &&
> >
> > Here, two strings are compared, so I'd expect to see `strncmp()` instead of `memcmp()`.
>
> Good point. There are other examples of this throughout
> kernel/module/main.c; albeit, I will use strncmp() here.
Comparing the length first may be an attempt to avoid the expensive
memcmp. But here we need to first execute strlen() to obtain the string
length. This is already accessing all characters so this
check is wasteful and a straight str[n]cmp is better.
Hello.
Thanks for this submission. Please see one comment inline.
On středa 20. dubna 2022 13:52:57 CEST Aaron Tomlin wrote:
> 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 <[email protected]>
> ---
> init/Kconfig | 11 ++++++++
> kernel/module/main.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> 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/main.c b/kernel/module/main.c
> index ea78cec316dd..d7cc64172dd1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -68,6 +68,16 @@
> */
> DEFINE_MUTEX(module_mutex);
> LIST_HEAD(modules);
> +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
> +static LIST_HEAD(unloaded_tainted_modules);
> +
> +struct mod_unload_taint {
> + struct list_head list;
> + char name[MODULE_NAME_LEN];
> + unsigned long taints;
> + u64 count;
> +};
> +#endif
>
> /* Work queue for freeing init sections in success case */
> static void do_free_init(struct work_struct *w);
> @@ -150,6 +160,41 @@ int unregister_module_notifier(struct notifier_block *nb)
> }
> EXPORT_SYMBOL(unregister_module_notifier);
>
> +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
> +static 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)) {
> + size_t len = strlen(mod_taint->name);
> +
> + if (len == strlen(mod->name) && !memcmp(mod_taint->name, mod->name, len) &&
Here, two strings are compared, so I'd expect to see `strncmp()` instead of `memcmp()`.
> + 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;
> +}
> +#else /* MODULE_UNLOAD_TAINT_TRACKING */
> +static int try_add_tainted_module(struct module *mod)
> +{
> + return 0;
> +}
> +#endif
> +
> /*
> * We require a truly strong try_module_get(): 0 means success.
> * Otherwise an error is returned due to ongoing or failed
> @@ -1201,6 +1246,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. */
> @@ -3126,6 +3174,9 @@ struct module *__module_text_address(unsigned long addr)
> void print_modules(void)
> {
> struct module *mod;
> +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
> + struct mod_unload_taint *mod_taint;
> +#endif
> char buf[MODULE_FLAGS_BUF_SIZE];
>
> printk(KERN_DEFAULT "Modules linked in:");
> @@ -3136,6 +3187,20 @@ void print_modules(void)
> continue;
> pr_cont(" %s%s", mod->name, module_flags(mod, buf));
> }
> +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
> + 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);
> + }
> +#endif
> + }
> preempt_enable();
> if (last_unloaded_module[0])
> pr_cont(" [last unloaded: %s]", last_unloaded_module);
>
--
Oleksandr Natalenko (post-factum)