[PATCH 2/2] livepatch: do relocation when initializing the patch

[Petr Mladek]

I think that the relocation is done in klp_enable_object() because
it safes the duplication in klp_module_notify_coming().

But I think that the relocation logically belongs to the init phase.
It will remove duplicate relocation if we allow repeated enable/disable
calls for the same patch. Also it removes the extra "pmod" parameter
from klp_enable_object() and makes it more symmetric with klp_disable_object().

This patch moves also the klp_find_object_module() to the init phase
where it belongs.

Finally, it moves some checks from callers to klp_write_object_relocation().
They must be there in each case. It makes the code easier when calling
from more locations.

Signed-off-by: Petr Mladek <pmladek@xxxxxxx>

---
 kernel/livepatch/core.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 9b1601729014..688a6b66e72f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -204,6 +204,14 @@ static int klp_write_object_relocations(struct module *pmod,
 	int ret;
 	struct klp_reloc *reloc;
 
+	/* nope when the patched module has not been loaded yet */
+	if (obj->name && !obj->mod)
+		return 0;
+
+	/* be happy when there is nothing to do */
+	if (!obj->relocs)
+		return 0;
+
 	for (reloc = obj->relocs; reloc->name; reloc++) {
 		if (!obj->name) {
 			ret = klp_verify_vmlinux_symbol(reloc->name,
@@ -313,7 +321,7 @@ static int klp_disable_object(struct klp_object *obj)
 	return 0;
 }
 
-static int klp_enable_object(struct module *pmod, struct klp_object *obj)
+static int klp_enable_object(struct klp_object *obj)
 {
 	struct klp_func *func;
 	int ret;
@@ -322,12 +330,6 @@ static int klp_enable_object(struct module *pmod, struct klp_object *obj)
 	if (obj->name && !obj->mod)
 		return 0;
 
-	if (obj->relocs) {
-		ret = klp_write_object_relocations(pmod, obj);
-		if (ret)
-			goto unregister;
-	}
-
 	for (func = obj->funcs; func->old_name; func++) {
 		ret = klp_find_verify_func_addr(func, obj->name);
 		if (ret)
@@ -402,8 +404,7 @@ static int __klp_enable_patch(struct klp_patch *patch)
 	pr_notice("enabling patch '%s'\n", patch->mod->name);
 
 	for (obj = patch->objs; obj->funcs; obj++) {
-		klp_find_object_module(obj);
-		ret = klp_enable_object(patch->mod, obj);
+		ret = klp_enable_object(obj);
 		if (ret)
 			goto unregister;
 	}
@@ -445,10 +446,17 @@ static void klp_module_notify_coming(struct module *pmod,
 	pr_notice("applying patch '%s' to loading module '%s'\n",
 		  pmod->name, mod->name);
 	obj->mod = mod;
-	ret = klp_enable_object(pmod, obj);
+	ret = klp_write_object_relocations(pmod, obj);
 	if (ret)
-		pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
-			pmod->name, mod->name, ret);
+		goto err;
+
+	ret = klp_enable_object(obj);
+	if (!ret)
+		return;
+
+err:
+	pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
+		pmod->name, mod->name, ret);
 }
 
 static void klp_module_notify_going(struct module *pmod,
@@ -674,15 +682,18 @@ static int klp_init_objects(struct klp_patch *patch)
 		return -EINVAL;
 
 	for (obj = patch->objs; obj->funcs; obj++) {
-		/* obj->mod set by klp_object_module_get() */
 		obj->state = KLP_DISABLED;
+		klp_find_object_module(obj);
 
-		/* sysfs */
+		ret = klp_write_object_relocations(patch->mod, obj);
+		if (ret)
+			goto free;
+
+		/* sysfs stuff */
 		obj->kobj = kobject_create_and_add(obj->name, &patch->kobj);
 		if (!obj->kobj)
 			goto free;
 
-		/* init functions */
 		ret = klp_init_funcs(obj);
 		if (ret) {
 			kobject_put(obj->kobj);
-- 
1.8.5.2


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/