Re: [PATCH RESEND] ARM: alignment: fix LSR #32 and ASR #32 offset decoding

[Robin Murphy]

On 12/08/2026 7:50 pm, Karl Mehltretter wrote:
> The register-offset form of LDR/STR may apply a shift to Rm.  Per
> DecodeImmShift() (ARM ARM DDI0406C section A8.4.3, "Pseudocode details
> of instruction-specified shifts and rotates"), an imm5 of 0 encodes a
> shift of 32 for LSR and ASR; only LSL treats 0 as "no shift", and ROR
> with 0 encodes RRX.
>
> do_alignment() special-cases RRX but not LSR or ASR, and IS_SHIFT() does
> not filter these encodings out, so the block is entered with
> shiftval == 0 and the offset becomes Rm instead of 0 (LSR #32) or the
> replicated sign of Rm (ASR #32).

Yikes! What kind of cursed compiler is emitting these nonsensical encodings?

> do_alignment_finish_ldst() applies the offset to the base-register
> writeback of the post-indexed form, so the emulated access itself uses
> the correct faulting address but Rn is left holding the wrong value.
>
> Reproduced on ARM926EJ-S (versatile_defconfig, CONFIG_ALIGNMENT_TRAP=y,
> gcc 13.3.0) with a misaligned base and Rm = 0x1000:
>
>    ldr r0, [r1], r2, lsr #32   Rn advanced by 0x1000, must be unchanged
>    ldr r0, [r1], r2, asr #32   Rn advanced by 0x1000, must be unchanged
>    ldr r0, [r1], r2, asr #32   with Rm negative, Rn must decrease by 1
>
> All three are correct with the patch applied, while a lsr #1 control
> case is emulated correctly both before and after.

While this does appear technically correct, the fact that it's gone 
unnoticed for 30 years and the relevant hardware is almost all virtually 
obsolete by now does make me wonder whether it's really worth messing 
with now, but at the same time, I guess it's a pretty low-impact change...

Thanks,
Robin.

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
> ---
> Resending after two weeks without feedback; add ARM alignment reviewers
> to Cc.
>
>   arch/arm/mm/alignment.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
> index 812380f30ae3..49045e09ae18 100644
> --- a/arch/arm/mm/alignment.c
> +++ b/arch/arm/mm/alignment.c
> @@ -892,11 +892,17 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>   				break;
>   
>   			case SHIFT_LSR:
> -				offset.un >>= shiftval;
> +				if (shiftval == 0)
> +					offset.un = 0;
> +				else
> +					offset.un >>= shiftval;
>   				break;
>   
>   			case SHIFT_ASR:
> -				offset.sn >>= shiftval;
> +				if (shiftval == 0)
> +					offset.sn >>= 31;
> +				else
> +					offset.sn >>= shiftval;
>   				break;
>   
>   			case SHIFT_RORRRX: