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

+++ Nicholas Piggin [16/06/21 11:18 +1000]:

Excerpts from Jessica Yu's message of June 15, 2021 10:17 pm:

quoted

+++ Nicholas Piggin [15/06/21 12:05 +1000]:

quoted

Excerpts from Jessica Yu's message of June 14, 2021 10:06 pm:

quoted

+++ Nicholas Piggin [11/06/21 19:39 +1000]:

quoted

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@ellerman.id.au>
[np: split patch, added changelog]
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
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.

Like this? I like it. Good idea.

Yeah! Also, maybe we can alternatively make elf_check_module_arch() a
separate check entirely so that the powerpc implementation doesn't
have to include that extra elf_check_arch() call. Something like this maybe?

Yeah we can do that. Would you be okay if it goes via powerpc tree? If
yes, then we should get your Ack (or SOB because it seems to be entirely
your patch now :D)

This can go through the powerpc tree. Will you do another respin
of this patch? And yes, feel free to take my SOB for this one -

    Signed-off-by: Jessica Yu [off-list ref]

Thanks!

Jessica