public inbox for op-tee@lists.trustedfirmware.org
 help / color / mirror / Atom feed
* [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
@ 2025-12-04 10:17 Arnd Bergmann via OP-TEE
  2025-12-04 13:27 ` Jens Wiklander
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann via OP-TEE @ 2025-12-04 10:17 UTC (permalink / raw)
  To: Jens Wiklander, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Amirreza Zarrabi, Sumit Garg
  Cc: Arnd Bergmann, Sumit Garg, Alexandre Ghiti, Etienne Carriere,
	Randy Dunlap, op-tee, linux-kernel, linux-riscv

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
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.
---
 include/uapi/linux/tee.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
index cab5cadca8ef..5203977ed35d 100644
--- a/include/uapi/linux/tee.h
+++ b/include/uapi/linux/tee.h
@@ -470,6 +470,7 @@ struct tee_ioctl_object_invoke_arg {
 	__u32 op;
 	__u32 ret;
 	__u32 num_params;
+	__u32 :32;
 	/* num_params tells the actual number of element in params */
 	struct tee_ioctl_param params[];
 };
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-04 10:17 [PATCH] tee: fix tee_ioctl_object_invoke_arg padding Arnd Bergmann via OP-TEE
@ 2025-12-04 13:27 ` Jens Wiklander
  2025-12-05 13:45   ` Harshal Dev via OP-TEE
  2025-12-08  5:24   ` Amirreza Zarrabi via OP-TEE
  0 siblings, 2 replies; 12+ messages in thread
From: Jens Wiklander @ 2025-12-04 13:27 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Amirreza Zarrabi,
	Sumit Garg, Arnd Bergmann, Sumit Garg, Alexandre Ghiti,
	Etienne Carriere, Randy Dunlap, op-tee, linux-kernel, linux-riscv

Hi,

On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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 <arnd@arndb.de>
> ---
> 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.

Thanks,
Jens

> ---
>  include/uapi/linux/tee.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> index cab5cadca8ef..5203977ed35d 100644
> --- a/include/uapi/linux/tee.h
> +++ b/include/uapi/linux/tee.h
> @@ -470,6 +470,7 @@ struct tee_ioctl_object_invoke_arg {
>         __u32 op;
>         __u32 ret;
>         __u32 num_params;
> +       __u32 :32;
>         /* num_params tells the actual number of element in params */
>         struct tee_ioctl_param params[];
>  };
> --
> 2.39.5
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-04 13:27 ` Jens Wiklander
@ 2025-12-05 13:45   ` Harshal Dev via OP-TEE
  2025-12-05 13:56     ` Arnd Bergmann
  2025-12-08  5:24   ` Amirreza Zarrabi via OP-TEE
  1 sibling, 1 reply; 12+ messages in thread
From: Harshal Dev via OP-TEE @ 2025-12-05 13:45 UTC (permalink / raw)
  To: Jens Wiklander, Arnd Bergmann
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Amirreza Zarrabi,
	Sumit Garg, Arnd Bergmann, Sumit Garg, Alexandre Ghiti,
	Etienne Carriere, Randy Dunlap, op-tee, linux-kernel, linux-riscv

Hi,

On 12/4/2025 6:57 PM, Jens Wiklander wrote:
> Hi,
> 
> On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> 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 <arnd@arndb.de>
>> ---
>> 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.
> 
> Thanks,
> Jens
> 
The only user-space client which is currently using this ABI (as per our knowledge)
is the libqcomtee library: https://github.com/quic/quic-teec/blob/main/libqcomtee/src/linux/tee.h#L432

If I understand Arnd's concern correctly, if a compiler used to build the user-space
client skips the padding for tee_ioctl_object_invoke_arg, it could lead to issues.

Let's wait for Amir's view here as well, however I do think the explicit padding would
benefit here.

Regards,
Harshal

>> ---
>>  include/uapi/linux/tee.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>> index cab5cadca8ef..5203977ed35d 100644
>> --- a/include/uapi/linux/tee.h
>> +++ b/include/uapi/linux/tee.h
>> @@ -470,6 +470,7 @@ struct tee_ioctl_object_invoke_arg {
>>         __u32 op;
>>         __u32 ret;
>>         __u32 num_params;
>> +       __u32 :32;
>>         /* num_params tells the actual number of element in params */
>>         struct tee_ioctl_param params[];
>>  };
>> --
>> 2.39.5
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-05 13:45   ` Harshal Dev via OP-TEE
@ 2025-12-05 13:56     ` Arnd Bergmann
  2025-12-05 14:11       ` Harshal Dev via OP-TEE
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2025-12-05 13:56 UTC (permalink / raw)
  To: Harshal Dev, Jens Wiklander, Arnd Bergmann
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Amirreza Zarrabi,
	Sumit Garg, Sumit Garg, Alexandre Ghiti, Etienne Carriere,
	Randy Dunlap, op-tee, linux-kernel, linux-riscv

On Fri, Dec 5, 2025, at 14:45, Harshal Dev wrote:
> On 12/4/2025 6:57 PM, Jens Wiklander wrote:
>> On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>> 
> The only user-space client which is currently using this ABI (as per 
> our knowledge)
> is the libqcomtee library: 
> https://github.com/quic/quic-teec/blob/main/libqcomtee/src/linux/tee.h#L432
>
> If I understand Arnd's concern correctly, if a compiler used to build 
> the user-space
> client skips the padding for tee_ioctl_object_invoke_arg, it could lead 
> to issues.
>
> Let's wait for Amir's view here as well, however I do think the 
> explicit padding would
> benefit here.
>

The problem is much narrower: as the amount of padding is determined
by the architecture specific ABI, kernel and userspace on the same
architecture always agree, and specifically 32-bit Arm userspace
and 64-bit Arm userspace (aarch64) also have the same rules, so there
is no problem on Qualcomm's platforms even with compat 32-bit userspace.

The only actual ABI problem would happen on 32-bit x86 (i386)
userspace running on a 64-bit x86 kernel, since i386-linux compilers
have different alignment rules from most other architectures.

        Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-05 13:56     ` Arnd Bergmann
@ 2025-12-05 14:11       ` Harshal Dev via OP-TEE
  0 siblings, 0 replies; 12+ messages in thread
From: Harshal Dev via OP-TEE @ 2025-12-05 14:11 UTC (permalink / raw)
  To: Arnd Bergmann, Jens Wiklander, Arnd Bergmann
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Amirreza Zarrabi,
	Sumit Garg, Sumit Garg, Alexandre Ghiti, Etienne Carriere,
	Randy Dunlap, op-tee, linux-kernel, linux-riscv



On 12/5/2025 7:26 PM, Arnd Bergmann wrote:
> On Fri, Dec 5, 2025, at 14:45, Harshal Dev wrote:
>> On 12/4/2025 6:57 PM, Jens Wiklander wrote:
>>> On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>>
>> The only user-space client which is currently using this ABI (as per 
>> our knowledge)
>> is the libqcomtee library: 
>> https://github.com/quic/quic-teec/blob/main/libqcomtee/src/linux/tee.h#L432
>>
>> If I understand Arnd's concern correctly, if a compiler used to build 
>> the user-space
>> client skips the padding for tee_ioctl_object_invoke_arg, it could lead 
>> to issues.
>>
>> Let's wait for Amir's view here as well, however I do think the 
>> explicit padding would
>> benefit here.
>>
> 
> The problem is much narrower: as the amount of padding is determined
> by the architecture specific ABI, kernel and userspace on the same
> architecture always agree, and specifically 32-bit Arm userspace
> and 64-bit Arm userspace (aarch64) also have the same rules, so there
> is no problem on Qualcomm's platforms even with compat 32-bit userspace.
> 
> The only actual ABI problem would happen on 32-bit x86 (i386)
> userspace running on a 64-bit x86 kernel, since i386-linux compilers
> have different alignment rules from most other architectures.
> 

Well, as Jens pointed out, the TEE_IOC_OBJECT_INVOKE ABI which supports
object-based IPC currently has no back-end drivers that attempt to communicate
with a TEE running on x86 and implementing their end of the object-IPC protocol
in the firmware. So this obviously won't be an immediate issue.

But as you pointed out, the issue would pop up if someone on x86 decides to implement
it at some point in the far away future.

I do not see any harm this patch would do for our existing implementation, we'll
just update our user-space library ABI in-line with this.

Thanks,
Harshal

>         Arnd


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-04 13:27 ` Jens Wiklander
  2025-12-05 13:45   ` Harshal Dev via OP-TEE
@ 2025-12-08  5:24   ` Amirreza Zarrabi via OP-TEE
  2025-12-08 12:20     ` Sumit Garg via OP-TEE
  1 sibling, 1 reply; 12+ messages in thread
From: Amirreza Zarrabi via OP-TEE @ 2025-12-08  5:24 UTC (permalink / raw)
  To: Jens Wiklander, Arnd Bergmann
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Sumit Garg,
	Arnd Bergmann, Sumit Garg, Alexandre Ghiti, Etienne Carriere,
	Randy Dunlap, op-tee, linux-kernel, linux-riscv

Hi,

On 12/5/2025 12:27 AM, Jens Wiklander wrote:
> Hi,
> 
> On Thu, Dec 4, 2025 at 11:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> 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 <arnd@arndb.de>
>> ---
>> 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.
> 
> 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.

Best Regards,
Amir

>> ---
>>  include/uapi/linux/tee.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>> index cab5cadca8ef..5203977ed35d 100644
>> --- a/include/uapi/linux/tee.h
>> +++ b/include/uapi/linux/tee.h
>> @@ -470,6 +470,7 @@ struct tee_ioctl_object_invoke_arg {
>>         __u32 op;
>>         __u32 ret;
>>         __u32 num_params;
>> +       __u32 :32;
>>         /* num_params tells the actual number of element in params */
>>         struct tee_ioctl_param params[];
>>  };
>> --
>> 2.39.5
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-08  5:24   ` Amirreza Zarrabi via OP-TEE
@ 2025-12-08 12:20     ` Sumit Garg via OP-TEE
  2025-12-08 12:54       ` Harshal Dev via OP-TEE
  0 siblings, 1 reply; 12+ messages in thread
From: Sumit Garg via OP-TEE @ 2025-12-08 12:20 UTC (permalink / raw)
  To: Amirreza Zarrabi
  Cc: Arnd Bergmann, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Sumit Garg, Arnd Bergmann, Alexandre Ghiti, Etienne Carriere,
	Randy Dunlap, op-tee, linux-kernel, linux-riscv

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 <arnd@kernel.org> wrote:
> >>
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> 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 <arnd@arndb.de>
> >> ---
> >> 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 <sumit.garg@oss.qualcomm.com>

> > 
> > 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.

-Sumit

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-08 12:20     ` Sumit Garg via OP-TEE
@ 2025-12-08 12:54       ` Harshal Dev via OP-TEE
  2025-12-09  3:54         ` Amirreza Zarrabi via OP-TEE
  0 siblings, 1 reply; 12+ messages in thread
From: Harshal Dev via OP-TEE @ 2025-12-08 12:54 UTC (permalink / raw)
  To: Sumit Garg, Amirreza Zarrabi, Arnd Bergmann, Jens Wiklander
  Cc: Arnd Bergmann, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Sumit Garg, Alexandre Ghiti, Etienne Carriere, Randy Dunlap,
	op-tee, linux-kernel, linux-riscv



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 <arnd@kernel.org> wrote:
>>>>
>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>
>>>> 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 <arnd@arndb.de>
>>>> ---
>>>> 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 <sumit.garg@oss.qualcomm.com>
> 
>>>
>>> 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.

Regards,
Harshal

> -Sumit
> 
> [...]


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-08 12:54       ` Harshal Dev via OP-TEE
@ 2025-12-09  3:54         ` Amirreza Zarrabi via OP-TEE
  2025-12-16  7:48           ` Jens Wiklander
  0 siblings, 1 reply; 12+ messages in thread
From: Amirreza Zarrabi via OP-TEE @ 2025-12-09  3:54 UTC (permalink / raw)
  To: Harshal Dev, Sumit Garg, Arnd Bergmann, Jens Wiklander
  Cc: Arnd Bergmann, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Sumit Garg, Alexandre Ghiti, Etienne Carriere, Randy Dunlap,
	op-tee, linux-kernel, linux-riscv

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 <arnd@kernel.org> wrote:
>>>>>
>>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>>
>>>>> 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 <arnd@arndb.de>
>>>>> ---
>>>>> 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 <sumit.garg@oss.qualcomm.com>
>>
>>>>
>>>> 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.

Best regards,
Amir

> Regards,
> Harshal
> 
>> -Sumit
>>
>> [...]
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-09  3:54         ` Amirreza Zarrabi via OP-TEE
@ 2025-12-16  7:48           ` Jens Wiklander
  2025-12-16 10:55             ` Harshal Dev via OP-TEE
  0 siblings, 1 reply; 12+ messages in thread
From: Jens Wiklander @ 2025-12-16  7:48 UTC (permalink / raw)
  To: Amirreza Zarrabi
  Cc: Harshal Dev, Sumit Garg, Arnd Bergmann, Arnd Bergmann,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Sumit Garg,
	Alexandre Ghiti, Etienne Carriere, Randy Dunlap, op-tee,
	linux-kernel, linux-riscv

Hi,

On Tue, Dec 9, 2025 at 4:54 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> 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 <arnd@kernel.org> wrote:
> >>>>>
> >>>>> From: Arnd Bergmann <arnd@arndb.de>
> >>>>>
> >>>>> 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 <arnd@arndb.de>
> >>>>> ---
> >>>>> 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 <sumit.garg@oss.qualcomm.com>
> >>
> >>>>
> >>>> 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?

Cheers,
Jens

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-16  7:48           ` Jens Wiklander
@ 2025-12-16 10:55             ` Harshal Dev via OP-TEE
  2025-12-16 13:17               ` Jens Wiklander
  0 siblings, 1 reply; 12+ messages in thread
From: Harshal Dev via OP-TEE @ 2025-12-16 10:55 UTC (permalink / raw)
  To: Jens Wiklander, Amirreza Zarrabi
  Cc: Sumit Garg, Arnd Bergmann, Arnd Bergmann, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Sumit Garg, Alexandre Ghiti,
	Etienne Carriere, Randy Dunlap, op-tee, linux-kernel, linux-riscv

Hi Jens,

On 12/16/2025 1:18 PM, Jens Wiklander wrote:
> Hi,
> 
> On Tue, Dec 9, 2025 at 4:54 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> 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 <arnd@kernel.org> wrote:
>>>>>>>
>>>>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>>>>
>>>>>>> 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 <arnd@arndb.de>
>>>>>>> ---
>>>>>>> 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 <sumit.garg@oss.qualcomm.com>
>>>>
>>>>>>
>>>>>> 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 <harshal.dev@oss.qualcomm.com>

> Cheers,
> Jens


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tee: fix tee_ioctl_object_invoke_arg padding
  2025-12-16 10:55             ` Harshal Dev via OP-TEE
@ 2025-12-16 13:17               ` Jens Wiklander
  0 siblings, 0 replies; 12+ messages in thread
From: Jens Wiklander @ 2025-12-16 13:17 UTC (permalink / raw)
  To: Harshal Dev
  Cc: Amirreza Zarrabi, Sumit Garg, Arnd Bergmann, Arnd Bergmann,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Sumit Garg,
	Alexandre Ghiti, Etienne Carriere, Randy Dunlap, op-tee,
	linux-kernel, linux-riscv

Hi,

On Tue, Dec 16, 2025 at 11:56 AM Harshal Dev
<harshal.dev@oss.qualcomm.com> 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
> > <amirreza.zarrabi@oss.qualcomm.com> 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 <arnd@kernel.org> wrote:
> >>>>>>>
> >>>>>>> From: Arnd Bergmann <arnd@arndb.de>
> >>>>>>>
> >>>>>>> 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 <arnd@arndb.de>
> >>>>>>> ---
> >>>>>>> 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 <sumit.garg@oss.qualcomm.com>
> >>>>
> >>>>>>
> >>>>>> 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 <harshal.dev@oss.qualcomm.com>

Good, thank you, Harshal.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Arnd, what do you prefer, taking this directly or should I send it in
a pull request?

Cheers,
Jens

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-16 13:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04 10:17 [PATCH] tee: fix tee_ioctl_object_invoke_arg padding Arnd Bergmann via OP-TEE
2025-12-04 13:27 ` Jens Wiklander
2025-12-05 13:45   ` Harshal Dev via OP-TEE
2025-12-05 13:56     ` Arnd Bergmann
2025-12-05 14:11       ` Harshal Dev via OP-TEE
2025-12-08  5:24   ` Amirreza Zarrabi via OP-TEE
2025-12-08 12:20     ` Sumit Garg via OP-TEE
2025-12-08 12:54       ` Harshal Dev via OP-TEE
2025-12-09  3:54         ` Amirreza Zarrabi via OP-TEE
2025-12-16  7:48           ` Jens Wiklander
2025-12-16 10:55             ` Harshal Dev via OP-TEE
2025-12-16 13:17               ` Jens Wiklander

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox