[v3,0/7] Fix getdents{64} regression on some FS

  This is updated version [1], rebased against master add with some
suggestion from previous review.  The main changes are:

  - The translation buffer is allocated as an auxiliarry one (instead
    of a reserved spacefrom the dirstream) and readdir might reallocate
    it if it finds a large entry.

  - Failures in non-LFS readdir (such as d_ino/d_off overflow or failure
    in expand the translation buffer) is ignored.  This allows code that
    does not handle possible failure in readdir to successful read the
    rest of entries in the directory.

  - So minor fixes in dirent/tst-seekdir2.c

  - Added __attribute_deprecated_msg__ on getdirentries to redirect to
    getdents64.

I have checked on x86_64-linux-gnu, powerpc64-linux-gnu and on some
32-bit ABIs arm-linux-gnueabihf, powerpc-linux-gnu, and sparcv9-linux-gnu.


[1] https://sourceware.org/pipermail/libc-alpha/2020-October/118274.html

Adhemerval Zanella (7):
  linux: Do not skip entries with zero d_ino values [BZ #12165]
  linux: Use getdents64 on non-LFS readdir
  linux: Set internal DIR filepos as off64_t [BZ #23960, BZ #24050]
  linux: Add __readdir64_unlocked
  linux: Add __old_readdir64_unlocked
  linux: Use getdents64 on readdir64 compat implementation
  dirent: Deprecate getdirentries

 NEWS                                  |   3 +
 dirent/Makefile                       |   2 +-
 dirent/dirent.h                       |  14 +-
 dirent/tst-seekdir2.c                 | 158 +++++++++++++++++++++
 include/dirent.h                      |   1 +
 sysdeps/unix/sysv/linux/Makefile      |   3 +
 sysdeps/unix/sysv/linux/closedir.c    |   8 ++
 sysdeps/unix/sysv/linux/dirstream.h   |  11 +-
 sysdeps/unix/sysv/linux/getdents64.c  |  93 ------------
 sysdeps/unix/sysv/linux/olddirent.h   |   4 +-
 sysdeps/unix/sysv/linux/opendir.c     |  37 +++++
 sysdeps/unix/sysv/linux/readdir.c     |  83 +++++++----
 sysdeps/unix/sysv/linux/readdir64.c   | 196 +++++++++++++++-----------
 sysdeps/unix/sysv/linux/readdir64_r.c | 159 +++++----------------
 sysdeps/unix/sysv/linux/rewinddir.c   |   5 +
 sysdeps/unix/sysv/linux/seekdir.c     |  36 ++++-
 sysdeps/unix/sysv/linux/telldir.c     |  47 +++++-
 sysdeps/unix/sysv/linux/telldir.h     |  64 +++++++++
 18 files changed, 583 insertions(+), 341 deletions(-)
 create mode 100644 dirent/tst-seekdir2.c
 create mode 100644 sysdeps/unix/sysv/linux/telldir.h
  

Hi!

On 10/21/20 4:15 PM, Adhemerval Zanella wrote:
> This is updated version [1], rebased against master add with some
> suggestion from previous review.  The main changes are:
> 
>   - The translation buffer is allocated as an auxiliarry one (instead
>     of a reserved spacefrom the dirstream) and readdir might reallocate
>     it if it finds a large entry.
> 
>   - Failures in non-LFS readdir (such as d_ino/d_off overflow or failure
>     in expand the translation buffer) is ignored.  This allows code that
>     does not handle possible failure in readdir to successful read the
>     rest of entries in the directory.
> 
>   - So minor fixes in dirent/tst-seekdir2.c
> 
>   - Added __attribute_deprecated_msg__ on getdirentries to redirect to
>     getdents64.
> 
> I have checked on x86_64-linux-gnu, powerpc64-linux-gnu and on some
> 32-bit ABIs arm-linux-gnueabihf, powerpc-linux-gnu, and sparcv9-linux-gnu.
Are there any news on this patch series? The bug is still present as far as I know.

Thanks,
Adrian
  

Hello!

On 10/21/20 4:15 PM, Adhemerval Zanella wrote:
> This is updated version [1], rebased against master add with some
> suggestion from previous review.  The main changes are:
> 
>   - The translation buffer is allocated as an auxiliarry one (instead
>     of a reserved spacefrom the dirstream) and readdir might reallocate
>     it if it finds a large entry.
> 
>   - Failures in non-LFS readdir (such as d_ino/d_off overflow or failure
>     in expand the translation buffer) is ignored.  This allows code that
>     does not handle possible failure in readdir to successful read the
>     rest of entries in the directory.
> 
>   - So minor fixes in dirent/tst-seekdir2.c
> 
>   - Added __attribute_deprecated_msg__ on getdirentries to redirect to
>     getdents64.
> 
> I have checked on x86_64-linux-gnu, powerpc64-linux-gnu and on some
> 32-bit ABIs arm-linux-gnueabihf, powerpc-linux-gnu, and sparcv9-linux-gnu.

Are there any news on this patch series besides the v3 version? [1]

Thanks,
Adrian

> [1] https://patchwork.ozlabs.org/project/glibc/list/?series=209257
  

On 19/08/2021 13:07, John Paul Adrian Glaubitz wrote:
> Hello!
> 
> On 10/21/20 4:15 PM, Adhemerval Zanella wrote:
>> This is updated version [1], rebased against master add with some
>> suggestion from previous review.  The main changes are:
>>
>>   - The translation buffer is allocated as an auxiliarry one (instead
>>     of a reserved spacefrom the dirstream) and readdir might reallocate
>>     it if it finds a large entry.
>>
>>   - Failures in non-LFS readdir (such as d_ino/d_off overflow or failure
>>     in expand the translation buffer) is ignored.  This allows code that
>>     does not handle possible failure in readdir to successful read the
>>     rest of entries in the directory.
>>
>>   - So minor fixes in dirent/tst-seekdir2.c
>>
>>   - Added __attribute_deprecated_msg__ on getdirentries to redirect to
>>     getdents64.
>>
>> I have checked on x86_64-linux-gnu, powerpc64-linux-gnu and on some
>> 32-bit ABIs arm-linux-gnueabihf, powerpc-linux-gnu, and sparcv9-linux-gnu.
> 
> Are there any news on this patch series besides the v3 version? [1]

Unfortunately it is requires reviews. Since it only affects legacy
ABIs there is no much traction for fixing it.

> 
> Thanks,
> Adrian
> 
>> [1] https://patchwork.ozlabs.org/project/glibc/list/?series=209257
>
  

Hi!

On 8/19/21 6:19 PM, Adhemerval Zanella wrote:
>> Are there any news on this patch series besides the v3 version? [1]
> 
> Unfortunately it is requires reviews. Since it only affects legacy
> ABIs there is no much traction for fixing it.

Would it help if I put a bounty on this bug report?

I still think this is an important fix as it breaks 32-bit qemu-user guests
on 64-bit systems and currently, I have to build the glibc package manually
for Debian whenever it's updated in the archives.

Adrian
  

On 19/08/2021 13:50, John Paul Adrian Glaubitz wrote:
> Hi!
> 
> On 8/19/21 6:19 PM, Adhemerval Zanella wrote:
>>> Are there any news on this patch series besides the v3 version? [1]
>>
>> Unfortunately it is requires reviews. Since it only affects legacy
>> ABIs there is no much traction for fixing it.
> 
> Would it help if I put a bounty on this bug report?

I am not sure in fact; the main problem is no one is really interested in
revise if my approach is the expected one and if the patch is what we want.

> 
> I still think this is an important fix as it breaks 32-bit qemu-user guests
> on 64-bit systems and currently, I have to build the glibc package manually
> for Debian whenever it's updated in the archives.
> 
> Adrian
>
  

On 8/19/21 6:52 PM, Adhemerval Zanella wrote:> On 19/08/2021 13:50, John Paul Adrian Glaubitz wrote:
>> Hi!
>>
>> On 8/19/21 6:19 PM, Adhemerval Zanella wrote:
>>>> Are there any news on this patch series besides the v3 version? [1]
>>>
>>> Unfortunately it is requires reviews. Since it only affects legacy
>>> ABIs there is no much traction for fixing it.
>>
>> Would it help if I put a bounty on this bug report?
> 
> I am not sure in fact; the main problem is no one is really interested in
> revise if my approach is the expected one and if the patch is what we want.

Well, maybe a little incentive can help which is why I asked :-).

I don't have the knowledge myself to help with the issue, but I would greatly
appreciate it if it could finally get fixed since it has been a burden ever
since the 2.29 release when this API got changed.

Adrian
  

* John Paul Adrian Glaubitz:

> On 8/19/21 6:52 PM, Adhemerval Zanella wrote:> On 19/08/2021 13:50, John Paul Adrian Glaubitz wrote:
>>> Hi!
>>>
>>> On 8/19/21 6:19 PM, Adhemerval Zanella wrote:
>>>>> Are there any news on this patch series besides the v3 version? [1]
>>>>
>>>> Unfortunately it is requires reviews. Since it only affects legacy
>>>> ABIs there is no much traction for fixing it.
>>>
>>> Would it help if I put a bounty on this bug report?
>> 
>> I am not sure in fact; the main problem is no one is really interested in
>> revise if my approach is the expected one and if the patch is what we want.
>
> Well, maybe a little incentive can help which is why I asked :-).

I can't speak for everyone, but for me offers of payment raise
complicated compliance issues.  It's hard to avoid the appearance of a
conflict of interest once third-party payments are involved.  That is
even true if the people involved never intended to claim any money.

Thanks,
Florian
  

On 8/19/21 7:15 PM, Florian Weimer wrote:
>> Well, maybe a little incentive can help which is why I asked :-).
> 
> I can't speak for everyone, but for me offers of payment raise
> complicated compliance issues.  It's hard to avoid the appearance of a
> conflict of interest once third-party payments are involved.  That is
> even true if the people involved never intended to claim any money.

So, what would be the best way to get this bug fixed then? I'm open to
any suggested path way.

It's an important fix for anyone using qemu-user in a common use case.

Adrian
  

Hi all,

It seems that Peter Maydell of qemu fame thinks it could be solved in qemu
if/when the kernel people add an explicit 32bit mode to the filesystem API.

See the bug here:
https://gitlab.com/qemu-project/qemu/-/issues/263

And specifically, he mentions that Linus Walleij has submitted a
preliminary patch, which has yet to be looked at.

https://lore.kernel.org/qemu-
devel/20201117233928.255671-1-linus.walleij@linaro.org/

Not sure if this is an easier path or not, but it might be worth pinging
the kernel ML.

Best regards,
Dave

On Thu, Aug 19, 2021 at 5:30 PM John Paul Adrian Glaubitz <
glaubitz@physik.fu-berlin.de> wrote:

> On 8/19/21 7:15 PM, Florian Weimer wrote:
> >> Well, maybe a little incentive can help which is why I asked :-).
> >
> > I can't speak for everyone, but for me offers of payment raise
> > complicated compliance issues.  It's hard to avoid the appearance of a
> > conflict of interest once third-party payments are involved.  That is
> > even true if the people involved never intended to claim any money.
>
> So, what would be the best way to get this bug fixed then? I'm open to
> any suggested path way.
>
> It's an important fix for anyone using qemu-user in a common use case.
>
> Adrian
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaubitz@debian.org
> `. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
>   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>
>