Re: Should legacy fstat (via __fxstat) and fstat@GLIBC_2.33 call the same syscall?

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 14/01/2021 11:38, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> On 14/01/2021 09:00, Florian Weimer via Libc-alpha wrote:
>>> Currently, __fxstat may get mapped to the fstat system call, while fstat
>>> goes to fstatat.  This makes sandboxing issues less obvious to debug:
>>>
>>>   <https://bugs.chromium.org/p/chromium/issues/detail?id=1164975>
>>>
>>> Should we change this before the release?  And if yes, in what
>>> direction?
>>
>> Implementing fstat/lstat/stat through fstatat does really simplify
>> the code at *lot* for some architectures. And on others and on some
>> ABI (32-bit with 64-bit time_t), it would require to use the same
>> syscall anyway (statx).
>>
>> I don't really see any gain in adding back this complexity back on
>> stat calls, specially with y2038 support requirement.  
> 
> But we currently have this code for the x*stat interfaces.  For example,
> in sysdeps/unix/sysv/linux/xstat.c:
> 
> __xstat (int vers, const char *name, struct stat *buf)
> {
>   switch (vers)
>     {
>     case _STAT_VER_KERNEL:
>       {
> # if STAT_IS_KERNEL_STAT
>         /* New kABIs which uses generic pre 64-bit time Linux ABI,
>            e.g. csky, nios2  */
>         int r = INLINE_SYSCALL_CALL (fstatat64, AT_FDCWD, name, buf, 0);
>         return r ?: stat_overflow (buf);
> # else
>         /* Old kABIs with old non-LFS support, e.g. arm, i386, hppa, m68k,
>            microblaze, s390, sh, powerpc, and sparc32.  */
>         return INLINE_SYSCALL_CALL (stat, name, buf);
> # endif
>       }
> 
>     default:
>       {
> # if STAT_IS_KERNEL_STAT
>         return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
> # else
>         struct stat64 buf64;
>         int r = INLINE_SYSCALL_CALL (stat64, name, &buf64);
>         return r ?: __xstat32_conv (vers, &buf64, buf);
> #endif
>       }
>     }
> }

For instance for !XSTAT_IS_XSTAT64 (xstat.c) we have that _STAT_VER is
set for _STAT_VER_LINUX on arm and _STAT_VER_KERNEL on csky.

It means that we will need to do something like:

  #if STAT_IS_KERNEL_STAT
  /* csky, nios2  */
  int r = INLINE_SYSCALL_CALL (fstatat64, AT_FDCWD, name, buf, 0);
  return r ?: stat_overflow (buf);
  #else
  /* Old kABIs with old non-LFS support, e.g. arm, i386, hppa, m68k,
     microblaze, s390, sh, powerpc, and sparc32.  */
  struct stat64 buf64;
  int r = INLINE_SYSCALL_CALL (stat64, name, &buf64);
  return r ?: __xstat32_conv (vers, &buf64, buf);
  #endif

And it would have something similar to other implementation. I don't
think this is really an improvement here.

The main issue is this kind of sandboxing implemented by chromium and
other projects are not really future-proof and constrains the libc 
implementation to keep using the old syscalls where the kernel contract
allows to use a more broad one.  In fact, this is *exactly* what kernel
intends to provide a more generic interface instead of multiple ones
to implement the POSIX interfaces.