[OpenWrt-Devel] [PATCH] mips: Free memory when load_module fails (#14453)

Sven Eckelmann sven at open-mesh.com
Thu Jun 25 10:32:53 EDT 2015


The mips reloc patch introduced new allocations which were done before
add_unformed_module but never freed them in case of an error. A new hook in
Linux 3.19 called module_arch_freeing_init can be used for freeing memory
which were allocated during this init phase.

The problem can be seen when trying to load a module (via busybox insmod)
when it was already loaded.

    free -m
    for i in `seq 1 100`; do
        /sbin/insmod /lib/modules/*/ath9k.ko >& /dev/null
    done
    free -m

This simple loop would leak ~3.2 MB.

Signed-off-by: Sven Eckelmann <sven at open-mesh.com>
---
 .../patches-3.18/305-mips_module_reloc.patch       |  11 +-
 ..._freeing_init-new-hook-for-archs-before-m.patch | 182 +++++++++++++++++++++
 .../patches-4.0/305-mips_module_reloc.patch        |  11 +-
 .../patches-4.1/305-mips_module_reloc.patch        |  11 +-
 4 files changed, 200 insertions(+), 15 deletions(-)
 create mode 100644 target/linux/generic/patches-3.18/311-module_arch_freeing_init-new-hook-for-archs-before-m.patch

diff --git a/target/linux/generic/patches-3.18/305-mips_module_reloc.patch b/target/linux/generic/patches-3.18/305-mips_module_reloc.patch
index 41cf806..f8ca914 100644
--- a/target/linux/generic/patches-3.18/305-mips_module_reloc.patch
+++ b/target/linux/generic/patches-3.18/305-mips_module_reloc.patch
@@ -317,7 +317,7 @@
  
  	return 0;
  }
-@@ -287,11 +529,32 @@ int module_finalize(const Elf_Ehdr *hdr,
+@@ -287,9 +529,33 @@ int module_finalize(const Elf_Ehdr *hdr,
  		list_add(&me->arch.dbe_list, &dbe_list);
  		spin_unlock_irq(&dbe_lock);
  	}
@@ -336,8 +336,8 @@
  	return 0;
  }
  
- void module_arch_cleanup(struct module *mod)
- {
++void module_arch_freeing_init(struct module *mod)
++{
 +	if (mod->arch.phys_plt_tbl) {
 +		__module_free(mod->arch.phys_plt_tbl);
 +		mod->arch.phys_plt_tbl = NULL;
@@ -346,7 +346,8 @@
 +		__module_free(mod->arch.virt_plt_tbl);
 +		mod->arch.virt_plt_tbl = NULL;
 +	}
++}
 +
+ void module_arch_cleanup(struct module *mod)
+ {
  	spin_lock_irq(&dbe_lock);
- 	list_del(&mod->arch.dbe_list);
- 	spin_unlock_irq(&dbe_lock);
diff --git a/target/linux/generic/patches-3.18/311-module_arch_freeing_init-new-hook-for-archs-before-m.patch b/target/linux/generic/patches-3.18/311-module_arch_freeing_init-new-hook-for-archs-before-m.patch
new file mode 100644
index 0000000..05cb258
--- /dev/null
+++ b/target/linux/generic/patches-3.18/311-module_arch_freeing_init-new-hook-for-archs-before-m.patch
@@ -0,0 +1,182 @@
+From: Rusty Russell <rusty at rustcorp.com.au>
+Date: Tue, 20 Jan 2015 09:07:04 +1030
+Subject: [PATCH] module_arch_freeing_init(): new hook for archs before module->module_init freed.
+
+Archs have been abusing module_free() to clean up their arch-specific
+allocations.  Since module_free() is also (ab)used by BPF and trace code,
+let's keep it to simple allocations, and provide a hook called before
+that.
+
+This means that avr32, ia64, parisc and s390 no longer need to implement
+their own module_free() at all.  avr32 doesn't need module_finalize()
+either.
+
+Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
+Cc: Chris Metcalf <cmetcalf at ezchip.com>
+Cc: Haavard Skinnemoen <hskinnemoen at gmail.com>
+Cc: Hans-Christian Egtvedt <egtvedt at samfundet.no>
+Cc: Tony Luck <tony.luck at intel.com>
+Cc: Fenghua Yu <fenghua.yu at intel.com>
+Cc: "James E.J. Bottomley" <jejb at parisc-linux.org>
+Cc: Helge Deller <deller at gmx.de>
+Cc: Martin Schwidefsky <schwidefsky at de.ibm.com>
+Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
+Cc: linux-kernel at vger.kernel.org
+Cc: linux-ia64 at vger.kernel.org
+Cc: linux-parisc at vger.kernel.org
+Cc: linux-s390 at vger.kernel.org
+
+Origin: backport, https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d453cded05ee219b77815ea194dc36efa5398bca
+---
+ arch/avr32/kernel/module.c   | 13 +------------
+ arch/ia64/kernel/module.c    |  6 ++----
+ arch/parisc/kernel/module.c  |  6 +-----
+ arch/s390/kernel/module.c    | 10 +++-------
+ arch/tile/kernel/module.c    |  2 +-
+ include/linux/moduleloader.h |  2 ++
+ kernel/module.c              |  7 +++++++
+ 7 files changed, 17 insertions(+), 29 deletions(-)
+
+--- a/arch/avr32/kernel/module.c
++++ b/arch/avr32/kernel/module.c
+@@ -19,12 +19,10 @@
+ #include <linux/moduleloader.h>
+ #include <linux/vmalloc.h>
+ 
+-void module_free(struct module *mod, void *module_region)
++void module_arch_freeing_init(struct module *mod)
+ {
+ 	vfree(mod->arch.syminfo);
+ 	mod->arch.syminfo = NULL;
+-
+-	vfree(module_region);
+ }
+ 
+ static inline int check_rela(Elf32_Rela *rela, struct module *module,
+@@ -291,12 +289,3 @@ int apply_relocate_add(Elf32_Shdr *sechd
+ 
+ 	return ret;
+ }
+-
+-int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
+-		    struct module *module)
+-{
+-	vfree(module->arch.syminfo);
+-	module->arch.syminfo = NULL;
+-
+-	return 0;
+-}
+--- a/arch/ia64/kernel/module.c
++++ b/arch/ia64/kernel/module.c
+@@ -305,14 +305,12 @@ plt_target (struct plt_entry *plt)
+ #endif /* !USE_BRL */
+ 
+ void
+-module_free (struct module *mod, void *module_region)
++module_arch_freeing_init (struct module *mod)
+ {
+-	if (mod && mod->arch.init_unw_table &&
+-	    module_region == mod->module_init) {
++	if (mod->arch.init_unw_table) {
+ 		unw_remove_unwind_table(mod->arch.init_unw_table);
+ 		mod->arch.init_unw_table = NULL;
+ 	}
+-	vfree(module_region);
+ }
+ 
+ /* Have we already seen one of these relocations? */
+--- a/arch/parisc/kernel/module.c
++++ b/arch/parisc/kernel/module.c
+@@ -298,14 +298,10 @@ static inline unsigned long count_stubs(
+ }
+ #endif
+ 
+-
+-/* Free memory returned from module_alloc */
+-void module_free(struct module *mod, void *module_region)
++void module_arch_freeing_init(struct module *mod)
+ {
+ 	kfree(mod->arch.section);
+ 	mod->arch.section = NULL;
+-
+-	vfree(module_region);
+ }
+ 
+ /* Additional bytes needed in front of individual sections */
+--- a/arch/s390/kernel/module.c
++++ b/arch/s390/kernel/module.c
+@@ -55,14 +55,10 @@ void *module_alloc(unsigned long size)
+ }
+ #endif
+ 
+-/* Free memory returned from module_alloc */
+-void module_free(struct module *mod, void *module_region)
++void module_arch_freeing_init(struct module *mod)
+ {
+-	if (mod) {
+-		vfree(mod->arch.syminfo);
+-		mod->arch.syminfo = NULL;
+-	}
+-	vfree(module_region);
++	vfree(mod->arch.syminfo);
++	mod->arch.syminfo = NULL;
+ }
+ 
+ static void
+--- a/arch/tile/kernel/module.c
++++ b/arch/tile/kernel/module.c
+@@ -83,7 +83,7 @@ void module_free(struct module *mod, voi
+ 		     0, 0, 0, NULL, NULL, 0);
+ 
+ 	/*
+-	 * FIXME: If module_region == mod->module_init, trim exception
++	 * FIXME: Add module_arch_freeing_init to trim exception
+ 	 * table entries.
+ 	 */
+ }
+--- a/include/linux/moduleloader.h
++++ b/include/linux/moduleloader.h
+@@ -80,4 +80,6 @@ int module_finalize(const Elf_Ehdr *hdr,
+ /* Any cleanup needed when module leaves. */
+ void module_arch_cleanup(struct module *mod);
+ 
++/* Any cleanup before freeing mod->module_init */
++void module_arch_freeing_init(struct module *mod);
+ #endif
+--- a/kernel/module.c
++++ b/kernel/module.c
+@@ -1855,6 +1855,10 @@ void __weak module_arch_cleanup(struct m
+ {
+ }
+ 
++void __weak module_arch_freeing_init(struct module *mod)
++{
++}
++
+ /* Free a module, remove from lists, etc. */
+ static void free_module(struct module *mod)
+ {
+@@ -1880,6 +1884,7 @@ static void free_module(struct module *m
+ 
+ 	/* This may be NULL, but that's OK */
+ 	unset_module_init_ro_nx(mod);
++	module_arch_freeing_init(mod);
+ 	module_free(mod, mod->module_init);
+ 	kfree(mod->args);
+ 	percpu_modfree(mod);
+@@ -2983,6 +2988,7 @@ out:
+ static void module_deallocate(struct module *mod, struct load_info *info)
+ {
+ 	percpu_modfree(mod);
++	module_arch_freeing_init(mod);
+ 	module_free(mod, mod->module_init);
+ 	module_free(mod, mod->module_core);
+ }
+@@ -3122,6 +3128,7 @@ static int do_init_module(struct module
+ 	mod->strtab = mod->core_strtab;
+ #endif
+ 	unset_module_init_ro_nx(mod);
++	module_arch_freeing_init(mod);
+ 	module_free(mod, mod->module_init);
+ 	mod->module_init = NULL;
+ 	mod->init_size = 0;
diff --git a/target/linux/generic/patches-4.0/305-mips_module_reloc.patch b/target/linux/generic/patches-4.0/305-mips_module_reloc.patch
index 87c205c..8b3975f 100644
--- a/target/linux/generic/patches-4.0/305-mips_module_reloc.patch
+++ b/target/linux/generic/patches-4.0/305-mips_module_reloc.patch
@@ -316,7 +316,7 @@
  
  	return 0;
  }
-@@ -287,11 +528,32 @@ int module_finalize(const Elf_Ehdr *hdr,
+@@ -287,9 +528,33 @@ int module_finalize(const Elf_Ehdr *hdr,
  		list_add(&me->arch.dbe_list, &dbe_list);
  		spin_unlock_irq(&dbe_lock);
  	}
@@ -335,8 +335,8 @@
  	return 0;
  }
  
- void module_arch_cleanup(struct module *mod)
- {
++void module_arch_freeing_init(struct module *mod)
++{
 +	if (mod->arch.phys_plt_tbl) {
 +		__module_free(mod->arch.phys_plt_tbl);
 +		mod->arch.phys_plt_tbl = NULL;
@@ -345,7 +345,8 @@
 +		__module_free(mod->arch.virt_plt_tbl);
 +		mod->arch.virt_plt_tbl = NULL;
 +	}
++}
 +
+ void module_arch_cleanup(struct module *mod)
+ {
  	spin_lock_irq(&dbe_lock);
- 	list_del(&mod->arch.dbe_list);
- 	spin_unlock_irq(&dbe_lock);
diff --git a/target/linux/generic/patches-4.1/305-mips_module_reloc.patch b/target/linux/generic/patches-4.1/305-mips_module_reloc.patch
index 87c205c..8b3975f 100644
--- a/target/linux/generic/patches-4.1/305-mips_module_reloc.patch
+++ b/target/linux/generic/patches-4.1/305-mips_module_reloc.patch
@@ -316,7 +316,7 @@
  
  	return 0;
  }
-@@ -287,11 +528,32 @@ int module_finalize(const Elf_Ehdr *hdr,
+@@ -287,9 +528,33 @@ int module_finalize(const Elf_Ehdr *hdr,
  		list_add(&me->arch.dbe_list, &dbe_list);
  		spin_unlock_irq(&dbe_lock);
  	}
@@ -335,8 +335,8 @@
  	return 0;
  }
  
- void module_arch_cleanup(struct module *mod)
- {
++void module_arch_freeing_init(struct module *mod)
++{
 +	if (mod->arch.phys_plt_tbl) {
 +		__module_free(mod->arch.phys_plt_tbl);
 +		mod->arch.phys_plt_tbl = NULL;
@@ -345,7 +345,8 @@
 +		__module_free(mod->arch.virt_plt_tbl);
 +		mod->arch.virt_plt_tbl = NULL;
 +	}
++}
 +
+ void module_arch_cleanup(struct module *mod)
+ {
  	spin_lock_irq(&dbe_lock);
- 	list_del(&mod->arch.dbe_list);
- 	spin_unlock_irq(&dbe_lock);
-- 
2.1.4
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list