mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.cz>
To: Seth Jennings <sjenning@redhat.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Jiri Kosina <jkosina@suse.cz>, Vojtech Pavlik <vojtech@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>,
	Miroslav Benes <mbenes@suse.cz>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	live-patching@vger.kernel.org, x86@kernel.org, kpatch@redhat.com,
	linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.cz>
Subject: [PATCH 2/6] livepatch v5: do not quietly ignore wrong usage
Date: Tue,  9 Dec 2014 19:05:03 +0100	[thread overview]
Message-ID: <1418148307-21434-3-git-send-email-pmladek@suse.cz> (raw)
In-Reply-To: <1418148307-21434-1-git-send-email-pmladek@suse.cz>

I suggested to add more checks between v4 and v5 of this patchset. Some
of them were lazy. They allowed to call the function even when there
was nothing to do. Mirek persuaded me that it was bad design because
it would hide some potential problems. We fixed this for klp_object_enable()
but not for the other lazy checks.

This patch removes the other lazy checks. Also it makes sure that
the functions are called only when allowed.

Note that the check in klp_disable_object() must be an error. It is called
when the patch is enabled. We could not disable it without the valid address.

Note that it adds some more nested checks but I plan to clean this later by
introducing __klp_init_object(). It will help to remove the duplicated
code from module_coming().

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 kernel/livepatch/core.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index b848069e44cc..034f79a926af 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -231,12 +231,11 @@ static int klp_write_object_relocations(struct module *pmod,
 	int ret;
 	struct klp_reloc *reloc;
 
-	if (!klp_is_object_loaded(obj))
-		return 0;
+	if (WARN_ON(!klp_is_object_loaded(obj)))
+		return -EINVAL;
 
-	/* be happy when there is nothing to do */
-	if (!obj->relocs)
-		return 0;
+	if (WARN_ON(!obj->relocs))
+		return -EINVAL;
 
 	for (reloc = obj->relocs; reloc->name; reloc++) {
 		if (!klp_is_module(obj)) {
@@ -315,9 +314,8 @@ static int klp_disable_func(struct klp_func *func)
 	if (WARN_ON(func->state != KLP_ENABLED))
 		return -EINVAL;
 
-	if (!func->old_addr)
-		/* parent object is not loaded */
-		return 0;
+	if (WARN_ON(!func->old_addr))
+		return -EINVAL;
 
 	ret = unregister_ftrace_function(func->fops);
 	if (ret) {
@@ -520,9 +518,11 @@ static void klp_module_notify_coming(struct module *pmod,
 	pr_notice("applying patch '%s' to loading module '%s'\n",
 		  pmod->name, mod->name);
 
-	ret = klp_write_object_relocations(pmod, obj);
-	if (ret)
-		goto err;
+	if (obj->relocs) {
+		ret = klp_write_object_relocations(pmod, obj);
+		if (ret)
+			goto err;
+	}
 
 	ret = klp_enable_object(obj);
 	if (!ret)
@@ -751,9 +751,11 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
 
 	klp_find_object_module(obj);
 
-	ret = klp_write_object_relocations(patch->mod, obj);
-	if (ret)
-		return ret;
+	if (obj->relocs && klp_is_object_loaded(obj)) {
+		ret = klp_write_object_relocations(patch->mod, obj);
+		if (ret)
+			return ret;
+	}
 
 	name = klp_is_module(obj) ? obj->name : "vmlinux";
 	obj->kobj = kobject_create_and_add(name, &patch->kobj);
-- 
1.8.5.2


  parent reply	other threads:[~2014-12-09 18:06 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-09 18:05 [PATCH 0/6] livepatch v5: more clean up for the v5 of the patchset Petr Mladek
2014-12-09 18:05 ` [PATCH 1/6] livepatch v5: avoid race when checking for state of the patch Petr Mladek
2014-12-09 18:32   ` Josh Poimboeuf
2014-12-10 10:11     ` Petr Mladek
2014-12-10 15:25       ` Josh Poimboeuf
2014-12-11 11:17         ` Petr Mladek
2014-12-11 14:23           ` Josh Poimboeuf
2014-12-09 18:05 ` Petr Mladek [this message]
2014-12-09 18:05 ` [PATCH 3/6] livepatch v5: find and verify the old address in klp_*_init() Petr Mladek
2014-12-09 18:05 ` [PATCH 4/6] livepatch v5: clean up ordering of functions Petr Mladek
2014-12-09 18:05 ` [PATCH 5/6] livepatch v5: split init and free code that is done only for loaded modules Petr Mladek
2014-12-09 18:51   ` Josh Poimboeuf
2014-12-10 10:30     ` Petr Mladek
2014-12-10 15:34       ` Josh Poimboeuf
2014-12-09 18:05 ` [PATCH 6/6] livepatch v5: clean up usage of the old and new addresses Petr Mladek
2014-12-09 19:20   ` Josh Poimboeuf
2014-12-10 13:50     ` Petr Mladek
2014-12-10 15:33       ` Josh Poimboeuf
2014-12-11 11:30         ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1418148307-21434-3-git-send-email-pmladek@suse.cz \
    --to=pmladek@suse.cz \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=jkosina@suse.cz \
    --cc=jpoimboe@redhat.com \
    --cc=kpatch@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mbenes@suse.cz \
    --cc=rostedt@goodmis.org \
    --cc=sjenning@redhat.com \
    --cc=vojtech@suse.cz \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®