lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CADBMgpyk8hA3tm+RWLL-9bv-k2eEPWz0E9wSSoKVZ+snq3ESHg@mail.gmail.com>
Date: Tue, 29 Apr 2025 11:26:07 -0700
From: Dylan Hatch <dylanbhatch@...gle.com>
To: Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>
Cc: John Fastabend <john.fastabend@...il.com>, Yonghong Song <yonghong.song@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Andrii Nakryiko <andrii@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Philippe Mathieu-Daudé <philmd@...aro.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, KP Singh <kpsingh@...nel.org>, 
	Xu Kuohai <xukuohai@...weicloud.com>, Puranjay Mohan <puranjay@...nel.org>, 
	Jiri Olsa <jolsa@...nel.org>, Hao Luo <haoluo@...gle.com>, 
	"Mike Rapoport (Microsoft)" <rppt@...nel.org>, Ard Biesheuvel <ardb@...nel.org>, Luis Chamberlain <mcgrof@...nel.org>, 
	Geert Uytterhoeven <geert@...ux-m68k.org>, Arnd Bergmann <arnd@...db.de>, Song Liu <song@...nel.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, Mark Rutland <mark.rutland@....com>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Roman Gushchin <roman.gushchin@...ux.dev>
Subject: Re: [PATCH v2 2/2] arm64/module: Use text-poke API for late relocations.

On Fri, Apr 11, 2025 at 6:10 PM Dylan Hatch <dylanbhatch@...gle.com> wrote:
>
> To enable late module patching, livepatch modules need to be able to
> apply some of their relocations well after being loaded. In this
> scenario, use the text-poking API to allow this, even with
> STRICT_MODULE_RWX.
>
> This patch is largely based off commit 88fc078a7a8f6 ("x86/module: Use
> text_poke() for late relocations").
>
> Signed-off-by: Dylan Hatch <dylanbhatch@...gle.com>
> ---
>  arch/arm64/kernel/module.c | 129 ++++++++++++++++++++++++-------------
>  1 file changed, 83 insertions(+), 46 deletions(-)
>
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 06bb680bfe975..0703502d9dc37 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -18,11 +18,13 @@
>  #include <linux/moduleloader.h>
>  #include <linux/random.h>
>  #include <linux/scs.h>
> +#include <linux/memory.h>
>
>  #include <asm/alternative.h>
>  #include <asm/insn.h>
>  #include <asm/scs.h>
>  #include <asm/sections.h>
> +#include <asm/text-patching.h>
>
>  enum aarch64_reloc_op {
>         RELOC_OP_NONE,
> @@ -48,7 +50,8 @@ static u64 do_reloc(enum aarch64_reloc_op reloc_op, __le32 *place, u64 val)
>         return 0;
>  }
>
> -static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
> +static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len,
> +                     void *(*write)(void *dest, const void *src, size_t len))
>  {
>         s64 sval = do_reloc(op, place, val);
>
> @@ -66,7 +69,7 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
>
>         switch (len) {
>         case 16:
> -               *(s16 *)place = sval;
> +               write(place, &sval, sizeof(s16));
>                 switch (op) {
>                 case RELOC_OP_ABS:
>                         if (sval < 0 || sval > U16_MAX)
> @@ -82,7 +85,7 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
>                 }
>                 break;
>         case 32:
> -               *(s32 *)place = sval;
> +               write(place, &sval, sizeof(s32));
>                 switch (op) {
>                 case RELOC_OP_ABS:
>                         if (sval < 0 || sval > U32_MAX)
> @@ -98,7 +101,7 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
>                 }
>                 break;
>         case 64:
> -               *(s64 *)place = sval;
> +               write(place, &sval, sizeof(s64));
>                 break;
>         default:
>                 pr_err("Invalid length (%d) for data relocation\n", len);
> @@ -113,11 +116,13 @@ enum aarch64_insn_movw_imm_type {
>  };
>
>  static int reloc_insn_movw(enum aarch64_reloc_op op, __le32 *place, u64 val,
> -                          int lsb, enum aarch64_insn_movw_imm_type imm_type)
> +                          int lsb, enum aarch64_insn_movw_imm_type imm_type,
> +                          void *(*write)(void *dest, const void *src, size_t len))
>  {
>         u64 imm;
>         s64 sval;
>         u32 insn = le32_to_cpu(*place);
> +       __le32 le_insn;
>
>         sval = do_reloc(op, place, val);
>         imm = sval >> lsb;
> @@ -145,7 +150,8 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, __le32 *place, u64 val,
>
>         /* Update the instruction with the new encoding. */
>         insn = aarch64_insn_encode_immediate(AARCH64_INSN_IMM_16, insn, imm);
> -       *place = cpu_to_le32(insn);
> +       le_insn = cpu_to_le32(insn);
> +       write(place, &le_insn, sizeof(le_insn));
>
>         if (imm > U16_MAX)
>                 return -ERANGE;
> @@ -154,11 +160,13 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, __le32 *place, u64 val,
>  }
>
>  static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
> -                         int lsb, int len, enum aarch64_insn_imm_type imm_type)
> +                         int lsb, int len, enum aarch64_insn_imm_type imm_type,
> +                         void *(*write)(void *dest, const void *src, size_t len))
>  {
>         u64 imm, imm_mask;
>         s64 sval;
>         u32 insn = le32_to_cpu(*place);
> +       __le32 le_insn;
>
>         /* Calculate the relocation value. */
>         sval = do_reloc(op, place, val);
> @@ -170,7 +178,8 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
>
>         /* Update the instruction's immediate field. */
>         insn = aarch64_insn_encode_immediate(imm_type, insn, imm);
> -       *place = cpu_to_le32(insn);
> +       le_insn = cpu_to_le32(insn);
> +       write(place, &le_insn, sizeof(le_insn));
>
>         /*
>          * Extract the upper value bits (including the sign bit) and
> @@ -189,17 +198,19 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, __le32 *place, u64 val,
>  }
>
>  static int reloc_insn_adrp(struct module *mod, Elf64_Shdr *sechdrs,
> -                          __le32 *place, u64 val)
> +                          __le32 *place, u64 val,
> +                          void *(*write)(void *dest, const void *src, size_t len))
>  {
>         u32 insn;
> +       __le32 le_insn;
>
>         if (!is_forbidden_offset_for_adrp(place))
>                 return reloc_insn_imm(RELOC_OP_PAGE, place, val, 12, 21,
> -                                     AARCH64_INSN_IMM_ADR);
> +                                     AARCH64_INSN_IMM_ADR, write);
>
>         /* patch ADRP to ADR if it is in range */
>         if (!reloc_insn_imm(RELOC_OP_PREL, place, val & ~0xfff, 0, 21,
> -                           AARCH64_INSN_IMM_ADR)) {
> +                           AARCH64_INSN_IMM_ADR, write)) {
>                 insn = le32_to_cpu(*place);
>                 insn &= ~BIT(31);
>         } else {
> @@ -211,15 +222,17 @@ static int reloc_insn_adrp(struct module *mod, Elf64_Shdr *sechdrs,
>                                                    AARCH64_INSN_BRANCH_NOLINK);
>         }
>
> -       *place = cpu_to_le32(insn);
> +       le_insn = cpu_to_le32(insn);
> +       write(place, &le_insn, sizeof(le_insn));
>         return 0;
>  }
>
> -int apply_relocate_add(Elf64_Shdr *sechdrs,
> +static int __apply_relocate_add(Elf64_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))
>  {
>         unsigned int i;
>         int ovf;
> @@ -255,23 +268,23 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>                 /* Data relocations. */
>                 case R_AARCH64_ABS64:
>                         overflow_check = false;
> -                       ovf = reloc_data(RELOC_OP_ABS, loc, val, 64);
> +                       ovf = reloc_data(RELOC_OP_ABS, loc, val, 64, write);
>                         break;
>                 case R_AARCH64_ABS32:
> -                       ovf = reloc_data(RELOC_OP_ABS, loc, val, 32);
> +                       ovf = reloc_data(RELOC_OP_ABS, loc, val, 32, write);
>                         break;
>                 case R_AARCH64_ABS16:
> -                       ovf = reloc_data(RELOC_OP_ABS, loc, val, 16);
> +                       ovf = reloc_data(RELOC_OP_ABS, loc, val, 16, write);
>                         break;
>                 case R_AARCH64_PREL64:
>                         overflow_check = false;
> -                       ovf = reloc_data(RELOC_OP_PREL, loc, val, 64);
> +                       ovf = reloc_data(RELOC_OP_PREL, loc, val, 64, write);
>                         break;
>                 case R_AARCH64_PREL32:
> -                       ovf = reloc_data(RELOC_OP_PREL, loc, val, 32);
> +                       ovf = reloc_data(RELOC_OP_PREL, loc, val, 32, write);
>                         break;
>                 case R_AARCH64_PREL16:
> -                       ovf = reloc_data(RELOC_OP_PREL, loc, val, 16);
> +                       ovf = reloc_data(RELOC_OP_PREL, loc, val, 16, write);
>                         break;
>
>                 /* MOVW instruction relocations. */
> @@ -280,88 +293,88 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>                         fallthrough;
>                 case R_AARCH64_MOVW_UABS_G0:
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_UABS_G1_NC:
>                         overflow_check = false;
>                         fallthrough;
>                 case R_AARCH64_MOVW_UABS_G1:
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_UABS_G2_NC:
>                         overflow_check = false;
>                         fallthrough;
>                 case R_AARCH64_MOVW_UABS_G2:
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_UABS_G3:
>                         /* We're using the top bits so we can't overflow. */
>                         overflow_check = false;
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 48,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_SABS_G0:
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>                 case R_AARCH64_MOVW_SABS_G1:
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>                 case R_AARCH64_MOVW_SABS_G2:
>                         ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G0_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 0,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G0:
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 0,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G1_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 16,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G1:
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 16,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G2_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 32,
> -                                             AARCH64_INSN_IMM_MOVKZ);
> +                                             AARCH64_INSN_IMM_MOVKZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G2:
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 32,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>                 case R_AARCH64_MOVW_PREL_G3:
>                         /* We're using the top bits so we can't overflow. */
>                         overflow_check = false;
>                         ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 48,
> -                                             AARCH64_INSN_IMM_MOVNZ);
> +                                             AARCH64_INSN_IMM_MOVNZ, write);
>                         break;
>
>                 /* Immediate instruction relocations. */
>                 case R_AARCH64_LD_PREL_LO19:
>                         ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19,
> -                                            AARCH64_INSN_IMM_19);
> +                                            AARCH64_INSN_IMM_19, write);
>                         break;
>                 case R_AARCH64_ADR_PREL_LO21:
>                         ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 0, 21,
> -                                            AARCH64_INSN_IMM_ADR);
> +                                            AARCH64_INSN_IMM_ADR, write);
>                         break;
>                 case R_AARCH64_ADR_PREL_PG_HI21_NC:
>                         overflow_check = false;
>                         fallthrough;
>                 case R_AARCH64_ADR_PREL_PG_HI21:
> -                       ovf = reloc_insn_adrp(me, sechdrs, loc, val);
> +                       ovf = reloc_insn_adrp(me, sechdrs, loc, val, write);
>                         if (ovf && ovf != -ERANGE)
>                                 return ovf;
>                         break;
> @@ -369,46 +382,46 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>                 case R_AARCH64_LDST8_ABS_LO12_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 0, 12,
> -                                            AARCH64_INSN_IMM_12);
> +                                            AARCH64_INSN_IMM_12, write);
>                         break;
>                 case R_AARCH64_LDST16_ABS_LO12_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 1, 11,
> -                                            AARCH64_INSN_IMM_12);
> +                                            AARCH64_INSN_IMM_12, write);
>                         break;
>                 case R_AARCH64_LDST32_ABS_LO12_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 2, 10,
> -                                            AARCH64_INSN_IMM_12);
> +                                            AARCH64_INSN_IMM_12, write);
>                         break;
>                 case R_AARCH64_LDST64_ABS_LO12_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 3, 9,
> -                                            AARCH64_INSN_IMM_12);
> +                                            AARCH64_INSN_IMM_12, write);
>                         break;
>                 case R_AARCH64_LDST128_ABS_LO12_NC:
>                         overflow_check = false;
>                         ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 4, 8,
> -                                            AARCH64_INSN_IMM_12);
> +                                            AARCH64_INSN_IMM_12, write);
>                         break;
>                 case R_AARCH64_TSTBR14:
>                         ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 14,
> -                                            AARCH64_INSN_IMM_14);
> +                                            AARCH64_INSN_IMM_14, write);
>                         break;
>                 case R_AARCH64_CONDBR19:
>                         ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19,
> -                                            AARCH64_INSN_IMM_19);
> +                                            AARCH64_INSN_IMM_19, write);
>                         break;
>                 case R_AARCH64_JUMP26:
>                 case R_AARCH64_CALL26:
>                         ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 26,
> -                                            AARCH64_INSN_IMM_26);
> +                                            AARCH64_INSN_IMM_26, write);
>                         if (ovf == -ERANGE) {
>                                 val = module_emit_plt_entry(me, sechdrs, loc, &rel[i], sym);
>                                 if (!val)
>                                         return -ENOEXEC;
>                                 ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2,
> -                                                    26, AARCH64_INSN_IMM_26);
> +                                                    26, AARCH64_INSN_IMM_26, write);
>                         }
>                         break;
>
> @@ -431,6 +444,30 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>         return -ENOEXEC;
>  }
>
> +int apply_relocate_add(Elf64_Shdr *sechdrs,
> +                      const char *strtab,
> +                      unsigned int symindex,
> +                      unsigned int relsec,
> +                      struct module *me)
> +{
> +       int ret;
> +       bool early = me->state == MODULE_STATE_UNFORMED;
> +       void *(*write)(void *, const void *, size_t) = memcpy;
> +
> +       if (!early) {
> +               write = text_poke;
> +               mutex_lock(&text_mutex);
> +       }
> +
> +       ret = __apply_relocate_add(sechdrs, strtab, symindex, relsec, me,
> +                                  write);
> +
> +       if (!early)
> +               mutex_unlock(&text_mutex);
> +
> +       return ret;
> +}
> +
>  static inline void __init_plt(struct plt_entry *plt, unsigned long addr)
>  {
>         *plt = get_plt_entry(addr, plt);
> --
> 2.49.0.604.gff1f9ca942-goog
>

Catalin and Will,

Are you comfortable with taking these patches? Is this a workable
approach to enable livepatching arm64 modules?

Thanks,
Dylan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ