[Bug libc/29730] New: broken y2038 support in fstatat on MIPS N64

[Bug libc/29730] New: broken y2038 support in fstatat on MIPS N64

[aurelien at aurel32 dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

            Bug ID: 29730
           Summary: broken y2038 support in fstatat on MIPS N64
           Product: glibc
           Version: 2.35
            Status: NEW
          Severity: normal
          Priority: P1
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: aurelien at aurel32 dot net
                CC: adhemerval.zanella at linaro dot org, drepper.fsp at gmail dot com
  Target Milestone: ---
              Host: mips64el-unknown-linux-gnuabi64
            Target: mips64el-unknown-linux-gnuabi64
             Build: mips64el-unknown-linux-gnuabi64

On MIPS N64 the fstat/fstatat/lstat functions return EOVERFLOW when they are
called on files with a mtime, atime or ctime that can't be represented within a 
32-bit time_t. This should not happen as time_t is 64-bit on mips64el. This can
be reproduce for instance with:

$ touch -d 20390101 t
$ chmod +x t
chmod: cannot access 't': Value too large for defined data type
$ 

This issue has been introduced in glibc 2.35 by the following commit:

commit 6e8a0aac2f883a23efb1683b120499138f9e6021
Author: Stafford Horne <shorne@gmail.com>
Date:   Mon Jun 7 22:10:19 2021 +0900

    time: Fix overflow itimer tests on 32-bit systems

    On the port of OpenRISC I am working on and it appears the rv32 port
    we have sets __TIMESIZE == 64 && __WORDSIZE == 32.  This causes the
    size of time_t to be 8 bytes, but the tv_sec in the kernel is still 32-bit
    causing truncation.

    The truncations are unavoidable on these systems so skip the
    testing/failures by guarding with __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64.

    Also, futher in the tests and in other parts of code checking for time_t
    overflow does not work on 32-bit systems when time_t is 64-bit.  As
    suggested by Adhemerval, update the in_time_t_range function to assume
    32-bits by using int32_t.

    This also brings in the header for stdint.h so we can update other
    usages of __int32_t to int32_t as suggested by Adhemerval.

    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

MIPS N64 uses a 64-bit time_t, however it does not define XSTAT_IS_XSTAT64.
Therefore fstatat is different than fstatat64, and uses the default
sysdeps/unix/sysv/linux/fstatat.c implementation, which uses in_time_t_range()
with a 64-bit time_t.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

YunQiang Su <syq at debian dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |syq at debian dot org

--- Comment #1 from YunQiang Su <syq at debian dot org> ---
the function name is 
    in_time_t_range
while, in its code, it is
    in_int32_t_range

anyway it is so confused.


Why this patch is approved?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #2 from YunQiang Su <syq at debian dot org> ---
I think that we should revert this patch, and make OpenRISC and RV32
to use statx just MIPS N64 did.

In fact, on kernel_stat on MIPS N64, the time in kernel_stat is uint32_t.

And we switch to use statx instead of fstat* syscall.

I think that OpenRISC and RV32 should do the same thing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #3 from YunQiang Su <syq at debian dot org> ---
Can you help to try on OpenRISC and RV32:


1. revert 6e8a0aac2f883a23efb1683b120499138f9e6021
2. define STAT_HAS_TIME32
3. define XSTAT_IS_XSTAT64 0

I think that it should works fine.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[aurelien at aurel32 dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #4 from Aurelien Jarno <aurelien at aurel32 dot net> ---
(In reply to YunQiang Su from comment #3)
> Can you help to try on OpenRISC and RV32:
> 
> 
> 1. revert 6e8a0aac2f883a23efb1683b120499138f9e6021
> 2. define STAT_HAS_TIME32
> 3. define XSTAT_IS_XSTAT64 0
> 
> I think that it should works fine.

I don't think it is that easy. You should look at the original thread:
https://sourceware.org/pipermail/libc-alpha/2021-July/128612.html

The problem for OpenRISC is not stat, but rather usage of in_time_t_range() in
many other places like in setitimer().

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #5 from YunQiang Su <syq at debian dot org> ---
Ohh, you are right.

For __TIMESIZE == 64 && __WORDSIZE == 32 systems, 
I think that the kernel should provide a setitimer64 syscall.

Currently, for glibc, maybe that we should define a new macro for
these systems:
   #define KERNEL_SETTIMER_IS_32_BIT

and in setitimer.c



#if KERNEL_SETTIMER_IS_32_BIT
  if (! in_int32_t_range (new_value->it_interval.tv_sec)
      || ! in_int32_t_range (new_value->it_value.tv_sec))
#else
  if (! in_time_t_range (new_value->it_interval.tv_sec)
      || ! in_time_t_range (new_value->it_value.tv_sec))
#endif
    {
      __set_errno (EOVERFLOW);
      return -1;
    }


Anyway, we shouldn't confuse the function name.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #6 from YunQiang Su <syq at debian dot org> ---
It seems that the altering of in_time_t_range to in fact int32_t,  also breaks
something like 
    __clock_adjtime
on all 64bit system.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #7 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
So MIPS ABI idiocrasies strike again.  What about:

diff --git a/include/time.h b/include/time.h
index 20abea69d4..d3aff01aaa 100644
--- a/include/time.h
+++ b/include/time.h
@@ -2,6 +2,7 @@
 #include <time/time.h>

 #ifndef _ISOMAC
+# include <time64-compat.h>
 # include <bits/types/struct_timeval.h>
 # include <struct___timespec64.h>
 # include <struct___timeval64.h>
@@ -344,12 +345,18 @@ libc_hidden_proto (__time64)
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE     3

-/* Check whether T fits in int32_t, assume all usages are for
-   sizeof(time_t) == 32.  */
+/* Used on 32 to 64 bit time_t routines to check whether T fits in the
+   default time_t.  Some ABIs might still have 64 bit time_t as default,
+   however with different layout for input/output arguments (for instance
+   fstatat/fstatat64 for mips64).  */
 static inline bool
 in_time_t_range (__time64_t t)
 {
+#ifdef TIME64_NON_DEFAULT
   int32_t s = t;
+#else
+  time_t s = t;
+#endif
   return s == t;
 }

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #8 from YunQiang Su <syq at debian dot org> ---
Yes. It works.
But I don't think that it is a good idea, as the function name is too confused.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[aurelien at aurel32 dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #9 from Aurelien Jarno <aurelien at aurel32 dot net> ---
(In reply to YunQiang Su from comment #6)
> It seems that the altering of in_time_t_range to in fact int32_t,  also
> breaks something like 
>     __clock_adjtime
> on all 64bit system.

Do you have more details about that? Do you mean all 64-bit systems including
MIPS and non-MIPS? In that case the patch from Adhemerval would not fix
everything.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #10 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Aurelien Jarno from comment #9)
> (In reply to YunQiang Su from comment #6)
> > It seems that the altering of in_time_t_range to in fact int32_t,  also
> > breaks something like 
> >     __clock_adjtime
> > on all 64bit system.
> 
> Do you have more details about that? Do you mean all 64-bit systems
> including MIPS and non-MIPS? In that case the patch from Adhemerval would
> not fix everything.

The stat on MIPS is an outlier (as everything for mips it seems) where the
userland non-LFS struct stat defers from LFS struct stat.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #11 from YunQiang Su <syq at debian dot org> ---
(In reply to Adhemerval Zanella from comment #10)
> (In reply to Aurelien Jarno from comment #9)
> > (In reply to YunQiang Su from comment #6)
> > > It seems that the altering of in_time_t_range to in fact int32_t,  also
> > > breaks something like 
> > >     __clock_adjtime
> > > on all 64bit system.
> > 
> > Do you have more details about that? Do you mean all 64-bit systems
> > including MIPS and non-MIPS? In that case the patch from Adhemerval would
> > not fix everything.
> 
> The stat on MIPS is an outlier (as everything for mips it seems) where the
> userland non-LFS struct stat defers from LFS struct stat.

Yes. It is quite annoying.
I am try to switch mips N64 to XSTAT_IS_XSTAT64.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #12 from YunQiang Su <syq at debian dot org> ---
(In reply to YunQiang Su from comment #11)
> (In reply to Adhemerval Zanella from comment #10)
> > (In reply to Aurelien Jarno from comment #9)
> > > (In reply to YunQiang Su from comment #6)
> > > > It seems that the altering of in_time_t_range to in fact int32_t,  also
> > > > breaks something like 
> > > >     __clock_adjtime
> > > > on all 64bit system.
> > > 
> > > Do you have more details about that? Do you mean all 64-bit systems
> > > including MIPS and non-MIPS? In that case the patch from Adhemerval would
> > > not fix everything.
> > 
> > The stat on MIPS is an outlier (as everything for mips it seems) where the
> > userland non-LFS struct stat defers from LFS struct stat.
> 
> Yes. It is quite annoying.
> I am try to switch mips N64 to XSTAT_IS_XSTAT64.

Ohh, it seems impossible.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #13 from YunQiang Su <syq at debian dot org> ---
(In reply to Adhemerval Zanella from comment #7)
> So MIPS ABI idiocrasies strike again.  What about:
> 
> diff --git a/include/time.h b/include/time.h
> index 20abea69d4..d3aff01aaa 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -2,6 +2,7 @@
>  #include <time/time.h>
> 
>  #ifndef _ISOMAC
> +# include <time64-compat.h>
>  # include <bits/types/struct_timeval.h>
>  # include <struct___timespec64.h>
>  # include <struct___timeval64.h>
> @@ -344,12 +345,18 @@ libc_hidden_proto (__time64)
>     actual clock ID.  */
>  #define CLOCK_IDFIELD_SIZE     3
> 
> -/* Check whether T fits in int32_t, assume all usages are for
> -   sizeof(time_t) == 32.  */
> +/* Used on 32 to 64 bit time_t routines to check whether T fits in the
> +   default time_t.  Some ABIs might still have 64 bit time_t as default,
> +   however with different layout for input/output arguments (for instance
> +   fstatat/fstatat64 for mips64).  */
>  static inline bool
>  in_time_t_range (__time64_t t)
>  {
> +#ifdef TIME64_NON_DEFAULT
>    int32_t s = t;
> +#else
> +  time_t s = t;
> +#endif
>    return s == t;
>  }

sysdeps/generic/time64-compat.h says:

/* Header included by Versions to generate the 64-bit time_t compat symbols.
   Legacy ABIs with default 32-bit time support define TIME64_NON_DEFAULT to
   generate the 64-bit symbols.  */

So, I don't think that rv32 and OpenRISC should define TIME64_NON_DEFAULT.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #14 from YunQiang Su <syq at debian dot org> ---
Created attachment 14422
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14422&action=edit
define in_int32_t_range

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #15 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to YunQiang Su from comment #12)
> (In reply to YunQiang Su from comment #11)
> > (In reply to Adhemerval Zanella from comment #10)
> > > (In reply to Aurelien Jarno from comment #9)
> > > > (In reply to YunQiang Su from comment #6)
> > > > > It seems that the altering of in_time_t_range to in fact int32_t,  also
> > > > > breaks something like 
> > > > >     __clock_adjtime
> > > > > on all 64bit system.
> > > > 
> > > > Do you have more details about that? Do you mean all 64-bit systems
> > > > including MIPS and non-MIPS? In that case the patch from Adhemerval would
> > > > not fix everything.
> > > 
> > > The stat on MIPS is an outlier (as everything for mips it seems) where the
> > > userland non-LFS struct stat defers from LFS struct stat.
> > 
> > Yes. It is quite annoying.
> > I am try to switch mips N64 to XSTAT_IS_XSTAT64.
> 
> Ohh, it seems impossible.

The XSTAT_IS_XSTAT64 is used by MIPS to advertise the kernel struct fstat have
different layouts from userland (an unfortunate MIPS ABI quick) as documented
briefly on sysdeps/unix/sysv/linux/mips/kernel_stat.h.

At least the LFS and non-LFS version have the same semantic, so we do not
actually need to provide different implementations for mips64. Maybe it would
be better to provide another macro, STAT_IS_STAT64, to make mips alias non-LFS
and LFS.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #16 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to YunQiang Su from comment #13)
> (In reply to Adhemerval Zanella from comment #7)
> > So MIPS ABI idiocrasies strike again.  What about:
> > 
> > diff --git a/include/time.h b/include/time.h
> > index 20abea69d4..d3aff01aaa 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -2,6 +2,7 @@
> >  #include <time/time.h>
> > 
> >  #ifndef _ISOMAC
> > +# include <time64-compat.h>
> >  # include <bits/types/struct_timeval.h>
> >  # include <struct___timespec64.h>
> >  # include <struct___timeval64.h>
> > @@ -344,12 +345,18 @@ libc_hidden_proto (__time64)
> >     actual clock ID.  */
> >  #define CLOCK_IDFIELD_SIZE     3
> > 
> > -/* Check whether T fits in int32_t, assume all usages are for
> > -   sizeof(time_t) == 32.  */
> > +/* Used on 32 to 64 bit time_t routines to check whether T fits in the
> > +   default time_t.  Some ABIs might still have 64 bit time_t as default,
> > +   however with different layout for input/output arguments (for instance
> > +   fstatat/fstatat64 for mips64).  */
> >  static inline bool
> >  in_time_t_range (__time64_t t)
> >  {
> > +#ifdef TIME64_NON_DEFAULT
> >    int32_t s = t;
> > +#else
> > +  time_t s = t;
> > +#endif
> >    return s == t;
> >  }
> 
> sysdeps/generic/time64-compat.h says:
> 
> /* Header included by Versions to generate the 64-bit time_t compat symbols.
>    Legacy ABIs with default 32-bit time support define TIME64_NON_DEFAULT to
>    generate the 64-bit symbols.  */
> 
> So, I don't think that rv32 and OpenRISC should define TIME64_NON_DEFAULT.

They do not, both uses 64 bit time_t as default.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #17 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to YunQiang Su from comment #14)
> Created attachment 14422 [details]
> define in_int32_t_range

Besides the stat issue (which I just send a fix for it [1]), is there another
case where in_time_t_range is being used by an ABI with 64-bit time that will
trigger and invalid usage?

I am kind worried to change such code again, since it will require even more
validation that this does not subtle break anymore more.

[1] https://sourceware.org/pipermail/libc-alpha/2022-October/143089.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #18 from YunQiang Su <syq at debian dot org> ---
(In reply to Adhemerval Zanella from comment #15)
> (In reply to YunQiang Su from comment #12)
> > (In reply to YunQiang Su from comment #11)
> > > (In reply to Adhemerval Zanella from comment #10)
> > > > (In reply to Aurelien Jarno from comment #9)
> > > > > (In reply to YunQiang Su from comment #6)
> > > > > > It seems that the altering of in_time_t_range to in fact int32_t,  also
> > > > > > breaks something like 
> > > > > >     __clock_adjtime
> > > > > > on all 64bit system.
> > > > > 
> > > > > Do you have more details about that? Do you mean all 64-bit systems
> > > > > including MIPS and non-MIPS? In that case the patch from Adhemerval would
> > > > > not fix everything.
> > > > 
> > > > The stat on MIPS is an outlier (as everything for mips it seems) where the
> > > > userland non-LFS struct stat defers from LFS struct stat.
> > > 
> > > Yes. It is quite annoying.
> > > I am try to switch mips N64 to XSTAT_IS_XSTAT64.
> > 
> > Ohh, it seems impossible.
> 
> The XSTAT_IS_XSTAT64 is used by MIPS to advertise the kernel struct fstat
> have different layouts from userland (an unfortunate MIPS ABI quick) as
> documented briefly on sysdeps/unix/sysv/linux/mips/kernel_stat.h.
> 

The reason that MIPS N64 cannot use XSTAT_IS_XSTAT64 is due to the difference
between
   struct stat
   struct stat64
Although both of them support large file and 64bit time, while they have
different padding. Brain damaged design.


> At least the LFS and non-LFS version have the same semantic, so we do not
> actually need to provide different implementations for mips64. Maybe it
> would be better to provide another macro, STAT_IS_STAT64, to make mips alias
> non-LFS and LFS.

Oh, no, we cannot define STAT_IS_STAT64 for MIPS N64.
Since it will broken binary compatible.

   struct stat
   struct stat64

is different!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #19 from YunQiang Su <syq at debian dot org> ---
(In reply to Adhemerval Zanella from comment #17)
> (In reply to YunQiang Su from comment #14)
> > Created attachment 14422 [details]
> > define in_int32_t_range
> 
> Besides the stat issue (which I just send a fix for it [1]), is there
> another case where in_time_t_range is being used by an ABI with 64-bit time
> that will trigger and invalid usage?
> 
> I am kind worried to change such code again, since it will require even more
> validation that this does not subtle break anymore more.
> 
> [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143089.html

Always Keep the function name sync with what it is doing.

We should use in_int32_t_range to determin the 32bit syscall or 64bit one
to use, in the bellow files:
          sysdeps/unix/sysv/linux/clock_adjtime.c
          sysdeps/unix/sysv/linux/timer_settime.c:
          sysdeps/unix/sysv/linux/clock_nanosleep.c
          sysdeps/unix/sysv/linux/clock_settime.c
          sysdeps/unix/sysv/linux/pselect.c
          sysdeps/unix/sysv/linux/mq_timedreceive.c
          sysdeps/unix/sysv/linux/semtimedop.c
          sysdeps/unix/sysv/linux/mq_timedsend.c
          sysdeps/unix/sysv/linux/ppoll.c
          sysdeps/unix/sysv/linux/setitimer.c
          sysdeps/unix/sysv/linux/utimensat.c
          sysdeps/unix/sysv/linux/recvmmsg.c
          sysdeps/unix/sysv/linux/timerfd_settime.c
          sysdeps/unix/sysv/linux/setsockopt.c
          sysdeps/unix/sysv/linux/sigtimedwait.c
          sysdeps/unix/sysv/linux/select.c
          nptl/futex-internal.c

We should use in_time_t_range to detect overflow from the syscall retval:
          time/timegm.c
          time/mktime.c
          sysdeps/unix/sysv/linux/ftime.c
          sysdeps/unix/sysv/linux/sched_rr_gi.c
          sysdeps/unix/sysv/linux/stat_t64_cp.c
          sysdeps/unix/sysv/linux/time.c
          sysdeps/unix/sysv/linux/timespec_get.c
          sysdeps/unix/sysv/linux/gettimeofday.c
          sysdeps/unix/sysv/linux/clock_gettime.c
          sysdeps/unix/sysv/linux/fstatat.c

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #20 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to YunQiang Su from comment #18)
> (In reply to Adhemerval Zanella from comment #15)
> > (In reply to YunQiang Su from comment #12)
> > > (In reply to YunQiang Su from comment #11)
> > > > (In reply to Adhemerval Zanella from comment #10)
> > > > > (In reply to Aurelien Jarno from comment #9)
> > > > > > (In reply to YunQiang Su from comment #6)
> > > > > > > It seems that the altering of in_time_t_range to in fact int32_t,  also
> > > > > > > breaks something like 
> > > > > > >     __clock_adjtime
> > > > > > > on all 64bit system.
> > > > > > 
> > > > > > Do you have more details about that? Do you mean all 64-bit systems
> > > > > > including MIPS and non-MIPS? In that case the patch from Adhemerval would
> > > > > > not fix everything.
> > > > > 
> > > > > The stat on MIPS is an outlier (as everything for mips it seems) where the
> > > > > userland non-LFS struct stat defers from LFS struct stat.
> > > > 
> > > > Yes. It is quite annoying.
> > > > I am try to switch mips N64 to XSTAT_IS_XSTAT64.
> > > 
> > > Ohh, it seems impossible.
> > 
> > The XSTAT_IS_XSTAT64 is used by MIPS to advertise the kernel struct fstat
> > have different layouts from userland (an unfortunate MIPS ABI quick) as
> > documented briefly on sysdeps/unix/sysv/linux/mips/kernel_stat.h.
> > 
> 
> The reason that MIPS N64 cannot use XSTAT_IS_XSTAT64 is due to the
> difference between
>    struct stat
>    struct stat64
> Although both of them support large file and 64bit time, while they have
> different padding. Brain damaged design.
> 
> 
> > At least the LFS and non-LFS version have the same semantic, so we do not
> > actually need to provide different implementations for mips64. Maybe it
> > would be better to provide another macro, STAT_IS_STAT64, to make mips alias
> > non-LFS and LFS.
> 
> Oh, no, we cannot define STAT_IS_STAT64 for MIPS N64.
> Since it will broken binary compatible.
> 
>    struct stat
>    struct stat64
> 
> is different!

Yeah, I just figured out that st_size does have the same offset due ABI
alignment quicks...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #21 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to YunQiang Su from comment #19)
> (In reply to Adhemerval Zanella from comment #17)
> > (In reply to YunQiang Su from comment #14)
> > > Created attachment 14422 [details]
> > > define in_int32_t_range
> > 
> > Besides the stat issue (which I just send a fix for it [1]), is there
> > another case where in_time_t_range is being used by an ABI with 64-bit time
> > that will trigger and invalid usage?
> > 
> > I am kind worried to change such code again, since it will require even more
> > validation that this does not subtle break anymore more.
> > 
> > [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143089.html
> 
> Always Keep the function name sync with what it is doing.
> 
> We should use in_int32_t_range to determin the 32bit syscall or 64bit one
> to use, in the bellow files:
>           sysdeps/unix/sysv/linux/clock_adjtime.c
>           sysdeps/unix/sysv/linux/timer_settime.c:
>           sysdeps/unix/sysv/linux/clock_nanosleep.c
>           sysdeps/unix/sysv/linux/clock_settime.c
>           sysdeps/unix/sysv/linux/pselect.c
>           sysdeps/unix/sysv/linux/mq_timedreceive.c
>           sysdeps/unix/sysv/linux/semtimedop.c
>           sysdeps/unix/sysv/linux/mq_timedsend.c
>           sysdeps/unix/sysv/linux/ppoll.c
>           sysdeps/unix/sysv/linux/setitimer.c
>           sysdeps/unix/sysv/linux/utimensat.c
>           sysdeps/unix/sysv/linux/recvmmsg.c
>           sysdeps/unix/sysv/linux/timerfd_settime.c
>           sysdeps/unix/sysv/linux/setsockopt.c
>           sysdeps/unix/sysv/linux/sigtimedwait.c
>           sysdeps/unix/sysv/linux/select.c
>           nptl/futex-internal.c
> 
> We should use in_time_t_range to detect overflow from the syscall retval:
>           time/timegm.c
>           time/mktime.c
>           sysdeps/unix/sysv/linux/ftime.c
>           sysdeps/unix/sysv/linux/sched_rr_gi.c
>           sysdeps/unix/sysv/linux/stat_t64_cp.c
>           sysdeps/unix/sysv/linux/time.c
>           sysdeps/unix/sysv/linux/timespec_get.c
>           sysdeps/unix/sysv/linux/gettimeofday.c
>           sysdeps/unix/sysv/linux/clock_gettime.c
>           sysdeps/unix/sysv/linux/fstatat.c

I don't have a strong opinion, this is essentially a refactor.  Besides the
type change on timeval, the rest should be ok (although not strickly related to
this issue).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[aurelien at aurel32 dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #22 from Aurelien Jarno <aurelien at aurel32 dot net> ---
(In reply to YunQiang Su from comment #19)
> (In reply to Adhemerval Zanella from comment #17)
> > (In reply to YunQiang Su from comment #14)
> > > Created attachment 14422 [details]
> > > define in_int32_t_range
> > 
> > Besides the stat issue (which I just send a fix for it [1]), is there
> > another case where in_time_t_range is being used by an ABI with 64-bit time
> > that will trigger and invalid usage?
> > 
> > I am kind worried to change such code again, since it will require even more
> > validation that this does not subtle break anymore more.
> > 
> > [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143089.html
> 
> Always Keep the function name sync with what it is doing.
> 
> We should use in_int32_t_range to determin the 32bit syscall or 64bit one
> to use, in the bellow files:
>           sysdeps/unix/sysv/linux/clock_adjtime.c
>           sysdeps/unix/sysv/linux/timer_settime.c:
>           sysdeps/unix/sysv/linux/clock_nanosleep.c
>           sysdeps/unix/sysv/linux/clock_settime.c
>           sysdeps/unix/sysv/linux/pselect.c
>           sysdeps/unix/sysv/linux/mq_timedreceive.c
>           sysdeps/unix/sysv/linux/semtimedop.c
>           sysdeps/unix/sysv/linux/mq_timedsend.c
>           sysdeps/unix/sysv/linux/ppoll.c
>           sysdeps/unix/sysv/linux/setitimer.c
>           sysdeps/unix/sysv/linux/utimensat.c
>           sysdeps/unix/sysv/linux/recvmmsg.c
>           sysdeps/unix/sysv/linux/timerfd_settime.c
>           sysdeps/unix/sysv/linux/setsockopt.c
>           sysdeps/unix/sysv/linux/sigtimedwait.c
>           sysdeps/unix/sysv/linux/select.c
>           nptl/futex-internal.c
> 
> We should use in_time_t_range to detect overflow from the syscall retval:
>           time/timegm.c
>           time/mktime.c

For these above two your choice is correct, but it's not linked to syscall
retval.

>           sysdeps/unix/sysv/linux/ftime.c
>           sysdeps/unix/sysv/linux/sched_rr_gi.c
>           sysdeps/unix/sysv/linux/stat_t64_cp.c
>           sysdeps/unix/sysv/linux/time.c
>           sysdeps/unix/sysv/linux/timespec_get.c
>           sysdeps/unix/sysv/linux/gettimeofday.c
>           sysdeps/unix/sysv/linux/clock_gettime.c
>           sysdeps/unix/sysv/linux/fstatat.c

Additional comments:
- No need to change int32_t into __int32_t in __timeval32
- Renaming STAT_HAS_TIME32 into KERNEL_STAT64_HAS_TIME32 is correct, but is not
needed to fix the issue. I think it might go into a separate patch

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[aurelien at aurel32 dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #23 from Aurelien Jarno <aurelien at aurel32 dot net> ---
(In reply to Aurelien Jarno from comment #22)
> (In reply to YunQiang Su from comment #19)
> > (In reply to Adhemerval Zanella from comment #17)
> > > (In reply to YunQiang Su from comment #14)
> > > > Created attachment 14422 [details]
> > > > define in_int32_t_range
> > > 
> > > Besides the stat issue (which I just send a fix for it [1]), is there
> > > another case where in_time_t_range is being used by an ABI with 64-bit time
> > > that will trigger and invalid usage?
> > > 
> > > I am kind worried to change such code again, since it will require even more
> > > validation that this does not subtle break anymore more.
> > > 
> > > [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143089.html
> > 
> > Always Keep the function name sync with what it is doing.
> > 
> > We should use in_int32_t_range to determin the 32bit syscall or 64bit one
> > to use, in the bellow files:
> >           sysdeps/unix/sysv/linux/clock_adjtime.c
> >           sysdeps/unix/sysv/linux/timer_settime.c:
> >           sysdeps/unix/sysv/linux/clock_nanosleep.c
> >           sysdeps/unix/sysv/linux/clock_settime.c
> >           sysdeps/unix/sysv/linux/pselect.c
> >           sysdeps/unix/sysv/linux/mq_timedreceive.c
> >           sysdeps/unix/sysv/linux/semtimedop.c
> >           sysdeps/unix/sysv/linux/mq_timedsend.c
> >           sysdeps/unix/sysv/linux/ppoll.c
> >           sysdeps/unix/sysv/linux/setitimer.c
> >           sysdeps/unix/sysv/linux/utimensat.c
> >           sysdeps/unix/sysv/linux/recvmmsg.c
> >           sysdeps/unix/sysv/linux/timerfd_settime.c
> >           sysdeps/unix/sysv/linux/setsockopt.c
> >           sysdeps/unix/sysv/linux/sigtimedwait.c
> >           sysdeps/unix/sysv/linux/select.c
> >           nptl/futex-internal.c
> > 
> > We should use in_time_t_range to detect overflow from the syscall retval:
> >           time/timegm.c
> >           time/mktime.c
> 
> For these above two your choice is correct, but it's not linked to syscall
> retval.
> 
> >           sysdeps/unix/sysv/linux/ftime.c
> >           sysdeps/unix/sysv/linux/sched_rr_gi.c
> >           sysdeps/unix/sysv/linux/stat_t64_cp.c
> >           sysdeps/unix/sysv/linux/time.c
> >           sysdeps/unix/sysv/linux/timespec_get.c
> >           sysdeps/unix/sysv/linux/gettimeofday.c
> >           sysdeps/unix/sysv/linux/clock_gettime.c
> >           sysdeps/unix/sysv/linux/fstatat.c
> 
> Additional comments:
> - No need to change int32_t into __int32_t in __timeval32
> - Renaming STAT_HAS_TIME32 into KERNEL_STAT64_HAS_TIME32 is correct, but is
> not needed to fix the issue. I think it might go into a separate patch

Maybe you can post an updated patch to the mailing list for easier discussion?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[aurelien at aurel32 dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

Aurelien Jarno <aurelien at aurel32 dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |2.37
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #24 from Aurelien Jarno <aurelien at aurel32 dot net> ---
Fixed by this commit:

commit 7457b7eef8dfe8cc48e55b9f9837df6dd397b80d (HEAD -> master, origin/master,
origin/HEAD)
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Tue Nov 1 20:43:55 2022 +0100

    linux: Fix fstatat on MIPSn64 (BZ #29730)

    Commit 6e8a0aac2f883 ("time: Fix overflow itimer tests on 32-bit
    systems") changed in_time_t_range to assume a 32-bit time_t. This broke
    fstatat on MIPSn64 that was using it with a 64-bit time_t due to
    difference between stat and stat64. This commit fix that by adding a
    MIPSn64 specific version, which bypasses the EOVERFLOW tests.

    Resolves: BZ #29730

    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #25 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Aurelien Jarno <aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7457b7eef8dfe8cc48e55b9f9837df6dd397b80d

commit 7457b7eef8dfe8cc48e55b9f9837df6dd397b80d
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Tue Nov 1 20:43:55 2022 +0100

    linux: Fix fstatat on MIPSn64 (BZ #29730)

    Commit 6e8a0aac2f883 ("time: Fix overflow itimer tests on 32-bit
    systems") changed in_time_t_range to assume a 32-bit time_t. This broke
    fstatat on MIPSn64 that was using it with a 64-bit time_t due to
    difference between stat and stat64. This commit fix that by adding a
    MIPSn64 specific version, which bypasses the EOVERFLOW tests.

    Resolves: BZ #29730

    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #26 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The release/2.36/master branch has been updated by Aurelien Jarno
<aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=dd4131c8322891a0ad7cfb661efa41aecc02b581

commit dd4131c8322891a0ad7cfb661efa41aecc02b581
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Tue Nov 1 20:43:55 2022 +0100

    linux: Fix fstatat on MIPSn64 (BZ #29730)

    Commit 6e8a0aac2f883 ("time: Fix overflow itimer tests on 32-bit
    systems") changed in_time_t_range to assume a 32-bit time_t. This broke
    fstatat on MIPSn64 that was using it with a 64-bit time_t due to
    difference between stat and stat64. This commit fix that by adding a
    MIPSn64 specific version, which bypasses the EOVERFLOW tests.

    Resolves: BZ #29730

    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    (cherry picked from commit 7457b7eef8dfe8cc48e55b9f9837df6dd397b80d)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #27 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The release/2.35/master branch has been updated by Aurelien Jarno
<aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=cbfe89a675a37ac8e7d0e0051e084e090ad240bb

commit cbfe89a675a37ac8e7d0e0051e084e090ad240bb
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Tue Nov 1 20:43:55 2022 +0100

    linux: Fix fstatat on MIPSn64 (BZ #29730)

    Commit 6e8a0aac2f883 ("time: Fix overflow itimer tests on 32-bit
    systems") changed in_time_t_range to assume a 32-bit time_t. This broke
    fstatat on MIPSn64 that was using it with a 64-bit time_t due to
    difference between stat and stat64. This commit fix that by adding a
    MIPSn64 specific version, which bypasses the EOVERFLOW tests.

    Resolves: BZ #29730

    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    (cherry picked from commit 7457b7eef8dfe8cc48e55b9f9837df6dd397b80d)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29730] broken y2038 support in fstatat on MIPS N64

[syq at debian dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29730

--- Comment #28 from YunQiang Su <syq at debian dot org> ---
Just for reference:


https://sourceware.org/pipermail/libc-alpha/2022-November/143194.html
[PATCH v2] Use in_int32_t_range to check need 64bit syscall 



https://sourceware.org/pipermail/libc-alpha/2022-November/143195.html
[PATCH] Rename STAT_HAS_TIME32 to KERNEL_STAT64_HAS_TIME32

-- 
You are receiving this mail because:
You are on the CC list for the bug.