[PATCH 1/2] drm/panthor: Fix missing explicit padding in drm_panthor_gpu_info

[Steven Price]

On 17/04/2025 11:05, Boris Brezillon wrote:
> drm_panthor_gpu_info::shader_present is currently automatically offset
> by 4 byte to meet Arm's 32-bit/64-bit field alignment rules, but those
> constraints don't stand on 32-bit x86 and cause a mismatch when running
> an x86 binary in a user emulated environment like FEX. It's also
> generally agreed that uAPIs should explicitly pad their struct fields,
> which we originally intended to do, but a mistake slipped through during
> the submission process, leading drm_panthor_gpu_info::shader_present to
> be misaligned.
> 
> This uAPI change doesn't break any of the existing users of panthor
> which are either arm32 or arm64 where the 64-bit alignment of
> u64 fields is already enforced a the compiler level.
> 
> Fixes: 0f25e493a246 ("drm/panthor: Add uAPI")
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  include/uapi/drm/panthor_drm.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index 97e2c4510e69..1379a2d4548c 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -293,6 +293,18 @@ struct drm_panthor_gpu_info {
>  	/** @as_present: Bitmask encoding the number of address-space exposed by the MMU. */
>  	__u32 as_present;
>  
> +	/**
> +	 * @garbage: Unused field that's not even zero-checked.
> +	 *
> +	 * This originates from a missing padding that leaked in the initial driver submission
> +	 * and was only found when testing the driver in a 32-bit x86 environment, where
> +	 * u64 field alignment rules are relaxed compared to aarch32.
> +	 *
> +	 * This field can't be repurposed, because it's never been checked by the driver and
> +	 * userspace is not guaranteed to zero it out.

Why would user space be providing this structure? This is meant to be
provided from the kernel to user space, and (fingers-crossed) we've been
zeroing the padding even though not explicitly? (rather than leaking
some kernel data).

Other than the comment - yes this is a uAPI mistake we should fix.

I'm not sure how much we care about historic x86 uAPI but it also should
be possible to identify an old x86 client using the x86 padding because
the structure will be too short. But my preference would be to say "it's
always been broken on x86" and therefore there's no regression.

Thanks,
Steve

> +	 */
> +	__u32 garbage;
> +
>  	/** @shader_present: Bitmask encoding the shader cores exposed by the GPU. */
>  	__u64 shader_present;
>