Re: [PATCH v3] Define in_int32_t_range to check if the 64 bit time_t syscall should be used

[Rich Felker <dalias@libc.org>]

On Tue, Nov 08, 2022 at 04:47:06PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 08/11/22 11:27, Adhemerval Zanella Netto wrote:
> > 
> > 
> > On 08/11/22 11:07, Arnd Bergmann wrote:
> >> On Tue, Nov 8, 2022, at 14:49, Adhemerval Zanella Netto wrote:
> >>> On 08/11/22 10:27, Arnd Bergmann wrote:
> >>>
> >>> Yes, the 32 bit fallback assumes that you either use the default minimum
> >>> kernel or configure with --enable-kernel with a value lower than 5.1.
> >>> And the optimization such as ecf2661281c was added on the basis that for
> >>> such configuration the 32 time_t is always present.  
> >>>
> >>> For __ASSUME_TIME64_SYSCALLS (default fro 64 bit time_t ABI and for 32
> >>> bit time_t with --enable-kernel=5.1) the 32 bit syscall should not be
> >>> issued.
> >>
> >> Ok, good. What is the amount of testing that this combination
> >> (--enable-kernel=5.1, CONFIG_COMPAT_32_BIT_TIME=n) has seen? Is
> >> this something that is already part of some regression test setup,
> >> or rather something that is supposed to work but isn't in
> >> widespread use?
> > 
> > I would say not much I am aware off, I usually check only against stock 
> > kernels from usual distributions (ubuntu, fedora, etc) and our patchwork 
> > instance also uses default kernels.
> > 
> >>
> >>> There are still the issue for a default configured glibc when running
> >>> on kernels with CONFIG_COMPAT_32BIT_TIME=y, this would require to remove
> >>> the fallback optimizations for !__ASSUME_TIME64_SYSCALLS.
> >>
> >> You mean CONFIG_COMPAT_32BIT_TIME=n with !__ASSUME_TIME64_SYSCALLS,
> >> right? As I said, I'm not too worried about this corner case, as
> >> long as there is some kind of use feedback that tells users to
> >> change either glibc or kernel configuration.
> >>
> >> My impression is that the optimizing for running on old kernels
> >> is not overly helpful, and that changing the fallback logic
> >> would be better, but this does not feel like a correctness
> >> issue when general-purpose distros always enable
> >> CONFIG_COMPAT_32BIT_TIME, and embedded users that want it
> >> disabled always build a matching glibc as well.
> > 
> > I added the optimization mainly because to disable the use of 32 bit
> > time_t support requires a configure switch.  It is worries me a bit
> > the combination of CONFIG_COMPAT_32BIT_TIME=n with 
> > !__ASSUME_TIME64_SYSCALLS as potential source of obscures bugs, but
> > I think that it would be unlikely in future when newer kernels will
> > be more present.
> 
> I am ccing Rich because he agrees that CONFIG_COMPAT_32BIT_TIME is a 
> userspace breakage and it will most likely break musl tim64 support.
> He suggested to move this option behind EXPERT [1] and fix the 
> documentation.

It's absolutely a violation of the "don't break userspace" syscall
API/ABI stability policy, and *does* break musl. We generally do not
use "new" syscalls except when the "old" syscall lacks the
functionality needed.

Rich