Bug 32119 - struct stat has different size depending on _POSIX_C_SOURCE
Summary: struct stat has different size depending on _POSIX_C_SOURCE
Status: UNCONFIRMED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.39
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-08-27 10:38 UTC by Benjamin Grossschartner
Modified: 2024-09-13 11:30 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Grossschartner 2024-08-27 10:38:20 UTC
On 32bit architectures with 64bit time enabled, struct stat misses the "__glibc_reserved" fields, if a POSIX version < 2008 is used.

As these reserved fields are also part of the struct used by the linux kernel, it seems that this is a bug in the glibc headers.

The bug was introduced by:
4e8521333bea6e89fcef1020e59a5f799241c5d4

The last "#endif" in the file "sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h" was missplaced.
In all other definitions of "struct stat" this "#ifdef __USE_XOPEN2K8" only controls the handling of the time structs, but not these reserved fields.


Suggested fix:

diff --git a/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h b/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
index 9ded57aa..124d0b59 100644                                                 
--- a/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h                  
+++ b/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h                  
@@ -58,9 +58,8 @@                                                               
   __fieldts64 (st_atime);                                                      
   __fieldts64 (st_mtime);                                                      
   __fieldts64 (st_ctime);                                                      
+# undef __fieldts64                                                            
+#endif                                                                         
                                                                                
   unsigned long int __glibc_reserved4;                                         
   unsigned long int __glibc_reserved5;                                         
-                                                                               
-# undef __fieldts64                                                            
-#endif
Comment 1 Sam James 2024-08-27 12:52:49 UTC
commit 4e8521333bea6e89fcef1020e59a5f799241c5d4
Author: Lukasz Majewski <lukma@denx.de>
Date:   Thu Oct 15 09:30:59 2020 +0200

    y2038: Use a common definition for stat

    Instead of replicate the same definitions from struct_stat_time64.h
    on the multiple struct_stat.h, use a common header which is included
    when required (struct_stat_time64_helper.h).  The 64-bit time support
    is added only for LFS support.

    The __USE_TIME_BITS64 is not defined internally yet, although the
    internal header is used when building the 64-bit stat implementations.

    Reviewed-by: Carlos O'Donell <carlos@redhat.com>
    Tested-by: Carlos O'Donell <carlos@redhat.com>
Comment 2 Adhemerval Zanella 2024-09-03 19:39:22 UTC
(In reply to Benjamin Grossschartner from comment #0)
> On 32bit architectures with 64bit time enabled, struct stat misses the
> "__glibc_reserved" fields, if a POSIX version < 2008 is used.
> 
> As these reserved fields are also part of the struct used by the linux
> kernel, it seems that this is a bug in the glibc headers.
> 
> The bug was introduced by:
> 4e8521333bea6e89fcef1020e59a5f799241c5d4
> 
> The last "#endif" in the file
> "sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h" was missplaced.
> In all other definitions of "struct stat" this "#ifdef __USE_XOPEN2K8" only
> controls the handling of the time structs, but not these reserved fields.
> 
> 
> Suggested fix:
> 
> diff --git a/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
> b/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
> index 9ded57aa..124d0b59 100644                                             
> 
> --- a/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h              
> 
> +++ b/sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h              
> 
> @@ -58,9 +58,8 @@                                                           
> 
>    __fieldts64 (st_atime);                                                  
> 
>    __fieldts64 (st_mtime);                                                  
> 
>    __fieldts64 (st_ctime);                                                  
> 
> +# undef __fieldts64                                                        
> 
> +#endif                                                                     
> 
>                                                                             
> 
>    unsigned long int __glibc_reserved4;                                     
> 
>    unsigned long int __glibc_reserved5;                                     
> 
> -                                                                           
> 
> -# undef __fieldts64                                                        
> 
> -#endif

I am not sure this is an issue, for the ABIs that would eventually use 'fstatat64_time64_stat' the __fstatat64_time64 (which would be called by stat/lstat/fstat on both non-LFS and LFS mode) it would first try statx (fstatat64_time64_statx) and then fallback to fstatat64 (fstatat64_time64_stat).

For both cases, the syscall will be issued with a temporary buffer and then copies to the caller-provided buffer.

It is still an issue for TU built with different flags, but we already have this issue for non-LFS and LFS; so I don't see an issue that __USE_XOPEN2K8 also generates incompatbile objects.
Comment 3 Andreas Schwab 2024-09-11 13:57:18 UTC
It is not ok to have a different layout and size depending on the value of _POSIX_C_SOURCE.  _FILE_OFFSET_BITS is an ABI-changing option, but selecting the standard level is not.
Comment 4 Florian Weimer 2024-09-11 17:06:48 UTC
(In reply to Andreas Schwab from comment #3)
> It is not ok to have a different layout and size depending on the value of
> _POSIX_C_SOURCE.  _FILE_OFFSET_BITS is an ABI-changing option, but selecting
> the standard level is not.

Thanks, Andreas. I meant to write the same thing.
Comment 5 Adhemerval Zanella 2024-09-12 19:50:56 UTC
(In reply to Andreas Schwab from comment #3)
> It is not ok to have a different layout and size depending on the value of
> _POSIX_C_SOURCE.  _FILE_OFFSET_BITS is an ABI-changing option, but selecting
> the standard level is not.

Right, but changing this would be technically an ABI break. I started to check this issue and besides all old 32 abi, it seems that some newer 32 also have similar problems due to how timespec is defined (the __timespec64 has padding, where the 'struct stat' does not).  

So it would require a lot of compact symbols, should we just paper over this issue and fix it on all releases?
Comment 6 Carlos O'Donell 2024-09-13 11:30:09 UTC
(In reply to Adhemerval Zanella from comment #5)
 > So it would require a lot of compact symbols, should we just paper over this
> issue and fix it on all releases?

That would be my inclination. I don't think these configurations are yet widely deployed. We should ask Gentoo though.