[PATCH v2] livepatch/module: make TAINT_LIVEPATCH module-specific

[PATCH v2] livepatch/module: make TAINT_LIVEPATCH module-specific

[Josh Poimboeuf]

There's no reliable way to determine which module tainted the kernel
with TAINT_LIVEPATCH.  For example, /sys/module/<klp module>/taint
doesn't report it.  Neither does the "mod -t" command in the crash tool.

Make it crystal clear who the guilty party is by associating
TAINT_LIVEPATCH with any module which sets the "livepatch" modinfo
attribute.  The flag will still get set in the kernel like before, but
now it also sets the same flag in mod->taint.

Note that now the taint flag gets set when the module is loaded rather
than when it's enabled.

I also renamed find_livepatch_modinfo() to check_modinfo_livepatch() to
better reflect its purpose: it's basically a livepatch-specific
sub-function of check_modinfo().

Reported-by: Chunyu Hu <chuhu@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 kernel/livepatch/core.c |  3 ---
 kernel/module.c         | 13 +++++++++----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 5fbabe0..af46438 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -545,9 +545,6 @@ static int __klp_enable_patch(struct klp_patch *patch)
 	    list_prev_entry(patch, list)->state == KLP_DISABLED)
 		return -EBUSY;
 
-	pr_notice_once("tainting kernel with TAINT_LIVEPATCH\n");
-	add_taint(TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
-
 	pr_notice("enabling patch '%s'\n", patch->mod->name);
 
 	klp_for_each_object(patch, obj) {
diff --git a/kernel/module.c b/kernel/module.c
index 529efae..f57dd63 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1149,6 +1149,8 @@ static size_t module_flags_taint(struct module *mod, char *buf)
 		buf[l++] = 'C';
 	if (mod->taints & (1 << TAINT_UNSIGNED_MODULE))
 		buf[l++] = 'E';
+	if (mod->taints & (1 << TAINT_LIVEPATCH))
+		buf[l++] = 'K';
 	/*
 	 * TAINT_FORCED_RMMOD: could be added.
 	 * TAINT_CPU_OUT_OF_SPEC, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
@@ -2792,14 +2794,17 @@ static int copy_chunked_from_user(void *dst, const void __user *usrc, unsigned l
 }
 
 #ifdef CONFIG_LIVEPATCH
-static int find_livepatch_modinfo(struct module *mod, struct load_info *info)
+static int check_modinfo_livepatch(struct module *mod, struct load_info *info)
 {
-	mod->klp = get_modinfo(info, "livepatch") ? true : false;
+	if (get_modinfo(info, "livepatch")) {
+		mod->klp = true;
+		add_taint_module(mod, TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
+	}
 
 	return 0;
 }
 #else /* !CONFIG_LIVEPATCH */
-static int find_livepatch_modinfo(struct module *mod, struct load_info *info)
+static int check_modinfo_livepatch(struct module *mod, struct load_info *info)
 {
 	if (get_modinfo(info, "livepatch")) {
 		pr_err("%s: module is marked as livepatch module, but livepatch support is disabled",
@@ -2969,7 +2974,7 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 			"is unknown, you have been warned.\n", mod->name);
 	}
 
-	err = find_livepatch_modinfo(mod, info);
+	err = check_modinfo_livepatch(mod, info);
 	if (err)
 		return err;
 
-- 
2.7.4

Re: [PATCH v2] livepatch/module: make TAINT_LIVEPATCH module-specific

[Petr Mladek]

On Thu 2016-08-25 10:04:45, Josh Poimboeuf wrote:
> There's no reliable way to determine which module tainted the kernel
> with TAINT_LIVEPATCH.  For example, /sys/module/<klp module>/taint
> doesn't report it.  Neither does the "mod -t" command in the crash tool.
> 
> Make it crystal clear who the guilty party is by associating
> TAINT_LIVEPATCH with any module which sets the "livepatch" modinfo
> attribute.  The flag will still get set in the kernel like before, but
> now it also sets the same flag in mod->taint.
> 
> Note that now the taint flag gets set when the module is loaded rather
> than when it's enabled.
> 
> I also renamed find_livepatch_modinfo() to check_modinfo_livepatch() to
> better reflect its purpose: it's basically a livepatch-specific
> sub-function of check_modinfo().
> 
> Reported-by: Chunyu Hu <chuhu@redhat.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Everything looks fine now.

Reviewed-by: Petr Mladek <pmladek@suse.com>


Best Regards,
Petr

Re: [PATCH v2] livepatch/module: make TAINT_LIVEPATCH module-specific

[Miroslav Benes]

On Thu, 25 Aug 2016, Josh Poimboeuf wrote:

> There's no reliable way to determine which module tainted the kernel
> with TAINT_LIVEPATCH.  For example, /sys/module/<klp module>/taint
> doesn't report it.  Neither does the "mod -t" command in the crash tool.
> 
> Make it crystal clear who the guilty party is by associating
> TAINT_LIVEPATCH with any module which sets the "livepatch" modinfo
> attribute.  The flag will still get set in the kernel like before, but
> now it also sets the same flag in mod->taint.
> 
> Note that now the taint flag gets set when the module is loaded rather
> than when it's enabled.
> 
> I also renamed find_livepatch_modinfo() to check_modinfo_livepatch() to
> better reflect its purpose: it's basically a livepatch-specific
> sub-function of check_modinfo().
> 
> Reported-by: Chunyu Hu <chuhu@redhat.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Acked-by: Miroslav Benes <mbenes@suse.cz>

Miroslav

Re: [PATCH v2] livepatch/module: make TAINT_LIVEPATCH module-specific

[Jiri Kosina]

On Thu, 25 Aug 2016, Josh Poimboeuf wrote:

> There's no reliable way to determine which module tainted the kernel
> with TAINT_LIVEPATCH.  For example, /sys/module/<klp module>/taint
> doesn't report it.  Neither does the "mod -t" command in the crash tool.
> 
> Make it crystal clear who the guilty party is by associating
> TAINT_LIVEPATCH with any module which sets the "livepatch" modinfo
> attribute.  The flag will still get set in the kernel like before, but
> now it also sets the same flag in mod->taint.
> 
> Note that now the taint flag gets set when the module is loaded rather
> than when it's enabled.
> 
> I also renamed find_livepatch_modinfo() to check_modinfo_livepatch() to
> better reflect its purpose: it's basically a livepatch-specific
> sub-function of check_modinfo().
> 
> Reported-by: Chunyu Hu <chuhu@redhat.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

I've preserved the acks from v1 (*) and applied. Thanks.

(*) Rusty's and Jessica's. If either if you disagrees with that, please 
    let me know

-- 
Jiri Kosina
SUSE Labs