linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

[PATCH] linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

Posted by no92 via qemu development 1 day, 17 hours ago

The members are declared as __kernel_pid_t in Linux UAPI headers.
Analogous members in struct target_shmid_ds (shm_[cl]pid) are also
declared as abi_int.

Fixes: 1c54ff97bbde ("linux-user: fix and cleanup IPCOP_msg* ipc calls handling")
Signed-off-by: no92 <leo@managarm.org>
---
 linux-user/syscall.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3da5530d42..616d543c7f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4234,8 +4234,8 @@ struct target_msqid_ds
     abi_ulong __msg_cbytes;
     abi_ulong msg_qnum;
     abi_ulong msg_qbytes;
-    abi_ulong msg_lspid;
-    abi_ulong msg_lrpid;
+    abi_int msg_lspid;
+    abi_int msg_lrpid;
     abi_ulong __unused4;
     abi_ulong __unused5;
 };
@@ -4255,8 +4255,8 @@ static inline abi_long target_to_host_msqid_ds(struct msqid_ds *host_md,
     host_md->__msg_cbytes = tswapal(target_md->__msg_cbytes);
     host_md->msg_qnum = tswapal(target_md->msg_qnum);
     host_md->msg_qbytes = tswapal(target_md->msg_qbytes);
-    host_md->msg_lspid = tswapal(target_md->msg_lspid);
-    host_md->msg_lrpid = tswapal(target_md->msg_lrpid);
+    host_md->msg_lspid = tswap32(target_md->msg_lspid);
+    host_md->msg_lrpid = tswap32(target_md->msg_lrpid);
     unlock_user_struct(target_md, target_addr, 0);
     return 0;
 }
@@ -4276,8 +4276,8 @@ static inline abi_long host_to_target_msqid_ds(abi_ulong target_addr,
     target_md->__msg_cbytes = tswapal(host_md->__msg_cbytes);
     target_md->msg_qnum = tswapal(host_md->msg_qnum);
     target_md->msg_qbytes = tswapal(host_md->msg_qbytes);
-    target_md->msg_lspid = tswapal(host_md->msg_lspid);
-    target_md->msg_lrpid = tswapal(host_md->msg_lrpid);
+    target_md->msg_lspid = tswap32(host_md->msg_lspid);
+    target_md->msg_lrpid = tswap32(host_md->msg_lrpid);
     unlock_user_struct(target_md, target_addr, 1);
     return 0;
 }
-- 
2.54.0

Re: [PATCH] linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

Posted by Pierrick Bouvier 1 day, 16 hours ago

On 7/24/2026 9:01 AM, no92 via qemu development wrote:
> The members are declared as __kernel_pid_t in Linux UAPI headers.
> Analogous members in struct target_shmid_ds (shm_[cl]pid) are also
> declared as abi_int.
> 
> Fixes: 1c54ff97bbde ("linux-user: fix and cleanup IPCOP_msg* ipc calls handling")
> Signed-off-by: no92 <leo@managarm.org>
> ---
>  linux-user/syscall.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 3da5530d42..616d543c7f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4234,8 +4234,8 @@ struct target_msqid_ds
>      abi_ulong __msg_cbytes;
>      abi_ulong msg_qnum;
>      abi_ulong msg_qbytes;
> -    abi_ulong msg_lspid;
> -    abi_ulong msg_lrpid;
> +    abi_int msg_lspid;
> +    abi_int msg_lrpid;
>      abi_ulong __unused4;
>      abi_ulong __unused5;
>  };
> @@ -4255,8 +4255,8 @@ static inline abi_long target_to_host_msqid_ds(struct msqid_ds *host_md,
>      host_md->__msg_cbytes = tswapal(target_md->__msg_cbytes);
>      host_md->msg_qnum = tswapal(target_md->msg_qnum);
>      host_md->msg_qbytes = tswapal(target_md->msg_qbytes);
> -    host_md->msg_lspid = tswapal(target_md->msg_lspid);
> -    host_md->msg_lrpid = tswapal(target_md->msg_lrpid);
> +    host_md->msg_lspid = tswap32(target_md->msg_lspid);
> +    host_md->msg_lrpid = tswap32(target_md->msg_lrpid);
>      unlock_user_struct(target_md, target_addr, 0);
>      return 0;
>  }
> @@ -4276,8 +4276,8 @@ static inline abi_long host_to_target_msqid_ds(abi_ulong target_addr,
>      target_md->__msg_cbytes = tswapal(host_md->__msg_cbytes);
>      target_md->msg_qnum = tswapal(host_md->msg_qnum);
>      target_md->msg_qbytes = tswapal(host_md->msg_qbytes);
> -    target_md->msg_lspid = tswapal(host_md->msg_lspid);
> -    target_md->msg_lrpid = tswapal(host_md->msg_lrpid);
> +    target_md->msg_lspid = tswap32(host_md->msg_lspid);
> +    target_md->msg_lrpid = tswap32(host_md->msg_lrpid);
>      unlock_user_struct(target_md, target_addr, 1);
>      return 0;
>  }

The change looks correct in itself.
Did it expose a bug, or did you found this reviewing original commit?

Regards,
Pierrick

Re: [PATCH] linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

Posted by Helge Deller 1 day, 11 hours ago

On 7/24/26 18:48, Pierrick Bouvier wrote:
> On 7/24/2026 9:01 AM, no92 via qemu development wrote:
>> The members are declared as __kernel_pid_t in Linux UAPI headers.
>> Analogous members in struct target_shmid_ds (shm_[cl]pid) are also
>> declared as abi_int.
>>
>> Fixes: 1c54ff97bbde ("linux-user: fix and cleanup IPCOP_msg* ipc calls handling")
>> Signed-off-by: no92 <leo@managarm.org>
>> ---
>>   linux-user/syscall.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 3da5530d42..616d543c7f 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4234,8 +4234,8 @@ struct target_msqid_ds
>>       abi_ulong __msg_cbytes;
>>       abi_ulong msg_qnum;
>>       abi_ulong msg_qbytes;
>> -    abi_ulong msg_lspid;
>> -    abi_ulong msg_lrpid;
>> +    abi_int msg_lspid;
>> +    abi_int msg_lrpid;
>...
> 
> The change looks correct in itself.

No.
abi_ulong is wrong, but abi_int isn't much better either if you look at the kernel sources:

grep -r __kernel_ipc_pid_t arch/

arch/sparc/include/uapi/asm/posix_types.h:typedef unsigned short         __kernel_ipc_pid_t;
arch/sparc/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/m68k/include/uapi/asm/posix_types.h:typedef unsigned short __kernel_ipc_pid_t;
arch/m68k/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/parisc/include/uapi/asm/posix_types.h:typedef unsigned short               __kernel_ipc_pid_t;
arch/parisc/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/arm/include/uapi/asm/posix_types.h:typedef unsigned short          __kernel_ipc_pid_t;
arch/arm/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/s390/include/uapi/asm/posix_types.h:typedef int             __kernel_ipc_pid_t;
arch/s390/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/xtensa/include/uapi/asm/posix_types.h:typedef unsigned short       __kernel_ipc_pid_t;
arch/xtensa/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/x86/include/uapi/asm/posix_types_32.h:typedef unsigned short       __kernel_ipc_pid_t;
arch/x86/include/uapi/asm/posix_types_32.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/sh/include/uapi/asm/posix_types_32.h:typedef unsigned short        __kernel_ipc_pid_t;
arch/sh/include/uapi/asm/posix_types_32.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t
arch/powerpc/include/uapi/asm/posix_types.h:typedef short               __kernel_ipc_pid_t;
arch/powerpc/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t __kernel_ipc_pid_t

So, we probably need to differntiate between platforms and use either abi_int or abi_short.

Helge

Re: [PATCH] linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

Posted by Pierrick Bouvier 1 day, 11 hours ago

On 7/24/2026 2:42 PM, Helge Deller wrote:
> On 7/24/26 18:48, Pierrick Bouvier wrote:
>> On 7/24/2026 9:01 AM, no92 via qemu development wrote:
>>> The members are declared as __kernel_pid_t in Linux UAPI headers.
>>> Analogous members in struct target_shmid_ds (shm_[cl]pid) are also
>>> declared as abi_int.
>>>
>>> Fixes: 1c54ff97bbde ("linux-user: fix and cleanup IPCOP_msg* ipc
>>> calls handling")
>>> Signed-off-by: no92 <leo@managarm.org>
>>> ---
>>>   linux-user/syscall.c | 12 ++++++------
>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 3da5530d42..616d543c7f 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -4234,8 +4234,8 @@ struct target_msqid_ds
>>>       abi_ulong __msg_cbytes;
>>>       abi_ulong msg_qnum;
>>>       abi_ulong msg_qbytes;
>>> -    abi_ulong msg_lspid;
>>> -    abi_ulong msg_lrpid;
>>> +    abi_int msg_lspid;
>>> +    abi_int msg_lrpid;
>> ...
>>
>> The change looks correct in itself.
> 
> No.
> abi_ulong is wrong, but abi_int isn't much better either if you look at
> the kernel sources:
>

I wrongly assumed the definition we had on bsd-user side was correct,
but didn't really think about checking it in linux sources, sorry.

> grep -r __kernel_ipc_pid_t arch/
> 
> arch/sparc/include/uapi/asm/posix_types.h:typedef unsigned short        
> __kernel_ipc_pid_t;
> arch/sparc/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/m68k/include/uapi/asm/posix_types.h:typedef unsigned short
> __kernel_ipc_pid_t;
> arch/m68k/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/parisc/include/uapi/asm/posix_types.h:typedef unsigned
> short               __kernel_ipc_pid_t;
> arch/parisc/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/arm/include/uapi/asm/posix_types.h:typedef unsigned short         
> __kernel_ipc_pid_t;
> arch/arm/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/s390/include/uapi/asm/posix_types.h:typedef int            
> __kernel_ipc_pid_t;
> arch/s390/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/xtensa/include/uapi/asm/posix_types.h:typedef unsigned short      
> __kernel_ipc_pid_t;
> arch/xtensa/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/x86/include/uapi/asm/posix_types_32.h:typedef unsigned short      
> __kernel_ipc_pid_t;
> arch/x86/include/uapi/asm/posix_types_32.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/sh/include/uapi/asm/posix_types_32.h:typedef unsigned short       
> __kernel_ipc_pid_t;
> arch/sh/include/uapi/asm/posix_types_32.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> arch/powerpc/include/uapi/asm/posix_types.h:typedef short              
> __kernel_ipc_pid_t;
> arch/powerpc/include/uapi/asm/posix_types.h:#define __kernel_ipc_pid_t
> __kernel_ipc_pid_t
> 
> So, we probably need to differntiate between platforms and use either
> abi_int or abi_short.
> 
> Helge

Re: [PATCH] linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

Posted by Leo via qemu development 1 day, 16 hours ago

Hi,

> The change looks correct in itself.
> Did it expose a bug, or did you found this reviewing original commit?
I noticed this when the 'msg_lrpid' member was unexpectedly zero after a
msgctl(IPC_STAT) call, and printing the 'struct msqid_ds' in question with gdb
revealed that the expected value was not in 'msg_lrpid', but the unused member
located right after. The 'Fixes' line is based purely on running git blame.

Cheers,
Leo

Re: [PATCH] linux-user: fix incorrect msg_l[sr]pid members of target_msqid_ds

Posted by Pierrick Bouvier 1 day, 16 hours ago

On 7/24/2026 10:13 AM, Leo via qemu development wrote:
> Hi,
> 
>> The change looks correct in itself.
>> Did it expose a bug, or did you found this reviewing original commit?
> I noticed this when the 'msg_lrpid' member was unexpectedly zero after a
> msgctl(IPC_STAT) call, and printing the 'struct msqid_ds' in question with gdb
> revealed that the expected value was not in 'msg_lrpid', but the unused member
> located right after. The 'Fixes' line is based purely on running git blame.
> 
> Cheers,
> Leo
> 
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>

Regards,
Pierrick