Re: [PATCH v3 07/17] riscv: Maintain epc on misaligned emulation error

[sashiko-bot@kernel.org]

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

New issues:
- [High] Removing `regs->epc = 0` breaks the emulation of misaligned stores that use the `zero` register (`x0`), causing them to leak and write the instruction pointer (EPC) to memory instead of zero.

Pre-existing issues:
- [High] Emulating kernel-mode misaligned memory accesses uses `memcpy()` without exception tables, leading to kernel panics when accessing unmapped memory (e.g., via `bpf_probe_read()` or `get_user()`).
--

--- Patch [7]: [PATCH v3 07/17] riscv: Maintain epc on misaligned emulation error ---
commit 4be6480bf402b9e8440e0f9f14c2dfd93692b4cd
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: Maintain epc on misaligned emulation error

Currently the epc is set to zero when there is a misaligned emulation
error. This will print a misleading error message with the current epc
value of zero instead of the epc value that triggered the emulation.
Avoid setting epc to zero to maintain the state of epc.

> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index 81b7682e6c6db..524215a186e03 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -238,8 +238,6 @@ static int handle_scalar_misaligned_load(struct pt_regs *regs)
>  	if (get_insn(regs, epc, &insn))
>  		return -1;
>  
> -	regs->epc = 0;
> -
>  	if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
>  		len = 4;
>  		shift = 8 * (sizeof(unsigned long) - len);

[Severity: High]
This is a pre-existing issue, but does emulating kernel-mode misaligned
loads and stores in these handlers risk a kernel panic?

Further down in handle_scalar_misaligned_load() and
handle_scalar_misaligned_store(), the code falls back to memcpy() when
user_mode(regs) is false:

	} else {
		memcpy(&val, (u8 *)addr, len);
	}

If a routine like get_user() or bpf_probe_read() is passed an unmapped,
misaligned pointer, the hardware traps here. Since memcpy() lacks exception
table entries, won't the resulting page fault be unhandled and bypass the
intended safety mechanisms?

[ ... ]

> @@ -349,8 +346,6 @@ static int handle_scalar_misaligned_store(struct pt_regs *regs)
>  	if (get_insn(regs, epc, &insn))
>  		return -1;
>  
> -	regs->epc = 0;
> -
>  	val.data_ulong = GET_RS2(insn, regs);

[Severity: High]
Does removing this assignment cause an information leak when emulating
stores from the zero register (x0)?

The GET_RS2() and GET_RS2C() macros calculate the register value by taking
the byte offset into struct pt_regs. For the zero register (index 0), this
maps to offset 0, which corresponds to regs->epc.

Previously, clearing regs->epc = 0 neutralized this aliasing, allowing
misaligned stores using x0 (e.g., sw x0, 1(a0)) to correctly write zero.
Without this, will the code now read the actual EPC value and write the
faulting instruction pointer to memory instead of zero?

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