* [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure
@ 2026-09-15 20:24 Arnd Bergmann
2026-09-16 15:43 ` Vincenzo Frascino
2026-09-17 22:15 ` Linus Walleij
0 siblings, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2026-09-15 20:24 UTC (permalink / raw)
To: Daniel Scally, Jacopo Mondi, Hans Verkuil, Linus Walleij,
Vincenzo Frascino
Cc: Arnd Bergmann, linux-media, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The newly added structure has extra padding on the on
some architectures, which triggers a pedantic uapi check:
./usr/include/linux/media/arm/mali-c55-config.h:807:1: error: padding struct size to alignment boundary with 2 bytes [-Werror=padded]
Add explicit padding here to avoid risking information leaks
and incompatibilities between architectures.
Fixes: bb401df68c06 ("media: mali-c55: Add support for CCM")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/uapi/linux/media/arm/mali-c55-config.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
index 84d8f3901405..9c922290e035 100644
--- a/include/uapi/linux/media/arm/mali-c55-config.h
+++ b/include/uapi/linux/media/arm/mali-c55-config.h
@@ -804,6 +804,7 @@ struct mali_c55_params_ccm {
__u16 coeffs[3][3];
__u16 gains[3];
__u16 offs[3];
+ __u16 __pad;
};
/**
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure
2026-09-15 20:24 [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure Arnd Bergmann
@ 2026-09-16 15:43 ` Vincenzo Frascino
2026-09-16 18:55 ` Arnd Bergmann
2026-09-17 22:15 ` Linus Walleij
1 sibling, 1 reply; 5+ messages in thread
From: Vincenzo Frascino @ 2026-09-16 15:43 UTC (permalink / raw)
To: Arnd Bergmann, Daniel Scally, Jacopo Mondi, Hans Verkuil, Linus Walleij
Cc: Arnd Bergmann, linux-media, linux-kernel
Hi Arnd,
quick question since you are mentioning information leaking in the commit message.
On 15/09/2026 21:24, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly added structure has extra padding on the on
> some architectures, which triggers a pedantic uapi check:
>
> ./usr/include/linux/media/arm/mali-c55-config.h:807:1: error: padding struct size to alignment boundary with 2 bytes [-Werror=padded]
>
> Add explicit padding here to avoid risking information leaks
> and incompatibilities between architectures.
>
> Fixes: bb401df68c06 ("media: mali-c55: Add support for CCM")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/uapi/linux/media/arm/mali-c55-config.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> index 84d8f3901405..9c922290e035 100644
> --- a/include/uapi/linux/media/arm/mali-c55-config.h
> +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> @@ -804,6 +804,7 @@ struct mali_c55_params_ccm {
> __u16 coeffs[3][3];
> __u16 gains[3];
> __u16 offs[3];
> + __u16 __pad;
Does this field need to be explicitly zeroed/validated anywhere the structure is
populated? Turning implicit padding into a named member fixes the layout
warning, but by itself does not seem to prevent leaking uninitialized data if
this structure is ever copied from the kernel to userspace. It might also be
worth documenting that __pad is reserved and must be zero.
I think you already checked that changing the explicit structure layout/size is
safe for existing userspace :)
> };
>
> /**
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure
2026-09-16 15:43 ` Vincenzo Frascino
@ 2026-09-16 18:55 ` Arnd Bergmann
2026-09-17 12:54 ` Vincenzo Frascino
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2026-09-16 18:55 UTC (permalink / raw)
To: Vincenzo Frascino, Arnd Bergmann, Daniel Scally, Jacopo Mondi,
Hans Verkuil, Linus Walleij
Cc: linux-media, linux-kernel
On Wed, Sep 16, 2026, at 17:43, Vincenzo Frascino wrote:
>> diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
>> index 84d8f3901405..9c922290e035 100644
>> --- a/include/uapi/linux/media/arm/mali-c55-config.h
>> +++ b/include/uapi/linux/media/arm/mali-c55-config.h
>> @@ -804,6 +804,7 @@ struct mali_c55_params_ccm {
>> __u16 coeffs[3][3];
>> __u16 gains[3];
>> __u16 offs[3];
>> + __u16 __pad;
>
> Does this field need to be explicitly zeroed/validated anywhere the structure is
> populated? Turning implicit padding into a named member fixes the layout
> warning, but by itself does not seem to prevent leaking uninitialized data if
> this structure is ever copied from the kernel to userspace. It might also be
> worth documenting that __pad is reserved and must be zero.
It depends on how the structure is initialized. Depending on the compiler
version and optimization level, a local variable declared as
struct mali_c55_params_ccm v = {};
may end up with uninitialized stack data in unnamed padding, but if you
do a memset(), that should always be safe. If the fields are set individually,
then you also have to set the __pad field, but that's not how you do it here.
> I think you already checked that changing the explicit structure layout/size is
> safe for existing userspace :)
On all architectures other than m68k, the position of the struct members
and the struct size are unchanged by my patch. On m68k. there is no
implied padding at the end of this structure, so this is theoretically
an ABI change, but nobody has a mali device on m68k, so we know that it
is safe.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure
2026-09-16 18:55 ` Arnd Bergmann
@ 2026-09-17 12:54 ` Vincenzo Frascino
0 siblings, 0 replies; 5+ messages in thread
From: Vincenzo Frascino @ 2026-09-17 12:54 UTC (permalink / raw)
To: Arnd Bergmann, Arnd Bergmann, Daniel Scally, Jacopo Mondi,
Hans Verkuil, Linus Walleij
Cc: linux-media, linux-kernel
On 16/09/2026 19:55, Arnd Bergmann wrote:
> It depends on how the structure is initialized. Depending on the compiler
> version and optimization level, a local variable declared as
>
> struct mali_c55_params_ccm v = {};
>
> may end up with uninitialized stack data in unnamed padding, but if you
> do a memset(), that should always be safe. If the fields are set individually,
> then you also have to set the __pad field, but that's not how you do it here.
Fine by me. With this:
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure
2026-09-15 20:24 [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure Arnd Bergmann
2026-09-16 15:43 ` Vincenzo Frascino
@ 2026-09-17 22:15 ` Linus Walleij
1 sibling, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2026-09-17 22:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Scally, Jacopo Mondi, Hans Verkuil, Vincenzo Frascino,
Arnd Bergmann, linux-media, linux-kernel
On Tue, Sep 15, 2026 at 10:25 PM Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly added structure has extra padding on the on
> some architectures, which triggers a pedantic uapi check:
>
> ./usr/include/linux/media/arm/mali-c55-config.h:807:1: error: padding struct size to alignment boundary with 2 bytes [-Werror=padded]
>
> Add explicit padding here to avoid risking information leaks
> and incompatibilities between architectures.
>
> Fixes: bb401df68c06 ("media: mali-c55: Add support for CCM")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-17 22:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 20:24 [PATCH] media: mali-c55: add padding to mali_c55_params_ccm structure Arnd Bergmann
2026-09-16 15:43 ` Vincenzo Frascino
2026-09-16 18:55 ` Arnd Bergmann
2026-09-17 12:54 ` Vincenzo Frascino
2026-09-17 22:15 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®