Re: [PATCH v5] arm64/module: Use text-poke API for late relocations.

[Will Deacon <will@kernel.org>]

On Fri, May 30, 2025 at 12:00:44AM +0000, Dylan Hatch 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 however, the livepatch module text and data is already RX-only,
> so special treatment is needed to make the late relocations possible. To
> do this, use the text-poking API for these late relocations.
> 
> This patch is partially based off commit 88fc078a7a8f6 ("x86/module: Use
> text_poke() for late relocations").
> 
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
> Acked-by: Song Liu <song@kernel.org>
> ---
>  arch/arm64/kernel/module.c | 110 ++++++++++++++++++++++---------------
>  1 file changed, 66 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 06bb680bfe975..93e6d63074afe 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -23,6 +23,7 @@
>  #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 +49,26 @@ 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 void write_data(void *place, s64 *sval, size_t len, struct module *me)
> +{
> +	if (me->state == MODULE_STATE_UNFORMED)
> +		memcpy(place, sval, len);
> +	else
> +		aarch64_insn_copy(place, sval, len);
> +}
> +
> +static void write_insn(__le32 *place, u32 insn, struct module *me)
> +{
> +	__le32 le = cpu_to_le32(insn);
> +
> +	if (me->state == MODULE_STATE_UNFORMED)
> +		*place = le;
> +	else
> +		aarch64_insn_copy(place, &le, sizeof(le));
> +}

Maybe combine these into a single macro such as below?

#define WRITE_PLACE(place, val, mod) do {				\
	if (mod->state == MODULE_STATE_UNFORMED)			\
		*(place) = val;						\
	else								\
		aarch64_insn_copy(place, &(val), sizeof(*place));	\
while (0)

> +static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len,
> +		      struct module *me)
>  {
>  	s64 sval = do_reloc(op, place, val);
>  
> @@ -66,7 +86,7 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
>  
>  	switch (len) {
>  	case 16:
> -		*(s16 *)place = sval;
> +		write_data(place, &sval, sizeof(s16), me);

Then this would be:

	WRITE_PLACE((s16 *)place, sval, me);

> @@ -145,7 +166,7 @@ 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);
> +	write_insn(place, insn, me);

and this would be:

	WRITE_PLACE(place, cpu_to_le32(insn), me);

Will