Re: [PATCH v5 1/5] linux: Use getdents64 on non-LFS readdir

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

On 20/02/23 14:55, Florian Weimer wrote:
> * Florian Weimer:
> 
>>> diff --git a/sysdeps/unix/sysv/linux/readdir.c b/sysdeps/unix/sysv/linux/readdir.c
>>> index 4a4c00ea07..cd0ccaf33a 100644
>>> --- a/sysdeps/unix/sysv/linux/readdir.c
>>> +++ b/sysdeps/unix/sysv/linux/readdir.c
>>> @@ -21,42 +21,71 @@
>>>  #if !_DIRENT_MATCHES_DIRENT64
>>>  #include <dirstream.h>
>>>  
>>> +/* Translate the DP64 entry to the non-LFS one in the translation entry
>>> +   at dirstream DS.  Return true is the translation was possible or
>>> +   false if either an internal field can not be represented in the non-LFS
>>> +   entry or if the name is too long.  */
>>> +static bool
>>> +dirstream_entry (struct __dirstream *ds, const struct dirent64 *dp64)
>>> +{
>>> +  /* Check for overflow.  */
>>> +  if (!in_off_t_range (dp64->d_off) || !in_ino_t_range (dp64->d_ino))
>>> +    return false;
>>> +
>>> +  /* And if name is too large.  */
>>> +  if (dp64->d_reclen - offsetof (struct dirent64, d_name) > NAME_MAX)
>>> +    return false;
>>
>> Sorry, I don't think this is the direction we should be going.  In
>> readdir_r, we at least delay the NAME_MAX error to the end of the
>> directory.  This just adds another rare case where 32-bit code fails and
>> 64-bit code works.
>>
>> struct dirent is always shorter than struct dirent64, right?  It should
>> be possible to do the translation in-place.  Or turn tdp into a pointer
>> and reallocate as needed.
>>
>> However, I think we should fix only readdir64, not readdir.  It's simply
>> not possible to fix readdir fully because of d_ino, so applications
>> which use readdir instead of readdir64 will remain buggy even with this
>> change.
> 
> Meh, during a walk it occurred tome that 64-bit d_ino is far less common
> than 64-bit d_off.  So we really need all this rewriting. 8-(
> 
> Could you do it in-place at least?  To address the d_name sizing issue?

I added the translation buffer only to simplify the copy logic, but it should
be doable to do it in place (similar to what getdents64 for mips64 does).