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];
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~
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
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
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
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)
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
© 2016 - 2024 Red Hat, Inc.