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:
 <OS7PR01MB13775AD2A7D99033AE27213A8D76DA@OS7PR01MB13775.jpnprd01.prod.outlook.com>
Date: Tue, 3 Jun 2025 00:14:03 +0000
From: "Toshiyuki Sato (Fujitsu)" <fj6611ie@...itsu.com>
To: 'Dylan Hatch' <dylanbhatch@...gle.com>
CC: Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
	Ard Biesheuvel <ardb@...nel.org>, Sami Tolvanen <samitolvanen@...gle.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>, Song Liu <song@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Roman Gushchin <roman.gushchin@...ux.dev>,
	"Toshiyuki Sato (Fujitsu)" <fj6611ie@...itsu.com>
Subject: RE: [PATCH v6] arm64/module: Use text-poke API for late relocations.

Hi Dylan,

> 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@...gle.com>
> Acked-by: Song Liu <song@...nel.org>
> ---
>  arch/arm64/kernel/module.c | 105
> +++++++++++++++++++++----------------
>  1 file changed, 61 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index
> 06bb680bfe975..fdfb71c8fc929 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,15 @@ 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)
> +#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 +75,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((s16 *)place, sval, me);
>  		switch (op) {
>  		case RELOC_OP_ABS:
>  			if (sval < 0 || sval > U16_MAX)
> @@ -82,7 +91,7 @@ static int reloc_data(enum aarch64_reloc_op op, void *place,
> u64 val, int len)
>  		}
>  		break;
>  	case 32:
> -		*(s32 *)place = sval;
> +		WRITE_PLACE((s32 *)place, sval, me);
>  		switch (op) {
>  		case RELOC_OP_ABS:
>  			if (sval < 0 || sval > U32_MAX)
> @@ -98,7 +107,7 @@ static int reloc_data(enum aarch64_reloc_op op, void
> *place, u64 val, int len)
>  		}
>  		break;
>  	case 64:
> -		*(s64 *)place = sval;
> +		WRITE_PLACE((s64 *)place, sval, me);
>  		break;
>  	default:
>  		pr_err("Invalid length (%d) for data relocation\n", len); @@
> -113,11 +122,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,
> +			   struct module *me)
>  {
>  	u64 imm;
>  	s64 sval;
>  	u32 insn = le32_to_cpu(*place);
> +	__le32 le_insn;
> 
>  	sval = do_reloc(op, place, val);
>  	imm = sval >> lsb;
> @@ -145,7 +156,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(place, le_insn, me);
> 
>  	if (imm > U16_MAX)
>  		return -ERANGE;
> @@ -154,11 +166,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,
> +			  struct module *me)
>  {
>  	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 +184,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(place, le_insn, me);
> 
>  	/*
>  	 * Extract the upper value bits (including the sign bit) and @@ -189,17
> +204,18 @@ 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, struct module *me)
>  {
>  	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, me);
> 
>  	/* 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, me)) {
>  		insn = le32_to_cpu(*place);
>  		insn &= ~BIT(31);
>  	} else {
> @@ -211,7 +227,8 @@ 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(place, le_insn, me);
>  	return 0;
>  }
> 
> @@ -255,23 +272,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, me);
>  			break;
>  		case R_AARCH64_ABS32:
> -			ovf = reloc_data(RELOC_OP_ABS, loc, val, 32);
> +			ovf = reloc_data(RELOC_OP_ABS, loc, val, 32, me);
>  			break;
>  		case R_AARCH64_ABS16:
> -			ovf = reloc_data(RELOC_OP_ABS, loc, val, 16);
> +			ovf = reloc_data(RELOC_OP_ABS, loc, val, 16, me);
>  			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, me);
>  			break;
>  		case R_AARCH64_PREL32:
> -			ovf = reloc_data(RELOC_OP_PREL, loc, val, 32);
> +			ovf = reloc_data(RELOC_OP_PREL, loc, val, 32, me);
>  			break;
>  		case R_AARCH64_PREL16:
> -			ovf = reloc_data(RELOC_OP_PREL, loc, val, 16);
> +			ovf = reloc_data(RELOC_OP_PREL, loc, val, 16, me);
>  			break;
> 
>  		/* MOVW instruction relocations. */
> @@ -280,88 +297,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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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,
> me);
>  			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, me);
>  			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,
> me);
>  			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, me);
>  			if (ovf && ovf != -ERANGE)
>  				return ovf;
>  			break;
> @@ -369,46 +386,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, me);
>  			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, me);
>  			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, me);
>  			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, me);
>  			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, me);
>  			break;
>  		case R_AARCH64_TSTBR14:
>  			ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 14,
> -					     AARCH64_INSN_IMM_14);
> +					     AARCH64_INSN_IMM_14, me);
>  			break;
>  		case R_AARCH64_CONDBR19:
>  			ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19,
> -					     AARCH64_INSN_IMM_19);
> +					     AARCH64_INSN_IMM_19, me);
>  			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, me);
>  			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, me);
>  			}
>  			break;
> 
> --
> 2.49.0.1204.g71687c7c1d-goog

Thanks for posting the new patch.

I ran kpatch's integration tests and no issues were detected.

The livepatch patches [1][2] (Manually adjusting arch/arm64/Kconfig) have been applied to the kernel (6.15.0).
The kpatch uses the same one as the previous test [3][4].

[1] https://lore.kernel.org/all/20250521111000.2237470-1-mark.rutland@arm.com/
[2] https://lore.kernel.org/all/20250320171559.3423224-3-song@kernel.org/
[3] https://lore.kernel.org/all/TY4PR01MB1377739F1CC08549A619C8635D7BA2@TY4PR01MB13777.jpnprd01.prod.outlook.com/
[4] https://github.com/dynup/kpatch/pull/1439

Tested-by: Toshiyuki Sato <fj6611ie@...jp.fujitsu.com>

Regards,
Toshiyuki Sato

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ