Re: [PATCH v4 1/2] module: add elf_check_module_arch for module specific elf arch checks

[Jessica Yu]

+++ Nicholas Piggin [11/06/21 19:39 +1000]:
> The elf_check_arch() function is used to test usermode binaries, but
> kernel modules may have more specific requirements. powerpc would like
> to test for ABI version compatibility.
>
> Add an arch-overridable function elf_check_module_arch() that defaults
> to elf_check_arch() and use it in elf_validity_check().
>
> Signed-off-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> [np: split patch, added changelog]
> Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
> ---
> include/linux/moduleloader.h | 5 +++++
> kernel/module.c              | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
> index 9e09d11ffe5b..fdc042a84562 100644
> --- a/include/linux/moduleloader.h
> +++ b/include/linux/moduleloader.h
> @@ -13,6 +13,11 @@
>  * must be implemented by each architecture.
>  */
>
> +// Allow arch to optionally do additional checking of module ELF header
> +#ifndef elf_check_module_arch
> +#define elf_check_module_arch elf_check_arch
> +#endif

Hi Nicholas,

Why not make elf_check_module_arch() consistent with the other
arch-specific functions? Please see module_frob_arch_sections(),
module_{init,exit}_section(), etc in moduleloader.h. That is, they are
all __weak functions that are overridable by arches. We can maybe make
elf_check_module_arch() a weak symbol, available for arches to
override if they want to perform additional elf checks. Then we don't
have to have this one-off #define.

Thanks,

Jessica

> +
> /* Adjust arch-specific sections.  Return 0 on success.  */
> int module_frob_arch_sections(Elf_Ehdr *hdr,
> 			      Elf_Shdr *sechdrs,
> diff --git a/kernel/module.c b/kernel/module.c
> index 7e78dfabca97..7c3f9b7478dc 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2946,7 +2946,7 @@ static int elf_validity_check(struct load_info *info)
>
> 	if (memcmp(info->hdr->e_ident, ELFMAG, SELFMAG) != 0
> 	    || info->hdr->e_type != ET_REL
> -	    || !elf_check_arch(info->hdr)
> +	    || !elf_check_module_arch(info->hdr)
> 	    || info->hdr->e_shentsize != sizeof(Elf_Shdr))
> 		return -ENOEXEC;
>
> --
> 2.23.0