[Bug libc/23960] [2.28 Regression]: New getdents{64} implementation breaks qemu-user

["adhemerval.zanella at linaro dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=23960

--- Comment #64 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
> (In reply to Adhemerval Zanella from comment #32)
> > although it is a subpar resolution for a kernel issue.  Newer architectures
> > with mixed 32 and 64 bits support will continue to be broken without a
> > proper kernel fix since they use SYS_getdents64 for getdents.
> 
> The kernel is the wrong place to work around this.  glibc should be using 64
> bit struct dirent so it can actually handle the truth.

I agree and this an overlook from we glibc maintainers to allow newer 32-bit
ABIs to support non-LFS interface. Current pratice now is to enforce 64-bit
off_t for all newer ABIs (for instance as done for arc and riscv32).

However, there are still legacy ABIs which supports non-LFS and even one that
support without having the legacy kernel interface (nios2 and csky for
instance).

> 
> > What I think we should do is:
> > 
> >   1. *Deprecate* non-LFS usage in a multi-step way as discussed in
> > libc-alpha [1]. We will need to take care of the issue brought by Joseph,
> > but it will mean eventually the non-LFS interfaces will be just provided as
> > compatibility symbols.
> 
> I agree.
> 
> >   2. Push to distro on 32-bits to *stop* building packages in non-LFS mode
> > as default. Some distro already gets this right, but it seems some still
> > lacking support.
> 
> For that to happen, please make glibc at least emit a warning--although with
> a problem this bad, I'd prefer an error--if _FILE_OFFSET_BITS != 64 and
> SIZEOF_LONG < 8 and readdir is used.
> 
> I use this in dirent.h:
> 
> #ifndef _LIBC
> #if __SIZEOF_LONG__ < 8
> #ifndef __USE_FILE_OFFSET64
> #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 32
> #warning \"Using -D_FILE_OFFSET_BITS=32 and using readdir is a bad idea, see
> <https://bugzilla.kernel.org/show_bug.cgi?id=205957>\"
> #else
> #undef readdir
> #define readdir @READDIR_WITHOUT_FILE_OFFSET64_IS_A_REALLY_BAD_IDEA@
> #endif
> #endif
> #endif
> #endif
> 
> It's much better to find problems this way than to have programs fail at
> random times at runtime depending on file system internals.
> 
> Or use gcc's "deprecated" attribute on readdir, with a message, in order to
> at least warn.  But, really, does this sound like something harmless enough
> to only warn?  It does not to me.

This is quite disruptive and with potentialy breakage in a lot of scenarios. As
suggested by Joseph [1], we need to make it more seamlessly over multiple
releases. We already have the bug tracer to make 64-bit LFS default (BZ#13047),
but we also need to take care of BZ#14106, BZ#15766, and glibc build/tests
itself.

What I would like to get on 2.33 is to move ld.so and libc.so to use non-LFS
internally first by using explicit *64 interface.  It will make build glibc
itself with default LFS easier.

The point that will require a *lot* of work is to check and adapt the testcases
to systematically check for all LFS interfaces in the various modes. 

> 
> >   3. Continue to push kernel developers to provide a correct fix for this
> > issue. 
> 
> We shouldn't do that in the kernel (see beginning of this text).
> 
> It's impossible to store a 64 bit result into a 32 bit slot.
> 
> Also, if you call SYS_getdents64, you should expect a 64 bit result.  It's
> in the name.
> 
> Please don't use SYS_getdents.  Just please mandate LFS instead.  This
> should have been done long (decades) ago.

I am not inclined to keep using non-LFS internally, ideally I would like to
remove *all* non-LFS usage even on non-LFS symbols.  It would be similar to
work done on y2038 support, with the advantage that it won't need to handle
ENOSYS and add non-LFS fallback (we will just need to handel overflow as some
ABIs do with 'generic' internal interfaces).

And I have a working solution for this issue [1]. I did not get much review on
my last try [3], but debian and gentoo developers told me that it has fixed
their issues on both qemu and bootstrap.

The bulk of the change is:

--
It allows to obtain the expected entry offset on telldir and set
it correctly on seekdir on platforms where long int is smaller
than off64_t.

On such cases telldir will mantain an internal list that maps the
DIR object off64_t offsets to the returned long int (the function
return value).  The seekdir will then set the correct offset from
the internal list using the telldir as the list key.

It also removes the overflow check on readdir and the returned value
will be truncated by the non-LFS off_t size.  As Joseph has noted
in BZ #23960 comment #22, d_off is an opaque value and since
telldir/seekdir works regardless of the returned dirent d_off value.

Finally it removed the requirement to check for overflow values on
telldir (BZ #24050).
--

And Florian has raised some question about making 'telldir' fails. The standard
does not really allow it, but I think it is feasible concession to deprecated
interface.

I will commit the first two patches, they have been acked on previous
iterations (they are mainly refactoring and some interfaces fixes) and send
remmaining ones to review. Maybe we get them on 2.33 and we can backport if
required.

[1] https://sourceware.org/legacy-ml/libc-alpha/2019-01/msg00124.html
[2]
https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/bz23960
[3] https://sourceware.org/pipermail/libc-alpha/2020-April/112866.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.