Re: [PATCH v3 06/10] s390/module: Use s390_kernel_write() for late relocations

From: Josh Poimboeuf
Date: Sat Apr 25 2020 - 07:15:19 EST

Next message: Colin King: "[PATCH] media: stv0900_core: remove redundant assignment to variable val"
Previous message: Sven Eckelmann: "Re: [PATCH] ath10k: increase rx buffer size to 2048"
In reply to: Josh Poimboeuf: "[PATCH v3 06/10] s390/module: Use s390_kernel_write() for late relocations"
Next in thread: Josh Poimboeuf: "[PATCH v3 08/10] livepatch: Remove module_disable_ro() usage"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sat, Apr 25, 2020 at 06:07:26AM -0500, Josh Poimboeuf wrote:
> From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
>
> Because of late module patching, a livepatch module needs to be able to
> apply some of its relocations well after it has been loaded. 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. Test and add fixes. ]
>
> Cc: linux-s390@xxxxxxxxxxxxxxx
> Cc: Heiko Carstens heiko.carstens@xxxxxxxxxx

Gah, I somehow messed up the formatting on this Cc, should be

Cc: Heiko Carstens <heiko.carstens@xxxxxxxxxx>

> Cc: Gerald Schaefer <gerald.schaefer@xxxxxxxxxx>
> Cc: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> Suggested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Acked-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
> arch/s390/kernel/module.c | 147 +++++++++++++++++++++++---------------
> 1 file changed, 88 insertions(+), 59 deletions(-)
>
> diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
> index ba8f19bb438b..4055f1c49814 100644
> --- a/arch/s390/kernel/module.c
> +++ b/arch/s390/kernel/module.c
> @@ -19,6 +19,7 @@
> #include <linux/kasan.h>
> #include <linux/moduleloader.h>
> #include <linux/bug.h>
> +#include <linux/memory.h>
> #include <asm/alternative.h>
> #include <asm/nospec-branch.h>
> #include <asm/facility.h>
> @@ -174,10 +175,12 @@ 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;
> + void *dest = (void *)loc;
>
> if (val & ((1UL << shift) - 1))
> return -ENOEXEC;
> @@ -194,26 +197,33 @@ 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) {
> + unsigned char tmp = val;
> + write(dest, &tmp, 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(dest, &tmp, 2);
> + } else if (bits == 16) {
> + unsigned short tmp = val;
> + write(dest, &tmp, 2);
> + } else if (bits == 20) {
> + unsigned int tmp = (val & 0xfff) << 16 |
> + (val & 0xff000) >> 4 | (*(unsigned int *) loc & 0xf00000ff);
> + write(dest, &tmp, 4);
> + } else if (bits == 32) {
> + unsigned int tmp = val;
> + write(dest, &tmp, 4);
> + } else if (bits == 64) {
> + unsigned long tmp = val;
> + write(dest, &tmp, 8);
> + }
> return 0;
> }
>
> static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> - const char *strtab, struct module *me)
> + const char *strtab, struct module *me,
> + void *(*write)(void *dest, const void *src, size_t len))
> {
> struct mod_arch_syminfo *info;
> Elf_Addr loc, val;
> @@ -241,17 +251,17 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> case R_390_64: /* Direct 64 bit. */
> val += rela->r_addend;
> if (r_type == R_390_8)
> - rc = apply_rela_bits(loc, val, 0, 8, 0);
> + rc = apply_rela_bits(loc, val, 0, 8, 0, write);
> else if (r_type == R_390_12)
> - rc = apply_rela_bits(loc, val, 0, 12, 0);
> + rc = apply_rela_bits(loc, val, 0, 12, 0, write);
> else if (r_type == R_390_16)
> - rc = apply_rela_bits(loc, val, 0, 16, 0);
> + rc = apply_rela_bits(loc, val, 0, 16, 0, write);
> else if (r_type == R_390_20)
> - rc = apply_rela_bits(loc, val, 1, 20, 0);
> + rc = apply_rela_bits(loc, val, 1, 20, 0, write);
> else if (r_type == R_390_32)
> - rc = apply_rela_bits(loc, val, 0, 32, 0);
> + rc = apply_rela_bits(loc, val, 0, 32, 0, write);
> else if (r_type == R_390_64)
> - rc = apply_rela_bits(loc, val, 0, 64, 0);
> + rc = apply_rela_bits(loc, val, 0, 64, 0, write);
> break;
> case R_390_PC16: /* PC relative 16 bit. */
> case R_390_PC16DBL: /* PC relative 16 bit shifted by 1. */
> @@ -260,15 +270,15 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> case R_390_PC64: /* PC relative 64 bit. */
> val += rela->r_addend - loc;
> if (r_type == R_390_PC16)
> - rc = apply_rela_bits(loc, val, 1, 16, 0);
> + rc = apply_rela_bits(loc, val, 1, 16, 0, write);
> else if (r_type == R_390_PC16DBL)
> - rc = apply_rela_bits(loc, val, 1, 16, 1);
> + rc = apply_rela_bits(loc, val, 1, 16, 1, write);
> else if (r_type == R_390_PC32DBL)
> - rc = apply_rela_bits(loc, val, 1, 32, 1);
> + rc = apply_rela_bits(loc, val, 1, 32, 1, write);
> else if (r_type == R_390_PC32)
> - rc = apply_rela_bits(loc, val, 1, 32, 0);
> + rc = apply_rela_bits(loc, val, 1, 32, 0, write);
> else if (r_type == R_390_PC64)
> - rc = apply_rela_bits(loc, val, 1, 64, 0);
> + rc = apply_rela_bits(loc, val, 1, 64, 0, write);
> break;
> case R_390_GOT12: /* 12 bit GOT offset. */
> case R_390_GOT16: /* 16 bit GOT offset. */
> @@ -283,33 +293,33 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> case R_390_GOTPLT64: /* 64 bit offset to jump slot. */
> case R_390_GOTPLTENT: /* 32 bit rel. offset to jump slot >> 1. */
> if (info->got_initialized == 0) {
> - Elf_Addr *gotent;
> + Elf_Addr *gotent = me->core_layout.base +
> + me->arch.got_offset +
> + info->got_offset;
>
> - gotent = me->core_layout.base + me->arch.got_offset +
> - info->got_offset;
> - *gotent = val;
> + write(gotent, &val, sizeof(*gotent));
> info->got_initialized = 1;
> }
> val = info->got_offset + rela->r_addend;
> if (r_type == R_390_GOT12 ||
> r_type == R_390_GOTPLT12)
> - rc = apply_rela_bits(loc, val, 0, 12, 0);
> + rc = apply_rela_bits(loc, val, 0, 12, 0, write);
> else if (r_type == R_390_GOT16 ||
> r_type == R_390_GOTPLT16)
> - rc = apply_rela_bits(loc, val, 0, 16, 0);
> + rc = apply_rela_bits(loc, val, 0, 16, 0, write);
> else if (r_type == R_390_GOT20 ||
> r_type == R_390_GOTPLT20)
> - rc = apply_rela_bits(loc, val, 1, 20, 0);
> + rc = apply_rela_bits(loc, val, 1, 20, 0, write);
> else if (r_type == R_390_GOT32 ||
> r_type == R_390_GOTPLT32)
> - rc = apply_rela_bits(loc, val, 0, 32, 0);
> + rc = apply_rela_bits(loc, val, 0, 32, 0, write);
> else if (r_type == R_390_GOT64 ||
> r_type == R_390_GOTPLT64)
> - rc = apply_rela_bits(loc, val, 0, 64, 0);
> + rc = apply_rela_bits(loc, val, 0, 64, 0, write);
> else if (r_type == R_390_GOTENT ||
> r_type == R_390_GOTPLTENT) {
> val += (Elf_Addr) me->core_layout.base - loc;
> - rc = apply_rela_bits(loc, val, 1, 32, 1);
> + rc = apply_rela_bits(loc, val, 1, 32, 1, write);
> }
> break;
> case R_390_PLT16DBL: /* 16 bit PC rel. PLT shifted by 1. */
> @@ -320,25 +330,29 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> case R_390_PLTOFF32: /* 32 bit offset from GOT to PLT. */
> case R_390_PLTOFF64: /* 16 bit offset from GOT to PLT. */
> if (info->plt_initialized == 0) {
> - unsigned int *ip;
> - ip = me->core_layout.base + me->arch.plt_offset +
> - info->plt_offset;
> - ip[0] = 0x0d10e310; /* basr 1,0 */
> - ip[1] = 0x100a0004; /* lg 1,10(1) */
> + unsigned int insn[5];
> + unsigned int *ip = me->core_layout.base +
> + me->arch.plt_offset +
> + info->plt_offset;
> +
> + insn[0] = 0x0d10e310; /* basr 1,0 */
> + insn[1] = 0x100a0004; /* lg 1,10(1) */
> if (IS_ENABLED(CONFIG_EXPOLINE) && !nospec_disable) {
> unsigned int *ij;
> ij = me->core_layout.base +
> me->arch.plt_offset +
> me->arch.plt_size - PLT_ENTRY_SIZE;
> - ip[2] = 0xa7f40000 + /* j __jump_r1 */
> + insn[2] = 0xa7f40000 + /* j __jump_r1 */
> (unsigned int)(u16)
> (((unsigned long) ij - 8 -
> (unsigned long) ip) / 2);
> } else {
> - ip[2] = 0x07f10000; /* br %r1 */
> + insn[2] = 0x07f10000; /* br %r1 */
> }
> - ip[3] = (unsigned int) (val >> 32);
> - ip[4] = (unsigned int) val;
> + insn[3] = (unsigned int) (val >> 32);
> + insn[4] = (unsigned int) val;
> +
> + write(ip, insn, sizeof(insn));
> info->plt_initialized = 1;
> }
> if (r_type == R_390_PLTOFF16 ||
> @@ -357,17 +371,17 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> val += rela->r_addend - loc;
> }
> if (r_type == R_390_PLT16DBL)
> - rc = apply_rela_bits(loc, val, 1, 16, 1);
> + rc = apply_rela_bits(loc, val, 1, 16, 1, write);
> else if (r_type == R_390_PLTOFF16)
> - rc = apply_rela_bits(loc, val, 0, 16, 0);
> + rc = apply_rela_bits(loc, val, 0, 16, 0, write);
> else if (r_type == R_390_PLT32DBL)
> - rc = apply_rela_bits(loc, val, 1, 32, 1);
> + rc = apply_rela_bits(loc, val, 1, 32, 1, write);
> else if (r_type == R_390_PLT32 ||
> r_type == R_390_PLTOFF32)
> - rc = apply_rela_bits(loc, val, 0, 32, 0);
> + rc = apply_rela_bits(loc, val, 0, 32, 0, write);
> else if (r_type == R_390_PLT64 ||
> r_type == R_390_PLTOFF64)
> - rc = apply_rela_bits(loc, val, 0, 64, 0);
> + rc = apply_rela_bits(loc, val, 0, 64, 0, write);
> break;
> case R_390_GOTOFF16: /* 16 bit offset to GOT. */
> case R_390_GOTOFF32: /* 32 bit offset to GOT. */
> @@ -375,20 +389,20 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> val = val + rela->r_addend -
> ((Elf_Addr) me->core_layout.base + me->arch.got_offset);
> if (r_type == R_390_GOTOFF16)
> - rc = apply_rela_bits(loc, val, 0, 16, 0);
> + rc = apply_rela_bits(loc, val, 0, 16, 0, write);
> else if (r_type == R_390_GOTOFF32)
> - rc = apply_rela_bits(loc, val, 0, 32, 0);
> + rc = apply_rela_bits(loc, val, 0, 32, 0, write);
> else if (r_type == R_390_GOTOFF64)
> - rc = apply_rela_bits(loc, val, 0, 64, 0);
> + rc = apply_rela_bits(loc, val, 0, 64, 0, write);
> break;
> case R_390_GOTPC: /* 32 bit PC relative offset to GOT. */
> case R_390_GOTPCDBL: /* 32 bit PC rel. off. to GOT shifted by 1. */
> val = (Elf_Addr) me->core_layout.base + me->arch.got_offset +
> rela->r_addend - loc;
> if (r_type == R_390_GOTPC)
> - rc = apply_rela_bits(loc, val, 1, 32, 0);
> + rc = apply_rela_bits(loc, val, 1, 32, 0, write);
> else if (r_type == R_390_GOTPCDBL)
> - rc = apply_rela_bits(loc, val, 1, 32, 1);
> + rc = apply_rela_bits(loc, val, 1, 32, 1, write);
> break;
> case R_390_COPY:
> case R_390_GLOB_DAT: /* Create GOT entry. */
> @@ -412,9 +426,10 @@ static int apply_rela(Elf_Rela *rela, Elf_Addr base, Elf_Sym *symtab,
> return 0;
> }
>
> -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;
> @@ -430,13 +445,27 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> n = sechdrs[relsec].sh_size / sizeof(Elf_Rela);
>
> for (i = 0; i < n; i++, rela++) {
> - rc = apply_rela(rela, base, symtab, strtab, me);
> + rc = apply_rela(rela, base, symtab, strtab, me, write);
> if (rc)
> return rc;
> }
> return 0;
> }
>
> +int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> + unsigned int symindex, unsigned int relsec,
> + struct module *me)
> +{
> + bool early = me->state == MODULE_STATE_UNFORMED;
> + void *(*write)(void *, const void *, size_t) = memcpy;
> +
> + if (!early)
> + write = s390_kernel_write;
> +
> + return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me,
> + write);
> +}
> +
> int module_finalize(const Elf_Ehdr *hdr,
> const Elf_Shdr *sechdrs,
> struct module *me)
> --
> 2.21.1
>

--
Josh

Next message: Colin King: "[PATCH] media: stv0900_core: remove redundant assignment to variable val"
Previous message: Sven Eckelmann: "Re: [PATCH] ath10k: increase rx buffer size to 2048"
In reply to: Josh Poimboeuf: "[PATCH v3 06/10] s390/module: Use s390_kernel_write() for late relocations"
Next in thread: Josh Poimboeuf: "[PATCH v3 08/10] livepatch: Remove module_disable_ro() usage"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]