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 13:45, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> 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.
> 
> Is there any architecture where we have an fxstat implementation and the
> struct stat layout is different from what fstat expects?  To achieve a
> consistent system call profile, xfstat for the last version should call
> the fstat function, and not use a different inline system call.

This is a spreadsheet with various supported architectures for the xstat
implementation prior the consolidation and the requirements for each
syscall and/or transformation required [1].

It is quite a mess and that's why I think making only fstatat64 the one
with the syscall logic does simplify a lot.

> 
>> 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.
> 
> Sure, but the problem here is that glibc is not consistent in what
> kernel interfaces are used to implement fstat.

But it does not need to be in fact. This syscall bound semantic keeps
coming as an issue and I agree that it is a potential issue when the
replacement of fallback does not provide the full semantic. But in
this case __NR_newfstatat/__NR_fstatat64/__NR_statx/__NR_fstatat64
is really a superset of all defined stat functions.

> 
> Thanks,
> Florian
> 

[1] https://docs.google.com/spreadsheets/d/e/2PACX-1vRG3-1W8roNgHXVaY8zqIBlGqCRtztdyDUvS_VLLkCnmXANYTmZZvONda6TAKcNSNsqfswMn1_IadE5/pubhtml