Re: [PATCH] ARC: check user addresses in unaligned access emulation

[Vineet Gupta <vgupta@kernel.org>]

On 8/24/26 13:37, Jérémy Jean wrote:
> ARC700 raises an alignment exception before checking access permissions.
> Consequently, a userspace load or store using a misaligned kernel address
> reaches misaligned_fixup() before the processor rejects it.

How do you know this for sure. Did you run into this issue yourself and 
were able to prove this ordering.

And even if you found some documentation can you confirm this to be 
happening on real ARC700 hardware.
I've been maintaining ARC forever and even do I don't have access to 
working ARC700 silicon.

I can sympathize with your need to send a fix and it might actually be 
correct.
But I can't take a fix that theoretically fixes something w/o 
demonstrating what real life case it caters so.

So Nack, unless you have one of the valid reasons above.

-Vineet

> misaligned_fixup() decodes the instruction and repeats the access using
> byte loads or stores. These accesses run in kernel mode, and exception
> tables only handle accesses that fault. A mapped kernel address is
> therefore read or written with supervisor permissions.
>
> Use access_ok() to reject addresses outside the user range before calling
> the helpers. Check the whole 2- or 4-byte range and use the checked address
> for the emulated operation.
>
> Fixes: 2e651ea1596b ("ARC: Unaligned access emulation")
> Assisted-by: Codex:gpt-daybreak-blue
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>   arch/arc/kernel/unaligned.c | 23 +++++++++++++++++++----
>   1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arc/kernel/unaligned.c b/arch/arc/kernel/unaligned.c
> index 3b2d8b1bd271..f6079dc89a6d 100644
> --- a/arch/arc/kernel/unaligned.c
> +++ b/arch/arc/kernel/unaligned.c
> @@ -133,6 +133,8 @@ int no_unaligned_warning __read_mostly = 1;	/* Only 1 warning by default */
>   static void fixup_load(struct disasm_state *state, struct pt_regs *regs,
>   			struct callee_regs *cregs)
>   {
> +	unsigned long address;
> +	unsigned int size;
>   	int val;
>   
>   	/* register write back */
> @@ -143,10 +145,15 @@ static void fixup_load(struct disasm_state *state, struct pt_regs *regs,
>   			state->src2 = 0;
>   	}
>   
> +	address = state->src1 + state->src2;
> +	size = state->zz ? 2 : 4;
> +	if (!access_ok((void __user *)address, size))
> +		goto fault;
> +
>   	if (state->zz == 0) {
> -		get32_unaligned_check(val, state->src1 + state->src2);
> +		get32_unaligned_check(val, address);
>   	} else {
> -		get16_unaligned_check(val, state->src1 + state->src2);
> +		get16_unaligned_check(val, address);
>   
>   		if (state->x)
>   			val = (val << 16) >> 16;
> @@ -163,6 +170,9 @@ fault:	state->fault = 1;
>   static void fixup_store(struct disasm_state *state, struct pt_regs *regs,
>   			struct callee_regs *cregs)
>   {
> +	unsigned long address;
> +	unsigned int size;
> +
>   	/* register write back */
>   	if ((state->aa == 1) || (state->aa == 2)) {
>   		set_reg(state->wb_reg, state->src2 + state->src3, regs, cregs);
> @@ -181,11 +191,16 @@ static void fixup_store(struct disasm_state *state, struct pt_regs *regs,
>   		}
>   	}
>   
> +	address = state->src2 + state->src3;
> +	size = state->zz ? 2 : 4;
> +	if (!access_ok((void __user *)address, size))
> +		goto fault;
> +
>   	/* write fix-up */
>   	if (!state->zz)
> -		put32_unaligned_check(state->src1, state->src2 + state->src3);
> +		put32_unaligned_check(state->src1, address);
>   	else
> -		put16_unaligned_check(state->src1, state->src2 + state->src3);
> +		put16_unaligned_check(state->src1, address);
>   
>   	return;
>