Thread (43 messages) 43 messages, 6 authors, 2022-02-28

Re: [PATCH v8 04/13] module: Move livepatch support to a separate file

From: Petr Mladek <pmladek@suse.com>
Date: 2022-02-25 09:34:17
Also in: lkml

On Tue 2022-02-22 14:12:54, Aaron Tomlin wrote:
No functional change.

This patch migrates livepatch support (i.e. used during module
add/or load and remove/or deletion) from core module code into
kernel/module/livepatch.c. At the moment it contains code to
persist Elf information about a given livepatch module, only.

Signed-off-by: Aaron Tomlin <redacted>
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/livepatch.c b/kernel/module/livepatch.c
new file mode 100644
index 000000000000..486d4ff92719
--- /dev/null
+++ b/kernel/module/livepatch.c
@@ -0,0 +1,74 @@
+ * Persist Elf information about a module. Copy the Elf header,
+ * section header table, section string table, and symtab section
+ * index from info to mod->klp_info.
+ */
+int copy_module_elf(struct module *mod, struct load_info *info)
+{
+	unsigned int size, symndx;
+	int ret;
+
+	size = sizeof(*mod->klp_info);
+	mod->klp_info = kmalloc(size, GFP_KERNEL);
+	if (!mod->klp_info)
+		return -ENOMEM;
+
+	/* Elf header */
+	size = sizeof(mod->klp_info->hdr);
+	memcpy(&mod->klp_info->hdr, info->hdr, size);
+
+	/* Elf section header table */
+	size = sizeof(*info->sechdrs) * info->hdr->e_shnum;
+	mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL);
+	if (!mod->klp_info->sechdrs) {
+		ret = -ENOMEM;
+		goto free_info;
+	}
+
+	/* Elf section name string table */
+	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
+	mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL);
+	if (!mod->klp_info->secstrings) {
+		ret = -ENOMEM;
+		goto free_sechdrs;
+	}
+
+	/* Elf symbol section index */
+	symndx = info->index.sym;
+	mod->klp_info->symndx = symndx;
+
+	/*
+	 * For livepatch modules, core_kallsyms.symtab is a complete
+	 * copy of the original symbol table. Adjust sh_addr to point
+	 * to core_kallsyms.symtab since the copy of the symtab in module
+	 * init memory is freed at the end of do_init_module().
+	 */
+	mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long)mod->core_kallsyms.symtab;
+
+	return 0;
This code include several other well hidden changes:
--- del.p	2022-02-24 16:55:26.570054922 +0100
+++ add.p	2022-02-24 16:56:04.766781394 +0100
@@ -3,14 +3,14 @@
  * section header table, section string table, and symtab section
  * index from info to mod->klp_info.
  */
-static int copy_module_elf(struct module *mod, struct load_info *info)
+int copy_module_elf(struct module *mod, struct load_info *info)
 {
 	unsigned int size, symndx;
 	int ret;
 
 	size = sizeof(*mod->klp_info);
 	mod->klp_info = kmalloc(size, GFP_KERNEL);
-	if (mod->klp_info == NULL)
+	if (!mod->klp_info)
 		return -ENOMEM;
 
 	/* Elf header */
@@ -20,7 +20,7 @@ static int copy_module_elf(struct module
 	/* Elf section header table */
 	size = sizeof(*info->sechdrs) * info->hdr->e_shnum;
 	mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL);
-	if (mod->klp_info->sechdrs == NULL) {
+	if (!mod->klp_info->sechdrs) {
 		ret = -ENOMEM;
 		goto free_info;
 	}
@@ -28,7 +28,7 @@ static int copy_module_elf(struct module
 	/* Elf section name string table */
 	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
 	mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL);
-	if (mod->klp_info->secstrings == NULL) {
+	if (!mod->klp_info->secstrings) {
 		ret = -ENOMEM;
 		goto free_sechdrs;
 	}
@@ -43,8 +43,7 @@ static int copy_module_elf(struct module
 	 * to core_kallsyms.symtab since the copy of the symtab in module
 	 * init memory is freed at the end of do_init_module().
 	 */
-	mod->klp_info->sechdrs[symndx].sh_addr = \
-		(unsigned long) mod->core_kallsyms.symtab;
+	mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long)mod->core_kallsyms.symtab;
 
 	return 0;

Please do not do these small coding style changes. It complicates the
review and increases the risk of regressions. Different people
have different preferences. Just imagine that every half a year
someone update style of a code by his personal preferences. The
real changes will then get lost in a lot of noise.

Coding style changes might be acceptable only when the code is
reworked or when it significantly improves readability.


That said. I reviewed and tested this patch and did not find any
problem. Feel free to use:

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

Please, take the above as an advice for your future work.

Best Regards,
Petr
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help