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

["jsm28 at gcc dot gnu.org" <sourceware-bugzilla at sourceware dot 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.