Use statfs64() in shm_open() and posix_getpt().

  Philip Guenther noted in [BZ 15514] that statfs() may EOVERFLOW when
running 32-bit programs on 64-bit systems, and that shm_open() and
posix_getpt() may be affected.

This patch modifies shm_open() and posix_getpt() to use statfs64()
instead of __statfs().

Reported-by: Philip Guenther <guenther@gmail.com>
---
 ChangeLog                          | 9 +++++++++
 sysdeps/unix/sysv/linux/getpt.c    | 6 +++---
 sysdeps/unix/sysv/linux/shm_open.c | 6 +++---
 3 files changed, 15 insertions(+), 6 deletions(-)
  

Peter TB Brett <peter@peter-b.co.uk> writes:

> +	      || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0

This is not namespace clean, and will introduce a PLT call (you did run
make check?).

Andreas.
  

On 2014-04-13 08:44, Andreas Schwab wrote:
> Peter TB Brett <peter@peter-b.co.uk> writes:
> 
>> +	      || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0
> 
> This is not namespace clean, and will introduce a PLT call (you did run
> make check?).

I specifically asked about this on libc-help yesterday:

http://thread.gmane.org/gmane.comp.lib.glibc.user/2196

I got the following answer:

On 2014-04-12 18:02, Carlos O'Donell wrote:
> 
> To make a long story short you should just call statfs64.

Some further advice and/or explanation would be appreciated.

                                          Peter
  

On 2014-04-13 08:44, Andreas Schwab wrote:
> Peter TB Brett <peter@peter-b.co.uk> writes:
> 
>> +	      || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0
> 
> This is not namespace clean, and will introduce a PLT call (you did run
> make check?).

I just re-ran `make check' to verify -- and received no output relating 
to statfs.  Which specific test am I supposed to be looking at?

                                           Peter
  

On Sun, Apr 13, 2014 at 09:44:11AM +0200, Andreas Schwab wrote:
> Peter TB Brett <peter@peter-b.co.uk> writes:
> 
> > +	      || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0
> 
> This is not namespace clean, and will introduce a PLT call (you did run
> make check?).
>
According to Carlos its part of librt.so and indirection is needed
there, see

https://www.sourceware.org/ml/libc-help/2014-04/msg00024.html
  

Ondřej Bílka <neleai@seznam.cz> writes:

> On Sun, Apr 13, 2014 at 09:44:11AM +0200, Andreas Schwab wrote:
>> Peter TB Brett <peter@peter-b.co.uk> writes:
>> 
>> > +	      || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0
>> 
>> This is not namespace clean, and will introduce a PLT call (you did run
>> make check?).
>>
> According to Carlos its part of librt.so

??? posix_openpt is part of libc.

Andreas.
  

On Sun 13 Apr 2014 19:07:49 Andreas Schwab wrote:
> Ondřej Bílka <neleai@seznam.cz> writes:
> > On Sun, Apr 13, 2014 at 09:44:11AM +0200, Andreas Schwab wrote:
> >> Peter TB Brett <peter@peter-b.co.uk> writes:
> >> > +	      || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0
> >> 
> >> This is not namespace clean, and will introduce a PLT call (you did run
> >> make check?).
> > 
> > According to Carlos its part of librt.so
> 
> ??? posix_openpt is part of libc.

right, posix_openpt is part of libc, but shm_open is part of librt.  so the 
former should use __statfs64 to avoid the PLT while the latter should use 
statfs64.

so can we get an updated patch now ? :)
-mike
  

On Sat, 2 Aug 2014, Mike Frysinger wrote:

> right, posix_openpt is part of libc, but shm_open is part of librt.  so the 
> former should use __statfs64 to avoid the PLT while the latter should use 
> statfs64.

No, both should use __statfs64 as statfs64 is not part of the POSIX 
namespace but shm_open is.  As previously discussed, PLT avoidance and 
being namespace-clean are separate issues.

That does of course mean __statfs64 needs to be exported from libc.so.  
Although you could use a GLIBC_PRIVATE export (and that's the conservative 
approach at this development stage), eventually we'll want such exports at 
public symbol versions as part of a fix for bug 14106.  (statfs isn't part 
of a standard, so the need for such a public export is a bit less clear, 
but it still seems appropriate given that you can use <sys/statfs.h> 
without _LARGEFILE64_SOURCE being defined, and in that case it seems 
reasonable to consider statfs64 part of the user's namespace.)
  

On Mon 04 Aug 2014 15:51:15 Joseph S. Myers wrote:
> On Sat, 2 Aug 2014, Mike Frysinger wrote:
> > right, posix_openpt is part of libc, but shm_open is part of librt.  so
> > the
> > former should use __statfs64 to avoid the PLT while the latter should use
> > statfs64.
> 
> No, both should use __statfs64 as statfs64 is not part of the POSIX
> namespace but shm_open is.  As previously discussed, PLT avoidance and
> being namespace-clean are separate issues.

why does that matter ?  statfs64 is always exported by libc and thus is in the 
visible ABI namespace.  i don't see why glibc's internals need to be concerned 
with conforming to POSIX at all -- they're internals.  what are you trying to 
cater to ?
-mike
  

On Mon, Aug 4, 2014 at 5:49 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Mon 04 Aug 2014 15:51:15 Joseph S. Myers wrote:
>> On Sat, 2 Aug 2014, Mike Frysinger wrote:
>> > right, posix_openpt is part of libc, but shm_open is part of librt.  so
>> > the
>> > former should use __statfs64 to avoid the PLT while the latter should use
>> > statfs64.
>>
>> No, both should use __statfs64 as statfs64 is not part of the POSIX
>> namespace but shm_open is.  As previously discussed, PLT avoidance and
>> being namespace-clean are separate issues.
>
> why does that matter ?  statfs64 is always exported by libc and thus is in the
> visible ABI namespace.  i don't see why glibc's internals need to be concerned
> with conforming to POSIX at all -- they're internals.  what are you trying to
> cater to ?


A program can define a variable called statfs64 which is used instead
of the libc.so one when calling from librt.so.

Thanks,
Andrew Pinski


> -mike
  

On Mon 04 Aug 2014 18:06:58 Andrew Pinski wrote:
> On Mon, Aug 4, 2014 at 5:49 PM, Mike Frysinger wrote:
> > On Mon 04 Aug 2014 15:51:15 Joseph S. Myers wrote:
> >> On Sat, 2 Aug 2014, Mike Frysinger wrote:
> >> > right, posix_openpt is part of libc, but shm_open is part of librt.  so
> >> > the
> >> > former should use __statfs64 to avoid the PLT while the latter should
> >> > use
> >> > statfs64.
> >> 
> >> No, both should use __statfs64 as statfs64 is not part of the POSIX
> >> namespace but shm_open is.  As previously discussed, PLT avoidance and
> >> being namespace-clean are separate issues.
> > 
> > why does that matter ?  statfs64 is always exported by libc and thus is in
> > the visible ABI namespace.  i don't see why glibc's internals need to be
> > concerned with conforming to POSIX at all -- they're internals.  what are
> > you trying to cater to ?
> 
> A program can define a variable called statfs64 which is used instead
> of the libc.so one when calling from librt.so.

librt/libpthread/etc... have lots of calls to funcs from libc which can be 
classified in the same way.  they aren't going through GLIBC_PRIVATE nor using 
__ prefixes.  what makes this any different ?
-mike
  

On Mon 04 Aug 2014 23:50:40 Mike Frysinger wrote:
> On Mon 04 Aug 2014 18:06:58 Andrew Pinski wrote:
> > On Mon, Aug 4, 2014 at 5:49 PM, Mike Frysinger wrote:
> > > On Mon 04 Aug 2014 15:51:15 Joseph S. Myers wrote:
> > >> On Sat, 2 Aug 2014, Mike Frysinger wrote:
> > >> > right, posix_openpt is part of libc, but shm_open is part of librt. 
> > >> > so
> > >> > the
> > >> > former should use __statfs64 to avoid the PLT while the latter should
> > >> > use
> > >> > statfs64.
> > >> 
> > >> No, both should use __statfs64 as statfs64 is not part of the POSIX
> > >> namespace but shm_open is.  As previously discussed, PLT avoidance and
> > >> being namespace-clean are separate issues.
> > > 
> > > why does that matter ?  statfs64 is always exported by libc and thus is
> > > in
> > > the visible ABI namespace.  i don't see why glibc's internals need to be
> > > concerned with conforming to POSIX at all -- they're internals.  what
> > > are
> > > you trying to cater to ?
> > 
> > A program can define a variable called statfs64 which is used instead
> > of the libc.so one when calling from librt.so.
> 
> librt/libpthread/etc... have lots of calls to funcs from libc which can be
> classified in the same way.  they aren't going through GLIBC_PRIVATE nor
> using __ prefixes.  what makes this any different ?

just caught up on another thread from while i was gone.  Carlos wrote up a 
nice explanation here:
https://sourceware.org/glibc/wiki/Style_and_Conventions#Double-underscore_names_for_public_API_functions

so the answer to the other examples i highlighted is that they're wrong too :)
-mike
  

On Mon, Aug 04, 2014 at 06:06:58PM -0700, Andrew Pinski wrote:
> On Mon, Aug 4, 2014 at 5:49 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Mon 04 Aug 2014 15:51:15 Joseph S. Myers wrote:
> >> On Sat, 2 Aug 2014, Mike Frysinger wrote:
> >> > right, posix_openpt is part of libc, but shm_open is part of librt.  so
> >> > the
> >> > former should use __statfs64 to avoid the PLT while the latter should use
> >> > statfs64.
> >>
> >> No, both should use __statfs64 as statfs64 is not part of the POSIX
> >> namespace but shm_open is.  As previously discussed, PLT avoidance and
> >> being namespace-clean are separate issues.
> >
> > why does that matter ?  statfs64 is always exported by libc and thus is in the
> > visible ABI namespace.  i don't see why glibc's internals need to be concerned
> > with conforming to POSIX at all -- they're internals.  what are you trying to
> > cater to ?
> 
> 
> A program can define a variable called statfs64 which is used instead
> of the libc.so one when calling from librt.so.

While according to POSIX this is true (and actually statfs64 is a
really bad example since even statfs isn't POSIX), in general making
your own symbols that clash with the *64 names really can't work with
glibc at all, at least not on 32-bit systems. -D_FILE_OFFSET_BITS=64
is basically mandatory on 32-bit systems for many functions to work
since modern systems have 64-bit inode numbers and all sorts of things
fail without it.

Isn't it about time glibc added namespace-safe names for these
functions (e.g. __open64, etc.) and changed the headers to redirect to
these namespace-safe names rather than using the old ones?

Rich