From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755600AbcHYEdV (ORCPT <rfc822;w@1wt.eu>);
        Thu, 25 Aug 2016 00:33:21 -0400
Received: from mx1.redhat.com ([209.132.183.28]:47338 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1755433AbcHYEdI (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 25 Aug 2016 00:33:08 -0400
Date: Wed, 24 Aug 2016 22:53:45 -0400
From: Jessica Yu <jeyu@redhat.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
        Petr Mladek <pmladek@suse.com>, linux-kernel@vger.kernel.org,
        live-patching@vger.kernel.org, Chunyu Hu <chuhu@redhat.com>,
        Rusty Russell <rusty@rustcorp.com.au>
Subject: Re: livepatch/module: make TAINT_LIVEPATCH module-specific
Message-ID: <20160825025345.GA6387@packer-debian-8-amd64.digitalocean.com>
References: <c6a606f6c1e291c6c292403ade24833801ad243d.1472074373.git.jpoimboe@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <c6a606f6c1e291c6c292403ade24833801ad243d.1472074373.git.jpoimboe@redhat.com>
X-OS: Linux eisen.io 3.16.0-4-amd64 x86_64
User-Agent: Mutt/1.5.23 (2014-03-12)
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 25 Aug 2016 02:53:47 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

+++ Josh Poimboeuf [24/08/16 16:33 -0500]:
>There's no reliable way to determine which module tainted the kernel
>with CONFIG_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 converting
>CONFIG_LIVEPATCH to a module taint flag.
>
>This changes the behavior a bit: now the the flag gets set when the
>module is loaded, rather than when it's enabled.

Did a quick sanity check and verified the module taint
shows up in crash and sysfs as expected, looks good.

>Reviewed-by: Chunyu Hu <chuhu@redhat.com>
>Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Acked-by: Jessica Yu <jeyu@redhat.com>

>---
> kernel/livepatch/core.c |  3 ---
> kernel/module.c         | 35 ++++++++++++-----------------------
> 2 files changed, 12 insertions(+), 26 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..fd5f95b 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
>@@ -2791,26 +2793,6 @@ static int copy_chunked_from_user(void *dst, const void __user *usrc, unsigned l
> 	return 0;
> }
>
>-#ifdef CONFIG_LIVEPATCH
>-static int find_livepatch_modinfo(struct module *mod, struct load_info *info)
>-{
>-	mod->klp = get_modinfo(info, "livepatch") ? true : false;
>-
>-	return 0;
>-}
>-#else /* !CONFIG_LIVEPATCH */
>-static int find_livepatch_modinfo(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",
>-		       mod->name);
>-		return -ENOEXEC;
>-	}
>-
>-	return 0;
>-}
>-#endif /* CONFIG_LIVEPATCH */
>-
> /* Sets info->hdr and info->len. */
> static int copy_module_from_user(const void __user *umod, unsigned long len,
> 				  struct load_info *info)
>@@ -2969,9 +2951,16 @@ 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);
>-	if (err)
>-		return err;
>+	if (get_modinfo(info, "livepatch")) {
>+		if (!IS_ENABLED(CONFIG_LIVEPATCH)) {
>+			pr_err("%s: module is marked as livepatch module, but livepatch support is disabled\n",
>+			       mod->name);
>+			return -ENOEXEC;
>+		}
>+		mod->klp = true;
>+		pr_warn("%s: loading livepatch module.\n", mod->name);
>+		add_taint_module(mod, TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
>+	}
>
> 	/* Set up license info based on the info section */
> 	set_license(mod, get_modinfo(info, "license"));
>-- 
>2.7.4
>