diff mbox series

[modules,1/2] module: allow arch overrides for .exit section names

Message ID 20190603105726.22436-2-matthias.schiffer@ew.tq-group.com (mailing list archive)
State New, archived
Headers show
Series Fix handling of exit unwinding sections (on ARM) | expand

Commit Message

Matthias Schiffer June 3, 2019, 10:57 a.m. UTC
Some archs like ARM store unwind information for .exit.text in sections
with unusual names. As this unwind information refers to .exit.text, it
must not be loaded when .exit.text is not loaded (when CONFIG_MODULE_UNLOAD
is unset); otherwise, loading a module can fail due to relocation failures.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 include/linux/moduleloader.h | 8 ++++++++
 kernel/module.c              | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Jessica Yu June 6, 2019, 3:09 p.m. UTC | #1
+++ Matthias Schiffer [03/06/19 12:57 +0200]:
>Some archs like ARM store unwind information for .exit.text in sections
>with unusual names. As this unwind information refers to .exit.text, it
>must not be loaded when .exit.text is not loaded (when CONFIG_MODULE_UNLOAD
>is unset); otherwise, loading a module can fail due to relocation failures.
>
>Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
>---
> include/linux/moduleloader.h | 8 ++++++++
> kernel/module.c              | 2 +-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
>index 31013c2effd3..cddbd85fb659 100644
>--- a/include/linux/moduleloader.h
>+++ b/include/linux/moduleloader.h
>@@ -5,6 +5,7 @@
>
> #include <linux/module.h>
> #include <linux/elf.h>
>+#include <linux/string.h>
>
> /* These may be implemented by architectures that need to hook into the
>  * module loader code.  Architectures that don't need to do anything special
>@@ -93,4 +94,11 @@ void module_arch_freeing_init(struct module *mod);
> #define MODULE_ALIGN PAGE_SIZE
> #endif
>
>+#ifndef HAVE_ARCH_MODULE_EXIT_SECTION
>+static inline bool module_exit_section(const char *name)
>+{
>+	return strstarts(name, ".exit");
>+}
>+#endif
>+

Hi Matthias,

For sake of consistency, could we implement this as an arch-overridable
__weak symbol like the rest of the module arch-overrides in moduleloader.h?

> #endif
>diff --git a/kernel/module.c b/kernel/module.c
>index 6e6712b3aaf5..e8e4cd0a471f 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -2924,7 +2924,7 @@ static int rewrite_section_headers(struct load_info *info, int flags)
>
> #ifndef CONFIG_MODULE_UNLOAD
> 		/* Don't load .exit sections */
>-		if (strstarts(info->secstrings+shdr->sh_name, ".exit"))
>+		if (module_exit_section(info->secstrings+shdr->sh_name))
> 			shdr->sh_flags &= ~(unsigned long)SHF_ALLOC;
> #endif
> 	}
>-- 
>2.17.1
>
Matthias Schiffer June 6, 2019, 3:27 p.m. UTC | #2
On Thu, 2019-06-06 at 17:09 +0200, Jessica Yu wrote:
> +++ Matthias Schiffer [03/06/19 12:57 +0200]:
> > Some archs like ARM store unwind information for .exit.text in
> > sections
> > with unusual names. As this unwind information refers to
> > .exit.text, it
> > must not be loaded when .exit.text is not loaded (when
> > CONFIG_MODULE_UNLOAD
> > is unset); otherwise, loading a module can fail due to relocation
> > failures.
> > 
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
> > >
> > ---
> > include/linux/moduleloader.h | 8 ++++++++
> > kernel/module.c              | 2 +-
> > 2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/moduleloader.h
> > b/include/linux/moduleloader.h
> > index 31013c2effd3..cddbd85fb659 100644
> > --- a/include/linux/moduleloader.h
> > +++ b/include/linux/moduleloader.h
> > @@ -5,6 +5,7 @@
> > 
> > #include <linux/module.h>
> > #include <linux/elf.h>
> > +#include <linux/string.h>
> > 
> > /* These may be implemented by architectures that need to hook into
> > the
> >  * module loader code.  Architectures that don't need to do
> > anything special
> > @@ -93,4 +94,11 @@ void module_arch_freeing_init(struct module
> > *mod);
> > #define MODULE_ALIGN PAGE_SIZE
> > #endif
> > 
> > +#ifndef HAVE_ARCH_MODULE_EXIT_SECTION
> > +static inline bool module_exit_section(const char *name)
> > +{
> > +	return strstarts(name, ".exit");
> > +}
> > +#endif
> > +
> 
> Hi Matthias,
> 
> For sake of consistency, could we implement this as an arch-
> overridable
> __weak symbol like the rest of the module arch-overrides in
> moduleloader.h?

Fine with me - making such a tiny function inlineable just seemed more
appropriate to me. I can send a v2 tomorrow.


> 
> > #endif
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 6e6712b3aaf5..e8e4cd0a471f 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2924,7 +2924,7 @@ static int rewrite_section_headers(struct
> > load_info *info, int flags)
> > 
> > #ifndef CONFIG_MODULE_UNLOAD
> > 		/* Don't load .exit sections */
> > -		if (strstarts(info->secstrings+shdr->sh_name, ".exit"))
> > +		if (module_exit_section(info->secstrings+shdr-
> > >sh_name))
> > 			shdr->sh_flags &= ~(unsigned long)SHF_ALLOC;
> > #endif
> > 	}
> > -- 
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
index 31013c2effd3..cddbd85fb659 100644
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -5,6 +5,7 @@ 
 
 #include <linux/module.h>
 #include <linux/elf.h>
+#include <linux/string.h>
 
 /* These may be implemented by architectures that need to hook into the
  * module loader code.  Architectures that don't need to do anything special
@@ -93,4 +94,11 @@  void module_arch_freeing_init(struct module *mod);
 #define MODULE_ALIGN PAGE_SIZE
 #endif
 
+#ifndef HAVE_ARCH_MODULE_EXIT_SECTION
+static inline bool module_exit_section(const char *name)
+{
+	return strstarts(name, ".exit");
+}
+#endif
+
 #endif
diff --git a/kernel/module.c b/kernel/module.c
index 6e6712b3aaf5..e8e4cd0a471f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2924,7 +2924,7 @@  static int rewrite_section_headers(struct load_info *info, int flags)
 
 #ifndef CONFIG_MODULE_UNLOAD
 		/* Don't load .exit sections */
-		if (strstarts(info->secstrings+shdr->sh_name, ".exit"))
+		if (module_exit_section(info->secstrings+shdr->sh_name))
 			shdr->sh_flags &= ~(unsigned long)SHF_ALLOC;
 #endif
 	}