Use statfs64() in shm_open() and posix_getpt().
Commit Message
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(-)
Comments
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
@@ -1,3 +1,12 @@
+2014-04-13 Peter Brett <peter@peter-b.co.uk>
+
+ [BZ 15514]
+ * sysdeps/unix/sysv/linux/shm_open.c (where_is_shmfs): Check for
+ shmfs using statfs64().
+ * sysdeps/unix/sysv/linux/getpt.c (__posix_openpt): Check for
+ /dev/pts and devfs using statfs64().
+ Reported by Philip Guenther <guenther@gmail.com>
+
2014-04-12 Allan McRae <allan@archlinux.org>
[BZ #16838]
@@ -46,15 +46,15 @@ __posix_openpt (oflag)
fd = __open (_PATH_DEVPTMX, oflag);
if (fd != -1)
{
- struct statfs fsbuf;
+ struct statfs64 fsbuf;
static int devpts_mounted;
/* Check that the /dev/pts filesystem is mounted
or if /dev is a devfs filesystem (this implies /dev/pts). */
if (devpts_mounted
- || (__statfs (_PATH_DEVPTS, &fsbuf) == 0
+ || (statfs64 (_PATH_DEVPTS, &fsbuf) == 0
&& fsbuf.f_type == DEVPTS_SUPER_MAGIC)
- || (__statfs (_PATH_DEV, &fsbuf) == 0
+ || (statfs64 (_PATH_DEV, &fsbuf) == 0
&& fsbuf.f_type == DEVFS_SUPER_MAGIC))
{
/* Everything is ok. */
@@ -55,14 +55,14 @@ static void
where_is_shmfs (void)
{
char buf[512];
- struct statfs f;
+ struct statfs64 f;
struct mntent resmem;
struct mntent *mp;
FILE *fp;
/* The canonical place is /dev/shm. This is at least what the
documentation tells everybody to do. */
- if (__statfs (defaultdir, &f) == 0 && (f.f_type == SHMFS_SUPER_MAGIC
+ if (statfs64 (defaultdir, &f) == 0 && (f.f_type == SHMFS_SUPER_MAGIC
|| f.f_type == RAMFS_MAGIC))
{
/* It is in the normal place. */
@@ -97,7 +97,7 @@ where_is_shmfs (void)
/* First make sure this really is the correct entry. At least
some versions of the kernel give wrong information because
of the implicit mount of the shmfs for SysV IPC. */
- if (__statfs (mp->mnt_dir, &f) != 0 || (f.f_type != SHMFS_SUPER_MAGIC
+ if (statfs64 (mp->mnt_dir, &f) != 0 || (f.f_type != SHMFS_SUPER_MAGIC
&& f.f_type != RAMFS_MAGIC))
continue;