[PATCH v4] ARM: module: Add all unwind tables when load module

[PATCH v4] ARM: module: Add all unwind tables when load module

[Chen Zhongjin]

For EABI stack unwinding, when loading .ko module
the EXIDX sections will be added to a unwind_table list.

However not all EXIDX sections are added because EXIDX
sections are searched by hardcoded section names.

For functions in other sections such as .ref.text
or .kprobes.text, gcc generates seprated EXIDX sections
(such as .ARM.exidx.ref.text or .ARM.exidx.kprobes.text).

These extra EXIDX sections are not loaded, so when unwinding
functions in these sections, we will failed with:

	unwind: Index not found xxx

To fix that, I refactor the code for searching and adding
EXIDX sections:

- Check section type to search EXIDX tables (0x70000001)
instead of strcmp() the hardcoded names. Then find the
corresponding text sections by their section names.

- Add a unwind_table list in module->arch to save their own
unwind_table instead of the fixed-lenth array.

- Save .ARM.exidx.init.text section ptr, because it should
be cleaned after module init.

Now all EXIDX sections of .ko can be added correctly.

Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
---
Changes v3 -> v4:
- Fix indent by replace tab with code.

Changes v2 -> v3:
- Unwind "txtname" and add some blank, to make code more clear.

Changes v1 -> v2:
- Rename "table_list" to "unwind_list" for consistency.
- Drop unnecessary locals variable "txtname" and "sectype".
- Merge condition checks for sh_flags and sh_type.
---
 arch/arm/include/asm/module.h | 17 ++------
 arch/arm/include/asm/unwind.h |  1 +
 arch/arm/kernel/module.c      | 78 ++++++++++++++++++-----------------
 3 files changed, 45 insertions(+), 51 deletions(-)

diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index cfffae67c04e..8139b6a33a22 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -3,20 +3,10 @@
 #define _ASM_ARM_MODULE_H
 
 #include <asm-generic/module.h>
-
-struct unwind_table;
+#include <asm/unwind.h>
 
 #ifdef CONFIG_ARM_UNWIND
-enum {
-	ARM_SEC_INIT,
-	ARM_SEC_DEVINIT,
-	ARM_SEC_CORE,
-	ARM_SEC_EXIT,
-	ARM_SEC_DEVEXIT,
-	ARM_SEC_HOT,
-	ARM_SEC_UNLIKELY,
-	ARM_SEC_MAX,
-};
+#define ELF_SECTION_UNWIND 0x70000001
 #endif
 
 #define PLT_ENT_STRIDE		L1_CACHE_BYTES
@@ -36,7 +26,8 @@ struct mod_plt_sec {
 
 struct mod_arch_specific {
 #ifdef CONFIG_ARM_UNWIND
-	struct unwind_table *unwind[ARM_SEC_MAX];
+	struct unwind_table unwind_list;
+	struct unwind_table *init_table;
 #endif
 #ifdef CONFIG_ARM_MODULE_PLTS
 	struct mod_plt_sec	core;
diff --git a/arch/arm/include/asm/unwind.h b/arch/arm/include/asm/unwind.h
index 0f8a3439902d..b51f85417f58 100644
--- a/arch/arm/include/asm/unwind.h
+++ b/arch/arm/include/asm/unwind.h
@@ -24,6 +24,7 @@ struct unwind_idx {
 
 struct unwind_table {
 	struct list_head list;
+	struct list_head mod_list;
 	const struct unwind_idx *start;
 	const struct unwind_idx *origin;
 	const struct unwind_idx *stop;
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 549abcedf795..b98bcb47e337 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -459,46 +459,41 @@ int module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs,
 #ifdef CONFIG_ARM_UNWIND
 	const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
 	const Elf_Shdr *sechdrs_end = sechdrs + hdr->e_shnum;
-	struct mod_unwind_map maps[ARM_SEC_MAX];
-	int i;
+	struct unwind_table *unwind_list = &mod->arch.unwind_list;
 
-	memset(maps, 0, sizeof(maps));
+	INIT_LIST_HEAD(&unwind_list->mod_list);
+	mod->arch.init_table = NULL;
 
 	for (s = sechdrs; s < sechdrs_end; s++) {
 		const char *secname = secstrs + s->sh_name;
+		const char *txtname;
+		const Elf_Shdr *txt_sec;
 
-		if (!(s->sh_flags & SHF_ALLOC))
+		if (!strcmp(".ARM.exidx", secname))
+			txtname = ".text";
+		else
+			txtname = secname + strlen(".ARM.exidx");
+
+		txt_sec = find_mod_section(hdr, sechdrs, txtname);
+
+		if (!(s->sh_flags & SHF_ALLOC) ||
+		    s->sh_type != ELF_SECTION_UNWIND)
 			continue;
 
-		if (strcmp(".ARM.exidx.init.text", secname) == 0)
-			maps[ARM_SEC_INIT].unw_sec = s;
-		else if (strcmp(".ARM.exidx", secname) == 0)
-			maps[ARM_SEC_CORE].unw_sec = s;
-		else if (strcmp(".ARM.exidx.exit.text", secname) == 0)
-			maps[ARM_SEC_EXIT].unw_sec = s;
-		else if (strcmp(".ARM.exidx.text.unlikely", secname) == 0)
-			maps[ARM_SEC_UNLIKELY].unw_sec = s;
-		else if (strcmp(".ARM.exidx.text.hot", secname) == 0)
-			maps[ARM_SEC_HOT].unw_sec = s;
-		else if (strcmp(".init.text", secname) == 0)
-			maps[ARM_SEC_INIT].txt_sec = s;
-		else if (strcmp(".text", secname) == 0)
-			maps[ARM_SEC_CORE].txt_sec = s;
-		else if (strcmp(".exit.text", secname) == 0)
-			maps[ARM_SEC_EXIT].txt_sec = s;
-		else if (strcmp(".text.unlikely", secname) == 0)
-			maps[ARM_SEC_UNLIKELY].txt_sec = s;
-		else if (strcmp(".text.hot", secname) == 0)
-			maps[ARM_SEC_HOT].txt_sec = s;
-	}
+		if (txt_sec) {
+			struct unwind_table *table =
+				unwind_table_add(s->sh_addr,
+						s->sh_size,
+						txt_sec->sh_addr,
+						txt_sec->sh_size);
+
+			list_add(&table->mod_list, &unwind_list->mod_list);
 
-	for (i = 0; i < ARM_SEC_MAX; i++)
-		if (maps[i].unw_sec && maps[i].txt_sec)
-			mod->arch.unwind[i] =
-				unwind_table_add(maps[i].unw_sec->sh_addr,
-					         maps[i].unw_sec->sh_size,
-					         maps[i].txt_sec->sh_addr,
-					         maps[i].txt_sec->sh_size);
+			/* save init table for module_arch_freeing_init */
+			if (strcmp(".ARM.exidx.init.text", secname) == 0)
+				mod->arch.init_table = table;
+		}
+	}
 #endif
 #ifdef CONFIG_ARM_PATCH_PHYS_VIRT
 	s = find_mod_section(hdr, sechdrs, ".pv_table");
@@ -519,11 +514,13 @@ void
 module_arch_cleanup(struct module *mod)
 {
 #ifdef CONFIG_ARM_UNWIND
-	int i;
+	struct unwind_table *tmp;
+	struct unwind_table *n;
 
-	for (i = 0; i < ARM_SEC_MAX; i++) {
-		unwind_table_del(mod->arch.unwind[i]);
-		mod->arch.unwind[i] = NULL;
+	list_for_each_entry_safe(tmp, n,
+			&mod->arch.unwind_list.mod_list, mod_list) {
+		list_del(&tmp->mod_list);
+		unwind_table_del(tmp);
 	}
 #endif
 }
@@ -531,7 +528,12 @@ module_arch_cleanup(struct module *mod)
 void __weak module_arch_freeing_init(struct module *mod)
 {
 #ifdef CONFIG_ARM_UNWIND
-	unwind_table_del(mod->arch.unwind[ARM_SEC_INIT]);
-	mod->arch.unwind[ARM_SEC_INIT] = NULL;
+	struct unwind_table *init = mod->arch.init_table;
+
+	if (init) {
+		mod->arch.init_table = NULL;
+		list_del(&init->mod_list);
+		unwind_table_del(init);
+	}
 #endif
 }
-- 
2.17.1

Re: [PATCH v4] ARM: module: Add all unwind tables when load module

[Chen Zhongjin]

Just remind for review. Thanks!

On 2022/4/1 21:15, Chen Zhongjin wrote:
> For EABI stack unwinding, when loading .ko module
> the EXIDX sections will be added to a unwind_table list.
>
> However not all EXIDX sections are added because EXIDX
> sections are searched by hardcoded section names.
>
> For functions in other sections such as .ref.text
> or .kprobes.text, gcc generates seprated EXIDX sections
> (such as .ARM.exidx.ref.text or .ARM.exidx.kprobes.text).
>
> These extra EXIDX sections are not loaded, so when unwinding
> functions in these sections, we will failed with:
>
> 	unwind: Index not found xxx
>
> To fix that, I refactor the code for searching and adding
> EXIDX sections:
>
> - Check section type to search EXIDX tables (0x70000001)
> instead of strcmp() the hardcoded names. Then find the
> corresponding text sections by their section names.
>
> - Add a unwind_table list in module->arch to save their own
> unwind_table instead of the fixed-lenth array.
>
> - Save .ARM.exidx.init.text section ptr, because it should
> be cleaned after module init.
>
> Now all EXIDX sections of .ko can be added correctly.
>
> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
> ---
> Changes v3 -> v4:
> - Fix indent by replace tab with code.
>
> Changes v2 -> v3:
> - Unwind "txtname" and add some blank, to make code more clear.
>
> Changes v1 -> v2:
> - Rename "table_list" to "unwind_list" for consistency.
> - Drop unnecessary locals variable "txtname" and "sectype".
> - Merge condition checks for sh_flags and sh_type.
> ---
>   arch/arm/include/asm/module.h | 17 ++------
>   arch/arm/include/asm/unwind.h |  1 +
>   arch/arm/kernel/module.c      | 78 ++++++++++++++++++-----------------
>   3 files changed, 45 insertions(+), 51 deletions(-)
>
> diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
> index cfffae67c04e..8139b6a33a22 100644
> --- a/arch/arm/include/asm/module.h
> +++ b/arch/arm/include/asm/module.h
> @@ -3,20 +3,10 @@
>   #define _ASM_ARM_MODULE_H
>   
>   #include <asm-generic/module.h>
> -
> -struct unwind_table;
> +#include <asm/unwind.h>
>   
>   #ifdef CONFIG_ARM_UNWIND
> -enum {
> -	ARM_SEC_INIT,
> -	ARM_SEC_DEVINIT,
> -	ARM_SEC_CORE,
> -	ARM_SEC_EXIT,
> -	ARM_SEC_DEVEXIT,
> -	ARM_SEC_HOT,
> -	ARM_SEC_UNLIKELY,
> -	ARM_SEC_MAX,
> -};
> +#define ELF_SECTION_UNWIND 0x70000001
>   #endif
>   
>   #define PLT_ENT_STRIDE		L1_CACHE_BYTES
> @@ -36,7 +26,8 @@ struct mod_plt_sec {
>   
>   struct mod_arch_specific {
>   #ifdef CONFIG_ARM_UNWIND
> -	struct unwind_table *unwind[ARM_SEC_MAX];
> +	struct unwind_table unwind_list;
> +	struct unwind_table *init_table;
>   #endif
>   #ifdef CONFIG_ARM_MODULE_PLTS
>   	struct mod_plt_sec	core;
> diff --git a/arch/arm/include/asm/unwind.h b/arch/arm/include/asm/unwind.h
> index 0f8a3439902d..b51f85417f58 100644
> --- a/arch/arm/include/asm/unwind.h
> +++ b/arch/arm/include/asm/unwind.h
> @@ -24,6 +24,7 @@ struct unwind_idx {
>   
>   struct unwind_table {
>   	struct list_head list;
> +	struct list_head mod_list;
>   	const struct unwind_idx *start;
>   	const struct unwind_idx *origin;
>   	const struct unwind_idx *stop;
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index 549abcedf795..b98bcb47e337 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -459,46 +459,41 @@ int module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs,
>   #ifdef CONFIG_ARM_UNWIND
>   	const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>   	const Elf_Shdr *sechdrs_end = sechdrs + hdr->e_shnum;
> -	struct mod_unwind_map maps[ARM_SEC_MAX];
> -	int i;
> +	struct unwind_table *unwind_list = &mod->arch.unwind_list;
>   
> -	memset(maps, 0, sizeof(maps));
> +	INIT_LIST_HEAD(&unwind_list->mod_list);
> +	mod->arch.init_table = NULL;
>   
>   	for (s = sechdrs; s < sechdrs_end; s++) {
>   		const char *secname = secstrs + s->sh_name;
> +		const char *txtname;
> +		const Elf_Shdr *txt_sec;
>   
> -		if (!(s->sh_flags & SHF_ALLOC))
> +		if (!strcmp(".ARM.exidx", secname))
> +			txtname = ".text";
> +		else
> +			txtname = secname + strlen(".ARM.exidx");
> +
> +		txt_sec = find_mod_section(hdr, sechdrs, txtname);
> +
> +		if (!(s->sh_flags & SHF_ALLOC) ||
> +		    s->sh_type != ELF_SECTION_UNWIND)
>   			continue;
>   
> -		if (strcmp(".ARM.exidx.init.text", secname) == 0)
> -			maps[ARM_SEC_INIT].unw_sec = s;
> -		else if (strcmp(".ARM.exidx", secname) == 0)
> -			maps[ARM_SEC_CORE].unw_sec = s;
> -		else if (strcmp(".ARM.exidx.exit.text", secname) == 0)
> -			maps[ARM_SEC_EXIT].unw_sec = s;
> -		else if (strcmp(".ARM.exidx.text.unlikely", secname) == 0)
> -			maps[ARM_SEC_UNLIKELY].unw_sec = s;
> -		else if (strcmp(".ARM.exidx.text.hot", secname) == 0)
> -			maps[ARM_SEC_HOT].unw_sec = s;
> -		else if (strcmp(".init.text", secname) == 0)
> -			maps[ARM_SEC_INIT].txt_sec = s;
> -		else if (strcmp(".text", secname) == 0)
> -			maps[ARM_SEC_CORE].txt_sec = s;
> -		else if (strcmp(".exit.text", secname) == 0)
> -			maps[ARM_SEC_EXIT].txt_sec = s;
> -		else if (strcmp(".text.unlikely", secname) == 0)
> -			maps[ARM_SEC_UNLIKELY].txt_sec = s;
> -		else if (strcmp(".text.hot", secname) == 0)
> -			maps[ARM_SEC_HOT].txt_sec = s;
> -	}
> +		if (txt_sec) {
> +			struct unwind_table *table =
> +				unwind_table_add(s->sh_addr,
> +						s->sh_size,
> +						txt_sec->sh_addr,
> +						txt_sec->sh_size);
> +
> +			list_add(&table->mod_list, &unwind_list->mod_list);
>   
> -	for (i = 0; i < ARM_SEC_MAX; i++)
> -		if (maps[i].unw_sec && maps[i].txt_sec)
> -			mod->arch.unwind[i] =
> -				unwind_table_add(maps[i].unw_sec->sh_addr,
> -					         maps[i].unw_sec->sh_size,
> -					         maps[i].txt_sec->sh_addr,
> -					         maps[i].txt_sec->sh_size);
> +			/* save init table for module_arch_freeing_init */
> +			if (strcmp(".ARM.exidx.init.text", secname) == 0)
> +				mod->arch.init_table = table;
> +		}
> +	}
>   #endif
>   #ifdef CONFIG_ARM_PATCH_PHYS_VIRT
>   	s = find_mod_section(hdr, sechdrs, ".pv_table");
> @@ -519,11 +514,13 @@ void
>   module_arch_cleanup(struct module *mod)
>   {
>   #ifdef CONFIG_ARM_UNWIND
> -	int i;
> +	struct unwind_table *tmp;
> +	struct unwind_table *n;
>   
> -	for (i = 0; i < ARM_SEC_MAX; i++) {
> -		unwind_table_del(mod->arch.unwind[i]);
> -		mod->arch.unwind[i] = NULL;
> +	list_for_each_entry_safe(tmp, n,
> +			&mod->arch.unwind_list.mod_list, mod_list) {
> +		list_del(&tmp->mod_list);
> +		unwind_table_del(tmp);
>   	}
>   #endif
>   }
> @@ -531,7 +528,12 @@ module_arch_cleanup(struct module *mod)
>   void __weak module_arch_freeing_init(struct module *mod)
>   {
>   #ifdef CONFIG_ARM_UNWIND
> -	unwind_table_del(mod->arch.unwind[ARM_SEC_INIT]);
> -	mod->arch.unwind[ARM_SEC_INIT] = NULL;
> +	struct unwind_table *init = mod->arch.init_table;
> +
> +	if (init) {
> +		mod->arch.init_table = NULL;
> +		list_del(&init->mod_list);
> +		unwind_table_del(init);
> +	}
>   #endif
>   }

Re: [PATCH v4] ARM: module: Add all unwind tables when load module

[Chen Zhongjin]

ping for review, thanks!

On 2022/4/1 21:15, Chen Zhongjin wrote:
> For EABI stack unwinding, when loading .ko module
> the EXIDX sections will be added to a unwind_table list.
> 
> However not all EXIDX sections are added because EXIDX
> sections are searched by hardcoded section names.
> 
> For functions in other sections such as .ref.text
> or .kprobes.text, gcc generates seprated EXIDX sections
> (such as .ARM.exidx.ref.text or .ARM.exidx.kprobes.text).
> 
> These extra EXIDX sections are not loaded, so when unwinding
> functions in these sections, we will failed with:
> 
> 	unwind: Index not found xxx
> 
> To fix that, I refactor the code for searching and adding
> EXIDX sections:
> 
> - Check section type to search EXIDX tables (0x70000001)
> instead of strcmp() the hardcoded names. Then find the
> corresponding text sections by their section names.
> 
> - Add a unwind_table list in module->arch to save their own
> unwind_table instead of the fixed-lenth array.
> 
> - Save .ARM.exidx.init.text section ptr, because it should
> be cleaned after module init.
> 
> Now all EXIDX sections of .ko can be added correctly.
> 
> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
> ---
> Changes v3 -> v4:
> - Fix indent by replace tab with code.
> 
> Changes v2 -> v3:
> - Unwind "txtname" and add some blank, to make code more clear.
> 
> Changes v1 -> v2:
> - Rename "table_list" to "unwind_list" for consistency.
> - Drop unnecessary locals variable "txtname" and "sectype".
> - Merge condition checks for sh_flags and sh_type.
> ---
>  arch/arm/include/asm/module.h | 17 ++------
>  arch/arm/include/asm/unwind.h |  1 +
>  arch/arm/kernel/module.c      | 78 ++++++++++++++++++-----------------
>  3 files changed, 45 insertions(+), 51 deletions(-)
> 
> diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
> index cfffae67c04e..8139b6a33a22 100644
> --- a/arch/arm/include/asm/module.h
> +++ b/arch/arm/include/asm/module.h
> @@ -3,20 +3,10 @@
>  #define _ASM_ARM_MODULE_H
>  
>  #include <asm-generic/module.h>
> -
> -struct unwind_table;
> +#include <asm/unwind.h>
>  
>  #ifdef CONFIG_ARM_UNWIND
> -enum {
> -	ARM_SEC_INIT,
> -	ARM_SEC_DEVINIT,
> -	ARM_SEC_CORE,
> -	ARM_SEC_EXIT,
> -	ARM_SEC_DEVEXIT,
> -	ARM_SEC_HOT,
> -	ARM_SEC_UNLIKELY,
> -	ARM_SEC_MAX,
> -};
> +#define ELF_SECTION_UNWIND 0x70000001
>  #endif
>  
>  #define PLT_ENT_STRIDE		L1_CACHE_BYTES
> @@ -36,7 +26,8 @@ struct mod_plt_sec {
>  
>  struct mod_arch_specific {
>  #ifdef CONFIG_ARM_UNWIND
> -	struct unwind_table *unwind[ARM_SEC_MAX];
> +	struct unwind_table unwind_list;
> +	struct unwind_table *init_table;
>  #endif
>  #ifdef CONFIG_ARM_MODULE_PLTS
>  	struct mod_plt_sec	core;
> diff --git a/arch/arm/include/asm/unwind.h b/arch/arm/include/asm/unwind.h
> index 0f8a3439902d..b51f85417f58 100644
> --- a/arch/arm/include/asm/unwind.h
> +++ b/arch/arm/include/asm/unwind.h
> @@ -24,6 +24,7 @@ struct unwind_idx {
>  
>  struct unwind_table {
>  	struct list_head list;
> +	struct list_head mod_list;
>  	const struct unwind_idx *start;
>  	const struct unwind_idx *origin;
>  	const struct unwind_idx *stop;
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index 549abcedf795..b98bcb47e337 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -459,46 +459,41 @@ int module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs,
>  #ifdef CONFIG_ARM_UNWIND
>  	const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>  	const Elf_Shdr *sechdrs_end = sechdrs + hdr->e_shnum;
> -	struct mod_unwind_map maps[ARM_SEC_MAX];
> -	int i;
> +	struct unwind_table *unwind_list = &mod->arch.unwind_list;
>  
> -	memset(maps, 0, sizeof(maps));
> +	INIT_LIST_HEAD(&unwind_list->mod_list);
> +	mod->arch.init_table = NULL;
>  
>  	for (s = sechdrs; s < sechdrs_end; s++) {
>  		const char *secname = secstrs + s->sh_name;
> +		const char *txtname;
> +		const Elf_Shdr *txt_sec;
>  
> -		if (!(s->sh_flags & SHF_ALLOC))
> +		if (!strcmp(".ARM.exidx", secname))
> +			txtname = ".text";
> +		else
> +			txtname = secname + strlen(".ARM.exidx");
> +
> +		txt_sec = find_mod_section(hdr, sechdrs, txtname);
> +
> +		if (!(s->sh_flags & SHF_ALLOC) ||
> +		    s->sh_type != ELF_SECTION_UNWIND)
>  			continue;
>  
> -		if (strcmp(".ARM.exidx.init.text", secname) == 0)
> -			maps[ARM_SEC_INIT].unw_sec = s;
> -		else if (strcmp(".ARM.exidx", secname) == 0)
> -			maps[ARM_SEC_CORE].unw_sec = s;
> -		else if (strcmp(".ARM.exidx.exit.text", secname) == 0)
> -			maps[ARM_SEC_EXIT].unw_sec = s;
> -		else if (strcmp(".ARM.exidx.text.unlikely", secname) == 0)
> -			maps[ARM_SEC_UNLIKELY].unw_sec = s;
> -		else if (strcmp(".ARM.exidx.text.hot", secname) == 0)
> -			maps[ARM_SEC_HOT].unw_sec = s;
> -		else if (strcmp(".init.text", secname) == 0)
> -			maps[ARM_SEC_INIT].txt_sec = s;
> -		else if (strcmp(".text", secname) == 0)
> -			maps[ARM_SEC_CORE].txt_sec = s;
> -		else if (strcmp(".exit.text", secname) == 0)
> -			maps[ARM_SEC_EXIT].txt_sec = s;
> -		else if (strcmp(".text.unlikely", secname) == 0)
> -			maps[ARM_SEC_UNLIKELY].txt_sec = s;
> -		else if (strcmp(".text.hot", secname) == 0)
> -			maps[ARM_SEC_HOT].txt_sec = s;
> -	}
> +		if (txt_sec) {
> +			struct unwind_table *table =
> +				unwind_table_add(s->sh_addr,
> +						s->sh_size,
> +						txt_sec->sh_addr,
> +						txt_sec->sh_size);
> +
> +			list_add(&table->mod_list, &unwind_list->mod_list);
>  
> -	for (i = 0; i < ARM_SEC_MAX; i++)
> -		if (maps[i].unw_sec && maps[i].txt_sec)
> -			mod->arch.unwind[i] =
> -				unwind_table_add(maps[i].unw_sec->sh_addr,
> -					         maps[i].unw_sec->sh_size,
> -					         maps[i].txt_sec->sh_addr,
> -					         maps[i].txt_sec->sh_size);
> +			/* save init table for module_arch_freeing_init */
> +			if (strcmp(".ARM.exidx.init.text", secname) == 0)
> +				mod->arch.init_table = table;
> +		}
> +	}
>  #endif
>  #ifdef CONFIG_ARM_PATCH_PHYS_VIRT
>  	s = find_mod_section(hdr, sechdrs, ".pv_table");
> @@ -519,11 +514,13 @@ void
>  module_arch_cleanup(struct module *mod)
>  {
>  #ifdef CONFIG_ARM_UNWIND
> -	int i;
> +	struct unwind_table *tmp;
> +	struct unwind_table *n;
>  
> -	for (i = 0; i < ARM_SEC_MAX; i++) {
> -		unwind_table_del(mod->arch.unwind[i]);
> -		mod->arch.unwind[i] = NULL;
> +	list_for_each_entry_safe(tmp, n,
> +			&mod->arch.unwind_list.mod_list, mod_list) {
> +		list_del(&tmp->mod_list);
> +		unwind_table_del(tmp);
>  	}
>  #endif
>  }
> @@ -531,7 +528,12 @@ module_arch_cleanup(struct module *mod)
>  void __weak module_arch_freeing_init(struct module *mod)
>  {
>  #ifdef CONFIG_ARM_UNWIND
> -	unwind_table_del(mod->arch.unwind[ARM_SEC_INIT]);
> -	mod->arch.unwind[ARM_SEC_INIT] = NULL;
> +	struct unwind_table *init = mod->arch.init_table;
> +
> +	if (init) {
> +		mod->arch.init_table = NULL;
> +		list_del(&init->mod_list);
> +		unwind_table_del(init);
> +	}
>  #endif
>  }

Re: [PATCH v4] ARM: module: Add all unwind tables when load module

[Linus Walleij]

On Fri, Apr 1, 2022 at 3:16 PM Chen Zhongjin <chenzhongjin@huawei.com> wrote:

> For EABI stack unwinding, when loading .ko module
> the EXIDX sections will be added to a unwind_table list.
>
> However not all EXIDX sections are added because EXIDX
> sections are searched by hardcoded section names.
>
> For functions in other sections such as .ref.text
> or .kprobes.text, gcc generates seprated EXIDX sections
> (such as .ARM.exidx.ref.text or .ARM.exidx.kprobes.text).
>
> These extra EXIDX sections are not loaded, so when unwinding
> functions in these sections, we will failed with:
>
>         unwind: Index not found xxx
>
> To fix that, I refactor the code for searching and adding
> EXIDX sections:
>
> - Check section type to search EXIDX tables (0x70000001)
> instead of strcmp() the hardcoded names. Then find the
> corresponding text sections by their section names.
>
> - Add a unwind_table list in module->arch to save their own
> unwind_table instead of the fixed-lenth array.
>
> - Save .ARM.exidx.init.text section ptr, because it should
> be cleaned after module init.
>
> Now all EXIDX sections of .ko can be added correctly.
>
> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>

This looks reasonable to me:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

If no one else comments I would say you should put this
into Russell's patch tracker for consideration.

Yours,
Linus Walleij

Re: [PATCH v4] ARM: module: Add all unwind tables when load module

[Chen Zhongjin]

Added,
9204/1 	module: Add all unwind tables when load module

Thanks!
Chen

On 2022/4/29 6:45, Linus Walleij wrote:
> On Fri, Apr 1, 2022 at 3:16 PM Chen Zhongjin <chenzhongjin@huawei.com> wrote:
> 
>> For EABI stack unwinding, when loading .ko module
>> the EXIDX sections will be added to a unwind_table list.
>>
>> However not all EXIDX sections are added because EXIDX
>> sections are searched by hardcoded section names.
>>
>> For functions in other sections such as .ref.text
>> or .kprobes.text, gcc generates seprated EXIDX sections
>> (such as .ARM.exidx.ref.text or .ARM.exidx.kprobes.text).
>>
>> These extra EXIDX sections are not loaded, so when unwinding
>> functions in these sections, we will failed with:
>>
>>         unwind: Index not found xxx
>>
>> To fix that, I refactor the code for searching and adding
>> EXIDX sections:
>>
>> - Check section type to search EXIDX tables (0x70000001)
>> instead of strcmp() the hardcoded names. Then find the
>> corresponding text sections by their section names.
>>
>> - Add a unwind_table list in module->arch to save their own
>> unwind_table instead of the fixed-lenth array.
>>
>> - Save .ARM.exidx.init.text section ptr, because it should
>> be cleaned after module init.
>>
>> Now all EXIDX sections of .ko can be added correctly.
>>
>> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
> 
> This looks reasonable to me:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> If no one else comments I would say you should put this
> into Russell's patch tracker for consideration.
> 
> Yours,
> Linus Walleij
> .

Re: [PATCH v4] ARM: module: Add all unwind tables when load module

[Russell King (Oracle)]

Hi,

On Fri, Apr 01, 2022 at 09:15:34PM +0800, Chen Zhongjin wrote:
>  struct mod_arch_specific {
>  #ifdef CONFIG_ARM_UNWIND
> -	struct unwind_table *unwind[ARM_SEC_MAX];
> +	struct unwind_table unwind_list;

Why is this not a:

	struct list_head unwind_list;

because, from what I can tell, the _only_ member that is used in this
struct unwind_table is the "mod_list" member - so everything else is
entirely unused and redundant.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v4] ARM: module: Add all unwind tables when load module

[Chen Zhongjin]

On 2022/5/17 22:19, Russell King (Oracle) wrote:
> Hi,
> 
> On Fri, Apr 01, 2022 at 09:15:34PM +0800, Chen Zhongjin wrote:
>>  struct mod_arch_specific {
>>  #ifdef CONFIG_ARM_UNWIND
>> -	struct unwind_table *unwind[ARM_SEC_MAX];
>> +	struct unwind_table unwind_list;
> 
> Why is this not a:
> 
> 	struct list_head unwind_list;
> 
> because, from what I can tell, the _only_ member that is used in this
> struct unwind_table is the "mod_list" member - so everything else is
> entirely unused and redundant.
> 
> Thanks.
> 
Thanks for review!

I have updated it to v5 with two other fixes. Also updated it on patch tracker.

Best,
Chen