On Tue, 14 Apr 2020, Josh Poimboeuf wrote:
> From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
>
> Instead of playing games with module_{dis,en}able_ro(), use existing
> text poking mechanisms to apply relocations after module loading.
>
> So far only x86, s390 and Power have HAVE_LIVEPATCH but only the first
> two also have STRICT_MODULE_RWX.
>
> This will allow removal of the last module_disable_ro() usage in
> livepatch. The ultimate goal is to completely disallow making
> executable mappings writable.
>
> [ jpoimboe: Split up patches. Use mod state to determine whether
> memcpy() can be used. ]
>
> Cc: linux-s390@xxxxxxxxxxxxxxx
> Cc: heiko.carstens@xxxxxxxxxx
> Suggested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
> arch/s390/kernel/module.c | 106 ++++++++++++++++++++++----------------
> 1 file changed, 61 insertions(+), 45 deletions(-)
>
> diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
> index ba8f19bb438b..e85e378f876e 100644
> --- a/arch/s390/kernel/module.c
> +++ b/arch/s390/kernel/module.c
> @@ -174,7 +174,8 @@ int module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> }
>
> static int apply_rela_bits(Elf_Addr loc, Elf_Addr val,
> - int sign, int bits, int shift)
> + int sign, int bits, int shift,
> + void (*write)(void *dest, const void *src, size_t len))
> {
> unsigned long umax;
> long min, max;
> @@ -194,26 +195,29 @@ static int apply_rela_bits(Elf_Addr loc, Elf_Addr val,
> return -ENOEXEC;
> }
>
> - if (bits == 8)
> - *(unsigned char *) loc = val;
> - else if (bits == 12)
> - *(unsigned short *) loc = (val & 0xfff) |
> + if (bits == 8) {
> + write(loc, &val, 1);
> + } else if (bits == 12) {
> + unsigned short tmp = (val & 0xfff) |
> (*(unsigned short *) loc & 0xf000);
> - else if (bits == 16)
> - *(unsigned short *) loc = val;
> - else if (bits == 20)
> - *(unsigned int *) loc = (val & 0xfff) << 16 |
> - (val & 0xff000) >> 4 |
> - (*(unsigned int *) loc & 0xf00000ff);
> - else if (bits == 32)
> - *(unsigned int *) loc = val;
> - else if (bits == 64)
> - *(unsigned long *) loc = val;
> + write(loc, &tmp, 2);
> + } else if (bits == 16) {
> + write(loc, &val, 2);
> + } else if (bits == 20) {
> + unsigned int tmp = (val & 0xfff) << 16 |
> + (val & 0xff000) >> 4 | (*(unsigned int *) loc & 0xf00000ff);
> + write(loc, &tmp, 4);
> + } else if (bits == 32) {
> + write(loc, &val, 4);
> + } else if (bits == 64) {
> + write(loc, &val, 8);
> + }
> return 0;
> }
The compiler complains about the above changes
arch/s390/kernel/module.c:199:9: warning: passing argument 1 of 'write' makes pointer from integer without a cast [-Wint-conversion]
write(loc, &val, 1);
^~~
arch/s390/kernel/module.c:199:9: note: expected 'void *' but argument is of type 'Elf64_Addr' {aka 'long long unsigned int'}
[...]
> -int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> +static int __apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> unsigned int symindex, unsigned int relsec,
> - struct module *me)
> + struct module *me,
> + void (*write)(void *dest, const void *src, size_t len))
> {
> Elf_Addr base;
> Elf_Sym *symtab;
You also need to update apply_rela() call site in this function. It is
missing write argument.
> @@ -437,6 +442,17 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> return 0;
> }
>
> +int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> + unsigned int symindex, unsigned int relsec,
> + struct module *me)
> +{
> + int ret;
ret is unused;
> + bool early = me->state == MODULE_STATE_UNFORMED;
> +
> + return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me,
> + early ? memcpy : s390_kernel_write);
The compiler warns about
arch/s390/kernel/module.c: In function 'apply_relocate_add':
arch/s390/kernel/module.c:453:24: warning: pointer type mismatch in conditional expression
early ? memcpy : s390_kernel_write);
Miroslav
