Re: [PATCH] media: mali-c55: Fix unaligned access of AEC histogram zone weights
From: David Laight
Date: Thu Jul 09 2026 - 16:56:52 EST
On Thu, 9 Jul 2026 20:16:51 +0100
David CARLIER <devnexen@xxxxxxxxx> wrote:
> > Out of curiosity: why is (u32 *) case a UB ?
>
> Alignment, not aliasing. zone_weights is a u8[] at offset 10, so it's
> only 2-byte aligned, and casting that to a u32* (which wants 4) is
> already UB - 6.3.2.3p7 - before you even load through it.
>
> > the usage of __packed triggers the compiler to emit an 'LDUR'
> > ... implications of using LDUR vs LDR on "unaligned access" ... not
> > 100% clear to me.
>
> LDUR vs LDR is only about how the offset is encoded, it's got nothing to
> do with alignment safety. LDR's scaled form needs the immediate to be a
> multiple of the access size, +10 isn't, so gcc can't use it and drops to
> LDUR (unscaled offset). Both happily load from an unaligned address on
> arm64 with SCTLR.A off - LDUR isn't "the unaligned one". The multiple-of-4
> you found is about the immediate field, not the address.
>
> So on arm64 __packed doesn't buy you a safer load, the plain cast already
> worked.
It stops the compiler using 'ldp' (to load two values) which IIRC will trap.
Although the function call following probably ensures that never happens
in this particular case.
David
> What it buys you is not lying to the compiler about the alignment
> (so the UB is gone), plus correct codegen on the arches that do trap -
> which is David's point.
>
> > I would be a bit hesitant in changing the uAPI if there is actually
> > nothing broken ... happy to defer
>
> Fair enough, and you're right that nothing's actually broken - it's arm64
> only so it never faults, this is tidy-up not a fix. I don't feel strongly
> either way. Leave the uAPI as is and I'll drop it, or if you'd rather have
> it cleaned up I'll send the packed union (with MALI_C55_MAX_ZONES / 4 like
> David said). Whatever you prefer.
>
> Cheers