Re: [PATCH v3 16/17] riscv: kexec: Use generated instruction headers for kexec relocations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unaligned memory accesses during instruction patching in kexec could cause hardware traps on RISC-V.
--

--- Patch [16]: [PATCH v3 16/17] riscv: kexec: Use generated instruction headers for kexec relocations ---
commit 8214fe744bbe604843e11511b3e732e4ee739dad
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: kexec: Use generated instruction headers for kexec relocations

This commit migrates the kexec relocation code to use the generated
instruction headers instead of the hand-written instruction composition
functions.

> diff --git a/arch/riscv/kernel/machine_kexec_file.c b/arch/riscv/kernel/machine_kexec_file.c
> index 54e2d9552e930..cad40e4afd867 100644
> --- a/arch/riscv/kernel/machine_kexec_file.c
> +++ b/arch/riscv/kernel/machine_kexec_file.c

[ ... ]

> @@ -197,12 +171,14 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
>  
>  		switch (r_type) {
>  		case R_RISCV_BRANCH:
> -			*(u32 *)loc = CLEAN_IMM(BTYPE, *(u32 *)loc) |
> -				 ENCODE_BTYPE_IMM(val - addr);
> +			/*
> +			 * For simplicity, use beq as represenative of all
> +			 * branches (they all have the same imm encoding)
> +			 */
> +			riscv_insn_beq_insert_imm((u32 *)loc, val - addr);

[Severity: High]
This is a pre-existing issue, but does casting loc to a u32 pointer here
trigger unaligned memory access traps on strict-alignment hardware?

If the purgatory ELF contains 32-bit instructions aligned on a 2-byte
boundary, which happens frequently due to the RISC-V Compressed extension,
passing this pointer to a function that performs a direct 32-bit
read-modify-write operation could cause a kernel panic during kexec.

>  			break;
>  		case R_RISCV_JAL:
> -			*(u32 *)loc = CLEAN_IMM(JTYPE, *(u32 *)loc) |
> -				 ENCODE_JTYPE_IMM(val - addr);
> +			riscv_insn_jal_insert_imm((u32 *)loc, val  - addr);

[Severity: High]
This is also a pre-existing issue, but could this direct u32 pointer cast
cause similar unaligned memory access traps if the location is 2-byte
aligned?

>  			break;

[ ... ]

>  		case R_RISCV_PCREL_HI20:
>  		case R_RISCV_CALL_PLT:
>  		case R_RISCV_CALL:
> -			*(u64 *)loc = CLEAN_IMM(UITYPE, *(u64 *)loc) |
> -				 ENCODE_UJTYPE_IMM(val - addr);
> +			riscv_insn_auipc_insert_imm((u32 *)loc, RISCV_CONST_HIGH_PART(val - addr));
> +			riscv_insn_jalr_insert_imm((u32 *)loc + 1,
> +						   RISCV_CONST_LOW_PART(val - addr));
>  			break;

[Severity: High]
This is a pre-existing issue, but would accessing loc and loc + 1 as u32
pointers lead to unaligned access faults in arch_kexec_apply_relocations_add
if the base instruction is misaligned?

The kernel module loader avoids this problem by correctly using 16-bit access
pairs, such as riscv_insn_rmw, for instruction patching.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-riscv_insn_table-v3-0-3f8e5f75ba97@gmail.com?part=16