[PATCH modules 0/2] Fix handling of exit unwinding sections (on ARM)

[PATCH modules 0/2] Fix handling of exit unwinding sections (on ARM)

[Matthias Schiffer]

For some time (050d18d1c651 "ARM: 8650/1: module: handle negative
R_ARM_PREL31 addends correctly", v4.11+), building a kernel without
CONFIG_MODULE_UNLOAD would lead to module loads failing on ARM systems with
certain memory layouts, with messages like:

  imx_sdma: section 16 reloc 0 sym '': relocation 42 out of range
  (0x7f015260 -> 0xc0f5a5e8)

(0x7f015260 is in the module load area, 0xc0f5a5e8 a regular vmalloc
address; relocation 42 is R_ARM_PREL31)

This is caused by relocatiosn in the .ARM.extab.exit.text and
.ARM.exidx.exit.text sections referencing the .exit.text section. As the
module loader will omit loading .exit.text without CONFIG_MODULE_UNLOAD,
there will be relocations from loaded to unloaded sections; the resulting
huge offsets trigger the sanity checks added in 050d18d1c651.

IA64 might be affected by a similar issue - sections with names like
.IA_64.unwind.exit.text and .IA_64.unwind_info.exit.text appear in the ld
script - but I don't know much about that arch.

Also, I'm not sure if this is stable-worthy - just enabling
CONFIG_MODULE_UNLOAD should be a viable workaround on affected kernels.


Kind regards,
Matthias


Matthias Schiffer (2):
  module: allow arch overrides for .exit section names
  ARM: module: recognize unwind exit sections

 arch/arm/include/asm/module.h | 11 +++++++++++
 include/linux/moduleloader.h  |  8 ++++++++
 kernel/module.c               |  2 +-
 3 files changed, 20 insertions(+), 1 deletion(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Matthias Schiffer]

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
+
 #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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jessica Yu]

+++ 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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Matthias Schiffer]

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
> > 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH modules 2/2] ARM: module: recognize unwind exit sections

[Matthias Schiffer]

In addition to the prefix ".exit", ".ARM.extab.exit" and ".ARM.exidx.exit"
must be recognizes as exit sections as well. Otherwise, loading modules can
fail without CONFIG_MODULE_UNLOAD depending on the memory layout, when
relocations for the unwind sections refer to the .exit.text section:

  imx_sdma: section 16 reloc 0 sym '': relocation 42 out of range
  (0x7f015260 -> 0xc0f5a5e8)

where 0x7F000000 is the module load area and 0xC0000000 is the vmalloc
area. Relocation 42 refers to R_ARM_PREL31, which is limited to signed
31bit offsets.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 arch/arm/include/asm/module.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index 182163b55546..f3401758d711 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -4,6 +4,8 @@
 
 #include <asm-generic/module.h>
 
+#include <linux/string.h>
+
 struct unwind_table;
 
 #ifdef CONFIG_ARM_UNWIND
@@ -72,4 +74,13 @@ static inline unsigned long kallsyms_symbol_value(const Elf_Sym *sym)
 }
 #endif
 
+#define HAVE_ARCH_MODULE_EXIT_SECTION
+static inline bool module_exit_section(const char *name)
+{
+	return strstarts(name, ".exit") ||
+		strstarts(name, ".ARM.extab.exit") ||
+		strstarts(name, ".ARM.exidx.exit");
+
+}
+
 #endif /* _ASM_ARM_MODULE_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH modules 0/2] Fix handling of exit unwinding sections (on ARM)

[Matthias Schiffer]

On Mon, 2019-06-03 at 12:57 +0200, Matthias Schiffer wrote:
> For some time (050d18d1c651 "ARM: 8650/1: module: handle negative
> R_ARM_PREL31 addends correctly", v4.11+), building a kernel without
> CONFIG_MODULE_UNLOAD would lead to module loads failing on ARM
> systems with
> certain memory layouts, with messages like:
> 
>   imx_sdma: section 16 reloc 0 sym '': relocation 42 out of range
>   (0x7f015260 -> 0xc0f5a5e8)
> 
> (0x7f015260 is in the module load area, 0xc0f5a5e8 a regular vmalloc
> address; relocation 42 is R_ARM_PREL31)
> 
> This is caused by relocatiosn in the .ARM.extab.exit.text and
> .ARM.exidx.exit.text sections referencing the .exit.text section. As
> the
> module loader will omit loading .exit.text without
> CONFIG_MODULE_UNLOAD,
> there will be relocations from loaded to unloaded sections; the
> resulting
> huge offsets trigger the sanity checks added in 050d18d1c651.
> 
> IA64 might be affected by a similar issue - sections with names like
> .IA_64.unwind.exit.text and .IA_64.unwind_info.exit.text appear in
> the ld
> script - but I don't know much about that arch.
> 
> Also, I'm not sure if this is stable-worthy - just enabling
> CONFIG_MODULE_UNLOAD should be a viable workaround on affected
> kernels.
> 
> 
> Kind regards,
> Matthias


Hi,
any comments on these patches? If not, who is going to take them in
their tree?

Note that I'll be out of office for the next week, and I won't be able
to read my mail during this period.


Kind regards,
Matthias




> 
> 
> Matthias Schiffer (2):
>   module: allow arch overrides for .exit section names
>   ARM: module: recognize unwind exit sections
> 
>  arch/arm/include/asm/module.h | 11 +++++++++++
>  include/linux/moduleloader.h  |  8 ++++++++
>  kernel/module.c               |  2 +-
>  3 files changed, 20 insertions(+), 1 deletion(-)
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH modules 0/2] Fix handling of exit unwinding sections (on ARM)

[Jessica Yu]

+++ Matthias Schiffer [06/06/19 10:14 +0200]:
>On Mon, 2019-06-03 at 12:57 +0200, Matthias Schiffer wrote:
>> For some time (050d18d1c651 "ARM: 8650/1: module: handle negative
>> R_ARM_PREL31 addends correctly", v4.11+), building a kernel without
>> CONFIG_MODULE_UNLOAD would lead to module loads failing on ARM
>> systems with
>> certain memory layouts, with messages like:
>>
>>   imx_sdma: section 16 reloc 0 sym '': relocation 42 out of range
>>   (0x7f015260 -> 0xc0f5a5e8)
>>
>> (0x7f015260 is in the module load area, 0xc0f5a5e8 a regular vmalloc
>> address; relocation 42 is R_ARM_PREL31)
>>
>> This is caused by relocatiosn in the .ARM.extab.exit.text and
>> .ARM.exidx.exit.text sections referencing the .exit.text section. As
>> the
>> module loader will omit loading .exit.text without
>> CONFIG_MODULE_UNLOAD,
>> there will be relocations from loaded to unloaded sections; the
>> resulting
>> huge offsets trigger the sanity checks added in 050d18d1c651.
>>
>> IA64 might be affected by a similar issue - sections with names like
>> .IA_64.unwind.exit.text and .IA_64.unwind_info.exit.text appear in
>> the ld
>> script - but I don't know much about that arch.
>>
>> Also, I'm not sure if this is stable-worthy - just enabling
>> CONFIG_MODULE_UNLOAD should be a viable workaround on affected
>> kernels.
>>
>>
>> Kind regards,
>> Matthias
>
>
>Hi,
>any comments on these patches? If not, who is going to take them in
>their tree?

I don't mind either way. I can take the patches through my tree if
Russell ack's the second one (after comments have been addressed).

Thanks!

Jessica

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel