Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding

Hi,

On Tue, Dec 16, 2025 at 2:17 PM Jens Wiklander
<[email protected]> wrote:
>
> Hi,
>
> On Tue, Dec 16, 2025 at 11:56 AM Harshal Dev
> <[email protected]> wrote:
> >
> > Hi Jens,
> >
> > On 12/16/2025 1:18 PM, Jens Wiklander wrote:
> > > Hi,
> > >
> > > On Tue, Dec 9, 2025 at 4:54 AM Amirreza Zarrabi
> > > <[email protected]> wrote:
> > >>
> > >> Hi,
> > >>
> > >> On 12/8/2025 11:54 PM, Harshal Dev wrote:
> > >>>
> > >>>
> > >>> On 12/8/2025 5:50 PM, Sumit Garg via OP-TEE wrote:
> > >>>> On Mon, Dec 08, 2025 at 04:24:17PM +1100, Amirreza Zarrabi wrote:
> > >>>>> Hi,
> > >>>>>
> > >>>>> On 12/5/2025 12:27 AM, Jens Wiklander wrote:
> > >>>>>> Hi,
> > >>>>>>
> > >>>>>> On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <[email protected]> wrote:
> > >>>>>>>
> > >>>>>>> From: Arnd Bergmann <[email protected]>
> > >>>>>>>
> > >>>>>>> The tee_ioctl_object_invoke_arg structure has padding on some
> > >>>>>>> architectures but not on x86-32 and a few others:
> > >>>>>>>
> > >>>>>>> include/linux/tee.h:474:32: error: padding struct to align 'params' [-Werror=padded]
> > >>>>>>>
> > >>>>>>> I expect that all current users of this are on architectures that do
> > >>>>>>> have implicit padding here (arm64, arm, x86, riscv), so make the padding
> > >>>>>>> explicit in order to avoid surprises if this later gets used elsewhere.
> > >>>>>>>
> > >>>>>>> Fixes: d5b8b0fa1775 ("tee: add TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF")
> > >>>>>>> Signed-off-by: Arnd Bergmann <[email protected]>
> > >>>>>>> ---
> > >>>>>>> The new interface showed up in 6.18, but I only came across this after
> > >>>>>>> that was released. Changing it now is technically an ABI change on
> > >>>>>>> architectures with unusual padding rules, so please consider carefully
> > >>>>>>> whether we want to do it this way or not.
> > >>>>>>>
> > >>>>>>> Working around the ABI differences without an ABI change is possible,
> > >>>>>>> but adds a lot of complexity for compat handling.
> > >>>>>>
> > >>>>>> This is currently only used by the recently introduced qcomtee backend
> > >>>>>> driver. So it's only used on a few arm64 Qualcomm platforms right now.
> > >>>>>>
> > >>>>>> I think we should take this patch, but let's hear what others think.
> > >>>>
> > >>>> Yeah since it's not an ABI issue on arm64 platforms where QTEE runs, so:
> > >>>>
> > >>>> Reviewed-by: Sumit Garg <[email protected]>
> > >>>>
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Jens
> > >>>>>>
> > >>>>>
> > >>>>> I agree. We should take this patch. As noted, there are not many
> > >>>>> clients relying on it yet, so updating the userspace should
> > >>>>> be straightforward.
> > >>>>
> > >>>> You should rather test without any userspace library update to test it's
> > >>>> not an ABI issue. Just for correctness sake, you can update the library
> > >>>> too.
> > >>>>
> > >>>
> > >>> I'll take the time to test it at some point this week both with and without updating
> > >>> the library ABI.
> > >>>
> > >>
> > >> Summit, that was the plan from the beginning, that's why I did not add "Reviewed-by:"
> > >> in the first place. Thanks Harshal for volunteering.
> > >
> > > Are we good with this patch now?
> > >
> >
> > Yes, this is good from our side.
> >
> > Tested-by: Harshal Dev <[email protected]>
>
> Good, thank you, Harshal.
>
> Reviewed-by: Jens Wiklander <[email protected]>
>
> Arnd, what do you prefer, taking this directly or should I send it in
> a pull request?

I forgot about this. I'm picking up this.

Cheers,
Jens