* stat64 wrong on sparc64 user
@ 2023-03-28 11:48 Luca Bonissi
2023-03-28 11:55 ` Thomas Huth
2023-03-28 14:52 ` Richard Henderson
0 siblings, 2 replies; 7+ messages in thread
From: Luca Bonissi @ 2023-03-28 11:48 UTC (permalink / raw)
To: qemu-devel, Laurent Vivier
On qemu-sparc64 (userspace) the struct "target_stat64" is not correctly
padded, so the field st_rdev is not correctly aligned and will report
wrong major/minor (e.g. for /dev/zero it reports 0,0x10500000 instead of
1,5).
Here patch to solve the issue (it also fixes incorrect size on some fields):
--- qemu-20230327/linux-user/syscall_defs.h 2023-03-27
15:41:42.000000000 +0200
+++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27
21:43:25.615115126 +0200
@@ -1450,7 +1450,7 @@ struct target_stat {
unsigned int st_dev;
abi_ulong st_ino;
unsigned int st_mode;
- unsigned int st_nlink;
+ short int st_nlink;
unsigned int st_uid;
unsigned int st_gid;
unsigned int st_rdev;
@@ -1465,8 +1465,7 @@ struct target_stat {
#define TARGET_HAS_STRUCT_STAT64
struct target_stat64 {
- unsigned char __pad0[6];
- unsigned short st_dev;
+ uint64_t st_dev;
uint64_t st_ino;
uint64_t st_nlink;
@@ -1476,14 +1475,13 @@ struct target_stat64 {
unsigned int st_uid;
unsigned int st_gid;
- unsigned char __pad2[6];
- unsigned short st_rdev;
+ unsigned int __pad0;
+ uint64_t st_rdev;
int64_t st_size;
int64_t st_blksize;
- unsigned char __pad4[4];
- unsigned int st_blocks;
+ int64_t st_blocks;
abi_ulong target_st_atime;
abi_ulong target_st_atime_nsec;
@@ -1522,8 +1520,7 @@ struct target_stat {
#define TARGET_HAS_STRUCT_STAT64
struct target_stat64 {
- unsigned char __pad0[6];
- unsigned short st_dev;
+ uint64_t st_dev;
uint64_t st_ino;
@@ -1533,8 +1530,7 @@ struct target_stat64 {
unsigned int st_uid;
unsigned int st_gid;
- unsigned char __pad2[6];
- unsigned short st_rdev;
+ uint64_t st_rdev;
unsigned char __pad3[8];
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: stat64 wrong on sparc64 user
2023-03-28 11:48 stat64 wrong on sparc64 user Luca Bonissi
@ 2023-03-28 11:55 ` Thomas Huth
2023-03-28 12:22 ` Luca Bonissi
2023-03-28 14:52 ` Richard Henderson
1 sibling, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2023-03-28 11:55 UTC (permalink / raw)
To: Luca Bonissi, qemu-devel, Laurent Vivier
On 28/03/2023 13.48, Luca Bonissi wrote:
> On qemu-sparc64 (userspace) the struct "target_stat64" is not correctly
> padded, so the field st_rdev is not correctly aligned and will report wrong
> major/minor (e.g. for /dev/zero it reports 0,0x10500000 instead of 1,5).
>
> Here patch to solve the issue (it also fixes incorrect size on some fields):
>
> --- qemu-20230327/linux-user/syscall_defs.h 2023-03-27 15:41:42.000000000
> +0200
> +++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27
> 21:43:25.615115126 +0200
> @@ -1450,7 +1450,7 @@ struct target_stat {
> unsigned int st_dev;
> abi_ulong st_ino;
> unsigned int st_mode;
> - unsigned int st_nlink;
> + short int st_nlink;
> unsigned int st_uid;
That looks wrong at a first glance. IIRC Sparc is a very strictly aligned
architecture, so if the previous field "st_mode" was aligned to a 4-byte
boundary, the "st_uid" field now would not be aligned anymore... are you
sure about this change? Maybe it needs a padding field now?
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: stat64 wrong on sparc64 user
2023-03-28 11:55 ` Thomas Huth
@ 2023-03-28 12:22 ` Luca Bonissi
2023-03-28 12:41 ` Thomas Huth
2023-03-29 16:22 ` Laurent Vivier
0 siblings, 2 replies; 7+ messages in thread
From: Luca Bonissi @ 2023-03-28 12:22 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Laurent Vivier
On 28/03/23 13:55, Thomas Huth wrote:
> On 28/03/2023 13.48, Luca Bonissi wrote:
>> --- qemu-20230327/linux-user/syscall_defs.h 2023-03-27
>> 15:41:42.000000000 +0200
>> +++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27
>> 21:43:25.615115126 +0200
>> @@ -1450,7 +1450,7 @@ struct target_stat {
>> unsigned int st_dev;
>> abi_ulong st_ino;
>> unsigned int st_mode;
>> - unsigned int st_nlink;
>> + short int st_nlink;
>> unsigned int st_uid;
>
> That looks wrong at a first glance. IIRC Sparc is a very strictly
> aligned architecture, so if the previous field "st_mode" was aligned to
> a 4-byte boundary, the "st_uid" field now would not be aligned
> anymore... are you sure about this change? Maybe it needs a padding
> field now?
The padding is automatic (either on Sparc or x86-64): short will be
aligned to 2-byte boundary, int will be aligned to 4-byte boundary, long
will be aligned to 8-byte boundary.
E.g.:
st_dev=0x05060708;
st_ino=0x1112131415161718;
st_mode=0x1a1b1c1d;
st_nlink=0x2728;
st_uid=0x2a2b2c2d;
st_gid=0x3a3b3c3d;
st_rdev=0x35363738;
st_size=0x4142434445464748;
st_blksize=0x5152535455565758;
will result (sparc64 - big endian):
00: 05 06 07 08 00 00 00 00
08: 11 12 13 14 15 16 17 18
10: 1A 1B 1C 1D 27 28 00 00
18: 2A 2B 2C 2D 3A 3B 3C 3D
20: 35 36 37 38 00 00 00 00
28: 41 42 43 44 45 46 47 48
30: 00 00 00 00 00 00 00 00
38: 00 00 00 00 00 00 00 00
40: 00 00 00 00 00 00 00 00
48: 51 52 53 54 55 56 57 58
50: 00 00 00 00 00 00 00 00
58: 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00
Or on x86-64 (little endian):
00: 08 07 06 05 00 00 00 00
08: 18 17 16 15 14 13 12 11
10: 1D 1C 1B 1A 28 27 00 00
18: 2D 2C 2B 2A 3D 3C 3B 3A
20: 38 37 36 35 00 00 00 00
28: 48 47 46 45 44 43 42 41
30: 00 00 00 00 00 00 00 00
38: 00 00 00 00 00 00 00 00
40: 00 00 00 00 00 00 00 00
48: 58 57 56 55 54 53 52 51
50: 00 00 00 00 00 00 00 00
58: 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00
Please note the automatic padding between "st_dev" and "st_ino" (offset
0x04, 4 bytes), "st_nlink" and "st_uid" (offset 0x16, 2 bytes),
"st_rdev" and "st_size" (offset 0x24, 4 bytes).
Placing st_nlink as int would result in incorrect big/little endian
conversion, so it should be set as short. If you like clearer source
code, you can optionally add padding, but it is not mandatory.
Thanks!
Luca
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: stat64 wrong on sparc64 user
2023-03-28 12:22 ` Luca Bonissi
@ 2023-03-28 12:41 ` Thomas Huth
2023-03-29 16:22 ` Laurent Vivier
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2023-03-28 12:41 UTC (permalink / raw)
To: Luca Bonissi, qemu-devel, Laurent Vivier
On 28/03/2023 14.22, Luca Bonissi wrote:
> On 28/03/23 13:55, Thomas Huth wrote:
>> On 28/03/2023 13.48, Luca Bonissi wrote:
>>> --- qemu-20230327/linux-user/syscall_defs.h 2023-03-27
>>> 15:41:42.000000000 +0200
>>> +++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27
>>> 21:43:25.615115126 +0200
>>> @@ -1450,7 +1450,7 @@ struct target_stat {
>>> unsigned int st_dev;
>>> abi_ulong st_ino;
>>> unsigned int st_mode;
>>> - unsigned int st_nlink;
>>> + short int st_nlink;
>>> unsigned int st_uid;
>>
>> That looks wrong at a first glance. IIRC Sparc is a very strictly aligned
>> architecture, so if the previous field "st_mode" was aligned to a 4-byte
>> boundary, the "st_uid" field now would not be aligned anymore... are you
>> sure about this change? Maybe it needs a padding field now?
>
> The padding is automatic (either on Sparc or x86-64): short will be aligned
> to 2-byte boundary, int will be aligned to 4-byte boundary, long will be
> aligned to 8-byte boundary.
Ah, right, I blindly assumed that we were using __attribute__((packed)) here
for such structs... but that does not seem to be the case, so never mind.
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: stat64 wrong on sparc64 user
2023-03-28 12:22 ` Luca Bonissi
2023-03-28 12:41 ` Thomas Huth
@ 2023-03-29 16:22 ` Laurent Vivier
2023-03-30 11:13 ` Luca Bonissi
1 sibling, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2023-03-29 16:22 UTC (permalink / raw)
To: Luca Bonissi, Thomas Huth, qemu-devel
Le 28/03/2023 à 14:22, Luca Bonissi a écrit :
> On 28/03/23 13:55, Thomas Huth wrote:
>> On 28/03/2023 13.48, Luca Bonissi wrote:
>>> --- qemu-20230327/linux-user/syscall_defs.h 2023-03-27 15:41:42.000000000 +0200
>>> +++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27 21:43:25.615115126 +0200
>>> @@ -1450,7 +1450,7 @@ struct target_stat {
>>> unsigned int st_dev;
>>> abi_ulong st_ino;
>>> unsigned int st_mode;
>>> - unsigned int st_nlink;
>>> + short int st_nlink;
>>> unsigned int st_uid;
>>
>> That looks wrong at a first glance. IIRC Sparc is a very strictly aligned architecture, so if the
>> previous field "st_mode" was aligned to a 4-byte boundary, the "st_uid" field now would not be
>> aligned anymore... are you sure about this change? Maybe it needs a padding field now?
>
> The padding is automatic (either on Sparc or x86-64): short will be aligned to 2-byte boundary, int
> will be aligned to 4-byte boundary, long will be aligned to 8-byte boundary.
>
> E.g.:
> st_dev=0x05060708;
> st_ino=0x1112131415161718;
> st_mode=0x1a1b1c1d;
> st_nlink=0x2728;
> st_uid=0x2a2b2c2d;
> st_gid=0x3a3b3c3d;
> st_rdev=0x35363738;
> st_size=0x4142434445464748;
> st_blksize=0x5152535455565758;
>
> will result (sparc64 - big endian):
> 00: 05 06 07 08 00 00 00 00
> 08: 11 12 13 14 15 16 17 18
> 10: 1A 1B 1C 1D 27 28 00 00
> 18: 2A 2B 2C 2D 3A 3B 3C 3D
> 20: 35 36 37 38 00 00 00 00
> 28: 41 42 43 44 45 46 47 48
> 30: 00 00 00 00 00 00 00 00
> 38: 00 00 00 00 00 00 00 00
> 40: 00 00 00 00 00 00 00 00
> 48: 51 52 53 54 55 56 57 58
> 50: 00 00 00 00 00 00 00 00
> 58: 00 00 00 00 00 00 00 00
> 60: 00 00 00 00 00 00 00 00
>
> Or on x86-64 (little endian):
> 00: 08 07 06 05 00 00 00 00
> 08: 18 17 16 15 14 13 12 11
> 10: 1D 1C 1B 1A 28 27 00 00
> 18: 2D 2C 2B 2A 3D 3C 3B 3A
> 20: 38 37 36 35 00 00 00 00
> 28: 48 47 46 45 44 43 42 41
> 30: 00 00 00 00 00 00 00 00
> 38: 00 00 00 00 00 00 00 00
> 40: 00 00 00 00 00 00 00 00
> 48: 58 57 56 55 54 53 52 51
> 50: 00 00 00 00 00 00 00 00
> 58: 00 00 00 00 00 00 00 00
> 60: 00 00 00 00 00 00 00 00
>
> Please note the automatic padding between "st_dev" and "st_ino" (offset 0x04, 4 bytes), "st_nlink"
> and "st_uid" (offset 0x16, 2 bytes), "st_rdev" and "st_size" (offset 0x24, 4 bytes).
>
> Placing st_nlink as int would result in incorrect big/little endian conversion, so it should be set
> as short. If you like clearer source code, you can optionally add padding, but it is not mandatory.
>
To have automatic alignment according to target ABI, you must use abi_XXX type (see
include/exec/user/abitypes.h)
For sparc, from the kernel, we have:
struct stat {
unsigned int st_dev;
__kernel_ino_t st_ino;
__kernel_mode_t st_mode;
short st_nlink;
__kernel_uid32_t st_uid;
__kernel_gid32_t st_gid;
unsigned int st_rdev;
long st_size;
long st_atime;
long st_mtime;
long st_ctime;
long st_blksize;
long st_blocks;
unsigned long __unused4[2];
};
So for the st_link we need to use abi_short.
We can do the same with stat64 and other fields (see linux/arch/sparc/include/uapi/asm/stat.h)
Thanks,
Laurent
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: stat64 wrong on sparc64 user
2023-03-29 16:22 ` Laurent Vivier
@ 2023-03-30 11:13 ` Luca Bonissi
0 siblings, 0 replies; 7+ messages in thread
From: Luca Bonissi @ 2023-03-30 11:13 UTC (permalink / raw)
To: Laurent Vivier, Thomas Huth, qemu-devel, Richard Henderson
On 29/03/23 18:22, Laurent Vivier wrote:
> Le 28/03/2023 à 14:22, Luca Bonissi a écrit :
>> On 28/03/23 13:55, Thomas Huth wrote:
>>> On 28/03/2023 13.48, Luca Bonissi wrote:
>>>> --- qemu-20230327/linux-user/syscall_defs.h 2023-03-27
>>>> 15:41:42.000000000 +0200
>>>> +++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27
>>>> 21:43:25.615115126 +0200
>>>> @@ -1450,7 +1450,7 @@ struct target_stat {
>>>> unsigned int st_dev;
>>>> abi_ulong st_ino;
>>>> unsigned int st_mode;
>>>> - unsigned int st_nlink;
>>>> + short int st_nlink;
>>>> unsigned int st_uid;
>>>
>>
>
> To have automatic alignment according to target ABI, you must use
> abi_XXX type (see include/exec/user/abitypes.h)
>
I tried to keep as much as possibile the source code untouched, but no
problem to change all fields with abi_XXX. Tested for sparc and sparc64:
--- qemu-20230327/linux-user/syscall_defs.h.orig 2023-03-27
15:41:42.000000000 +0200
+++ qemu-20230327/linux-user/syscall_defs.h 2023-03-30
12:52:46.308640526 +0200
@@ -1447,13 +1447,13 @@ struct target_eabi_stat64 {
#elif defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
struct target_stat {
- unsigned int st_dev;
+ abi_uint st_dev;
abi_ulong st_ino;
- unsigned int st_mode;
- unsigned int st_nlink;
- unsigned int st_uid;
- unsigned int st_gid;
- unsigned int st_rdev;
+ abi_uint st_mode;
+ abi_short st_nlink;
+ abi_uint st_uid;
+ abi_uint st_gid;
+ abi_uint st_rdev;
abi_long st_size;
abi_long target_st_atime;
abi_long target_st_mtime;
@@ -1465,25 +1465,23 @@ struct target_stat {
#define TARGET_HAS_STRUCT_STAT64
struct target_stat64 {
- unsigned char __pad0[6];
- unsigned short st_dev;
+ abi_ullong st_dev;
- uint64_t st_ino;
- uint64_t st_nlink;
+ abi_ullong st_ino;
+ abi_ullong st_nlink;
- unsigned int st_mode;
+ abi_uint st_mode;
- unsigned int st_uid;
- unsigned int st_gid;
+ abi_uint st_uid;
+ abi_uint st_gid;
- unsigned char __pad2[6];
- unsigned short st_rdev;
+ abi_uint __pad0;
+ abi_ullong st_rdev;
- int64_t st_size;
- int64_t st_blksize;
+ abi_llong st_size;
+ abi_llong st_blksize;
- unsigned char __pad4[4];
- unsigned int st_blocks;
+ abi_llong st_blocks;
abi_ulong target_st_atime;
abi_ulong target_st_atime_nsec;
@@ -1501,13 +1499,13 @@ struct target_stat64 {
#define TARGET_STAT_HAVE_NSEC
struct target_stat {
- unsigned short st_dev;
+ abi_ushort st_dev;
abi_ulong st_ino;
- unsigned short st_mode;
- short st_nlink;
- unsigned short st_uid;
- unsigned short st_gid;
- unsigned short st_rdev;
+ abi_ushort st_mode;
+ abi_short st_nlink;
+ abi_ushort st_uid;
+ abi_ushort st_gid;
+ abi_ushort st_rdev;
abi_long st_size;
abi_long target_st_atime;
abi_ulong target_st_atime_nsec;
@@ -1522,39 +1520,37 @@ struct target_stat {
#define TARGET_HAS_STRUCT_STAT64
struct target_stat64 {
- unsigned char __pad0[6];
- unsigned short st_dev;
+ abi_ullong st_dev;
- uint64_t st_ino;
+ abi_ullong st_ino;
- unsigned int st_mode;
- unsigned int st_nlink;
+ abi_uint st_mode;
+ abi_uint st_nlink;
- unsigned int st_uid;
- unsigned int st_gid;
+ abi_uint st_uid;
+ abi_uint st_gid;
- unsigned char __pad2[6];
- unsigned short st_rdev;
+ abi_ullong st_rdev;
unsigned char __pad3[8];
- int64_t st_size;
- unsigned int st_blksize;
+ abi_llong st_size;
+ abi_uint st_blksize;
unsigned char __pad4[8];
- unsigned int st_blocks;
+ abi_uint st_blocks;
- unsigned int target_st_atime;
- unsigned int target_st_atime_nsec;
+ abi_uint target_st_atime;
+ abi_uint target_st_atime_nsec;
- unsigned int target_st_mtime;
- unsigned int target_st_mtime_nsec;
+ abi_uint target_st_mtime;
+ abi_uint target_st_mtime_nsec;
- unsigned int target_st_ctime;
- unsigned int target_st_ctime_nsec;
+ abi_uint target_st_ctime;
+ abi_uint target_st_ctime_nsec;
- unsigned int __unused1;
- unsigned int __unused2;
+ abi_uint __unused1;
+ abi_uint __unused2;
};
#elif defined(TARGET_PPC)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: stat64 wrong on sparc64 user
2023-03-28 11:48 stat64 wrong on sparc64 user Luca Bonissi
2023-03-28 11:55 ` Thomas Huth
@ 2023-03-28 14:52 ` Richard Henderson
1 sibling, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2023-03-28 14:52 UTC (permalink / raw)
To: Luca Bonissi, qemu-devel, Laurent Vivier
On 3/28/23 04:48, Luca Bonissi wrote:
> On qemu-sparc64 (userspace) the struct "target_stat64" is not correctly padded, so the
> field st_rdev is not correctly aligned and will report wrong major/minor (e.g. for
> /dev/zero it reports 0,0x10500000 instead of 1,5).
>
> Here patch to solve the issue (it also fixes incorrect size on some fields):
>
> --- qemu-20230327/linux-user/syscall_defs.h 2023-03-27 15:41:42.000000000 +0200
> +++ qemu-20230327/linux-user/syscall_defs.h.new 2023-03-27 21:43:25.615115126 +0200
> @@ -1450,7 +1450,7 @@ struct target_stat {
> unsigned int st_dev;
> abi_ulong st_ino;
> unsigned int st_mode;
> - unsigned int st_nlink;
> + short int st_nlink;
> unsigned int st_uid;
> unsigned int st_gid;
> unsigned int st_rdev;
> @@ -1465,8 +1465,7 @@ struct target_stat {
>
> #define TARGET_HAS_STRUCT_STAT64
> struct target_stat64 {
> - unsigned char __pad0[6];
> - unsigned short st_dev;
> + uint64_t st_dev;
All use of the normal C types is wrong for target structs.
You must use abi_{short,int,long} etc.
Otherwise, you may not get the alignment you're expecting.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-30 11:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-28 11:48 stat64 wrong on sparc64 user Luca Bonissi
2023-03-28 11:55 ` Thomas Huth
2023-03-28 12:22 ` Luca Bonissi
2023-03-28 12:41 ` Thomas Huth
2023-03-29 16:22 ` Laurent Vivier
2023-03-30 11:13 ` Luca Bonissi
2023-03-28 14:52 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).