Re: [PATCH] linux-user: Fix msqid_ds struct wrt 32-bit big endian architectures

Hello Peter,

On 7/31/26 20:05, Peter Maydell wrote:
> On Tue, 28 Jul 2026 at 20:32, Helge Deller <[email protected]> wrote:
>>
>> From: Helge Deller <[email protected]>
>>
>> Make sure that the time entries (msg_stime, msg_rtime and msg_ctime)
>> are defined as 64-bit time_t values, since the userspace may access
>> the whole 64-bit value. By this change we fix the word ordering for
>> 32-bit big endian architectures as well.
>>
>> This fixes the msgctl01 LTP testcase on hppa32.
>>
>> Signed-off-by: Helge Deller <[email protected]>
>> ---
>>   linux-user/syscall.c | 30 ++++++++++++------------------
>>   1 file changed, 12 insertions(+), 18 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 740142825d..c93b770ced 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4216,21 +4216,15 @@ static inline abi_long do_semtimedop(int semid,
>>   }
>>   #endif
> 
> I see this has already gone into git, but some late review
> comments. I suspect this is mostly me being confused rather
> than actual problems.
> 
>>
>> +#define target_time64_t         abi_ullong
>> +#define target_swap_time64(x)   tswap64(x)
>> +
>>   struct target_msqid_ds
> 
> Is this the kernel's "struct msqid_ds" (which it calls "Obsolete, used
> only for backwards compatibility and libc5 compiles") or its msqid64_ds?

Yes, it's msqid64_ds.

> The layout matches msqid64_ds, which makes our struct a bit
> confusingly named.

True.
In a follow-up patch this better should be renamed to target_msqid64_ds.

>>   {
>>       struct target_ipc_perm msg_perm;
>> -    abi_ulong msg_stime;
>> -#if TARGET_ABI_BITS == 32
>> -    abi_ulong __unused1;
>> -#endif
>> -    abi_ulong msg_rtime;
>> -#if TARGET_ABI_BITS == 32
>> -    abi_ulong __unused2;
>> -#endif
>> -    abi_ulong msg_ctime;
>> -#if TARGET_ABI_BITS == 32
>> -    abi_ulong __unused3;
>> -#endif
>> +    target_time64_t msg_stime;
>> +    target_time64_t msg_rtime;
>> +    target_time64_t msg_ctime;
> 
> Assuming msqid64_ds, the kernel version of this struct has a comment:
>   * 64 bit architectures use a 64-bit long time field here, while
>   * 32 bit architectures have a pair of unsigned long values.
>   * On big-endian systems, the lower half is in the wrong place.

Those comments are from the generic header:
include/uapi/asm-generic/msgbuf.h
and I think the last sentence about big-endian systems is wrong, as
most platforms provide an own architecture-specific header file, e.g:
arch/xtensa/include/uapi/asm/msgbuf.h
and the big-endian platforms seem to have their high-word first.

> That would make tswap64() not the right swap for 32-bit big
> endian guests.
  
I did my testing on the 32-bit hppa/parisc platform, and with my patch
the results in qemu were correct and were the same as on physical machines.

I just tried in a 32-bit powerpc chroot and the msgctl01 now succeeds there as well.
So, I think my patch is generally ok.

Helge