Re: [PATCH] media: mali-c55: fix dropped last AEC histogram zone weight

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Endianness bug places the last AEC histogram zone weight in the wrong hardware register byte on big-endian architectures.

Pre-existing issues:
- [High] Casting the unaligned `zone_weights` byte array to `u32 *` causes undefined behavior and alignment faults.
--

commit 731a31e43f59014f133f6c66b9d8861ce15cfc76
Author: David Carlier <devnexen@gmail.com>

media: mali-c55: fix dropped last AEC histogram zone weight

The 15x15 AEC histogram metering grid has 225 per-zone weights, packed
by userspace as a u8 array. The driver writes the first 56 registers
(zones 0 through 223) in a loop, then handles the final register on its
own to keep static analysers from flagging the array access.

> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> index de0e9d898db7b..33e2232ec8f5b 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> @@ -212,6 +212,7 @@ mali_c55_params_aexp_hist_weights(struct mali_c55 *mali_c55,

[ ... ]

>  	for (unsigned int i = 0; i < 56; i++) {
>  		val = ((u32 *)params->zone_weights)[i]
>  			    & MALI_C55_AEXP_HIST_ZONE_WEIGHT_MASK;

[Severity: High]
This isn't a bug introduced by this patch, but could this cast in
mali_c55_params_aexp_hist_weights() in
drivers/media/platform/arm/mali-c55/mali-c55-params.c trigger an alignment
fault on strict architectures?

The zone_weights array is located at offset 10 within struct
mali_c55_params_aexp_weights. Because the configuration block resides at a
4-byte aligned offset within the V4L2 buffer, the zone_weights array starts
at an address that is only 2-byte aligned.

Casting this unaligned pointer to a 32-bit pointer and dereferencing it
inside the loop triggers undefined behavior in C. On strict architectures
like ARM with SCTLR.A enabled, or when auto-vectorized by the compiler,
this can trigger kernel alignment faults.

Because userspace controls the V4L2 configuration buffer, can an
unprivileged user use this to trigger a kernel oops and cause a local denial
of service?

>  		addr = base + MALI_C55_AEXP_HIST_ZONE_WEIGHTS_OFFSET + (4 * i);
>  
>  		mali_c55_ctx_write(mali_c55, addr, val);
>  	}
>  
>  	val = params->zone_weights[MALI_C55_MAX_ZONES - 1];
>  	addr = base + MALI_C55_AEXP_HIST_ZONE_WEIGHTS_OFFSET + (4 * 56);
> +	mali_c55_ctx_write(mali_c55, addr, val & MALI_C55_AEXP_HIST_ZONE_WEIGHT_MASK);

[Severity: Medium]
Is there an endianness issue on big-endian platforms when extracting this
weight as an 8-bit value and implicitly zero-extending it to a 32-bit
integer in mali_c55_params_aexp_hist_weights()?

The original loop processes 4 weights at a time by reading them as a 32-bit
integer and writing that to the memory-backed ctx->registers array. This
preserves the byte stream natively on both architectures.

However, extracting the final 225th weight as an 8-bit value zero-extends
it to a 32-bit integer like 0x0000000E. On big-endian systems, storing this
32-bit value natively places the numeric value at the highest byte address
of the 32-bit word (byte offset 3).

When memcpy_toio() flushes this to the device, will the hardware receive
the weight in the 4th byte of the register instead of the 1st byte (offset 0)
as it expects, silently corrupting the auto-exposure metering weight?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602185305.30759-1-devnexen@gmail.com?part=1