[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID

--- Comment #7 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-02-18 19:51:29 UTC ---
I do not believe there is a glibc bug here.

* The bug report quotes a manpage for the readdir system call as saying that
d_off refers to the current dirent rather than the next dirent.  That is
correct, but irrelevant.  The readdir system call is a backwards compatibility
one using a very old version of the dirent structure.  With the getdents and
getdents64 syscalls, different structures are used, and in those structures the
semantics are that d_off refers to the next dirent, as you will see if you
refer to the kernel sources or the getdents manpage.  The getdents64 syscalls
is the relevant one here.

* The bug report claims there is an assumption that d_off contains a byte
offset.  Actually, there is no such assumption.  The only requirement is that
the kernel does not use -1 as an offset.  Otherwise, the offsets are used as
opaque values, stored, copied, converted to the userspace type (with the result
only compared for equality, not used for arithmetic) and used as an argument
for __lseek64.  Using as an argument for __lseek64 (with SEEK_SET) is not using
the value as a byte offset; it's up to each filesystem in the kernel to
implement seeking on directories correctly so that it works with the opaque
d_off values provided by the kernel.  Some filesystems may use byte offsets and
some use other values involving a more complicated implementation of that seek
operation in the kernel.

* Various different filesystems have been used for automounting (e.g. autofs /
autofs4) and this report is not specific about exactly what filesystem, with
what kernel version, is used in the problem situations.  (Kernels before 2.6.0
are in any case not in practice relevant to current glibc, although some
support code for them has yet to be removed.)  But if a filesystem did not
handle seeking with values provided in d_off, that would be a bug in the
filesystem.

* It is certainly not correct to seek with byte offsets computed by glibc,
since offsets for seeking in a directory are the same sort of opaque cookie
provided in d_off and only those values may be passed to the kernel as a
position to which to seek.  So the patch here cannot be correct.

* Finally, there is a concern about the conversion of d_off from 64-bit to
32-bit.  It's true that some applications may not use this value (used for
telldir/seekdir), but the same applies to any other value returned by the
kernel (inode numbers in particular); glibc cannot know what values the
application wants and so must return EOVERFLOW if any value from the kernel
cannot be represented in the userspace type.  The way for applications to avoid
this is to be built with _FILE_OFFSET_BITS=64 so that the 64-bit interfaces are
used, and this is the normal practice nowadays for most applications given the
desire to support files over 2GB on 32-bit systems.

Regarding the other issues described with the source code, note that nbytes is
not provided by the user of glibc since getdents is an internal-only interface.
 There may well be issues there, but on any given system they would either be
latent or appear every time the relevant code in getdents is executed, so it
seems unlikely they affect any current system.  In any case, please always
avoid mixing different issues in a single bug; file separate issues for these
separate problems if you think they are still present.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[dan.tsafrir at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14

Dan Tsafrir <dan.tsafrir at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |

--- Comment #8 from Dan Tsafrir <dan.tsafrir at gmail dot com> 2012-02-19 14:36:28 UTC ---
Joseph,

It seems you've mistakenly misread the bug report as claiming that
making assumptions regarding the offset is somehow correct/justified,
whereas, conversely, the bug reports just points to *glibc code* that
makes the erroneous assumption.

Hence, the report does not need to specify the filesystem on which the
bug was triggered, because it points to *glibc code* that suffers from
what you yourself acknowledge is a serious problem.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID

--- Comment #9 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-02-19 16:03:04 UTC ---
Dan,

Please read my comments in more detail.  Because offsets in directories are
opaque cookies (if you look at the autofs sources in the kernel, you'll see
that they are hash values for autofs, for example), and because the __lseek64
call's offset ends up being passed to the filesystem's llseek method which
expects just such an opaque cookies, it can never be correct to pass a value
calculated by adding up record lengths to __lseek64 on a file descriptor for a
directory - and so those parts of your patch (adjusting how last_offset is
computed) cannot be correct since last_offset is (only) used to pass to
__lseek64 for such a file descriptor.  Maybe last_offset should be renamed to
make clear that in normal terms it is not an offset; it's an opaque value on
which no arithmetic is valid.

I do not see any glibc code (without your patch) that does any arithmetic on
the offset values; it only copies them.  If you see any code that assumes that
d_off values are meaningful in arithmetic (as opposed to being opaque values
that can be used later to seek to a particular point in a directory), what
specific lines of code (in current git master) are they?

For reference, "grep last_offset getdents.c" shows the following for me, none
of which treat last_offset as anything other than an opaque value.

  off64_t last_offset = -1;
                  if (last_offset != -1)
                      __lseek64 (fd, last_offset, SEEK_SET);
              last_offset = d_off;
            assert (last_offset != -1);
            __lseek64 (fd, last_offset, SEEK_SET);
        last_offset = kdp->d_off;

Similarly, d_off is purely treated as opaque.

The second and third issues are likely bugs (albeit latent bugs), but
*different* bugs, and one issue in the tracker should only ever be used for a
single issue with the library.  Thus, those second and third issues (relating
to nbytes, not d_off) should be filed as two separate tracker issues.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[carlos at systemhalted dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14

Carlos O'Donell <carlos at systemhalted dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |testsuite
                 CC|                            |carlos at systemhalted dot
                   |                            |org
              Flags|testsuite+                  |

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[carlos at systemhalted dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14

Carlos O'Donell <carlos at systemhalted dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|examined+                   |

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[jackie.rosen at hushmail dot com]

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

Jackie Rosen <jackie.rosen at hushmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jackie.rosen at hushmail dot com

--- Comment #10 from Jackie Rosen <jackie.rosen at hushmail dot com> ---
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.

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

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[schwab at sourceware dot org]

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

Andreas Schwab <schwab at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|jackie.rosen at hushmail dot com   |

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

[Bug libc/14] Bug (+fix) in readdir() due to getdents()

[pasky at suse dot cz]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |glibc-bugs at sources dot
                   |                            |redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=14

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.