Bug #11685 “libc6: Bug (+fix) in readdir() due to getdents()” : Bugs : glibc package : Ubuntu

Revision history for this message

In Sourceware.org Bugzilla #14, Dan-tsafrir (dan-tsafrir) wrote on 2004-02-09:

#10

Download full text (8.8 KiB)

- This bug report was already posted in <email address hidden>
I'm resubmitting it here at the request of Andreas Jaeger.

- All the following relates to the file:

glibc-2.3.2/sysdeps/unix/sysv/linux/getdents.c

  I report more than one bug in getdents() so please bare with me :> (the
  description is actually much longer than the patch). In a nutshell, the
  following program might (nondeterministically) enter an endless-loop:

      DIR *dir = opendir("some-autofs-directory");
      struct dirent *e;
      while( (e = readdir(dir)) != NULL ) // might never return null
          printf("ino=%d name=%s\n", e->d_ino, e->d_name)

- In the implementation of getdents(), the `d_off' field (belonging to the
  linux kernel's dirent structure) is falsely assumed to contain the *byte*
  offset to the next dirent.
  Note that the linux manual of the readdir system-call states that d_off
  is the "offset to *this* dirent" while glibc's getdents treats it as the
  offset to the *next* dirent.

- In practice, both of the above are wrong/misleading. The `d_off' field
  may contain illegal negative values, 0 (should also never happen as the
  "next" dirent's offset must always be bigger then 0), or positive values
  that are bigger than the size of the directory-file itself:

  o We're not sure what the Linux kernel intended to place in this field,
    but our experience shows that on "real" file systems (that actually
    reside on some disk) the offset seems to be a simple (not necessarily
    continuous) counter: e.g. first entry may have d_off=1, second: d_off=2,
    third: d_off=4096, fourth=d_off=4097 etc. We conjecture this is the
    serial of the dirent record within the directory (and so, this is indeed
    the "offset", but counted in records out of which some were already
    removed).

  o For file systems that are maintained by the amd automounter (automount,
    directories) the d_off seems to be arbitrary (and may be negative, zero
    or beyond the scope of a 32bit integer). We conjecture the amd doesn't
    assign this field and the received values are simply garbage.

- Fortunately, there is actually no need to use d_off as all the needed
  information is embedded in the d_reclen field (dirents are consecutive
  and the "next" dirent is found exactly after the "current" dirent).
  And indeed, glibc's getdents() uses the above technique most of the time.
  But, there are certain exceptional-conditions that might trigger
  getdents() to stop using on the dependable d_reclen and start using
  the unreliable d_off (Furthermore, these exceptional-conditions are
  identified based on the unreliable information held by d_off).

- One aspect of this bug was actually reported in 2001:
      http://mail.gnu.org/archive/html/bug-glibc/2001-03/msg00048.html
  but was wrongly (IMHO) dismissed by Ulrich Drepper who claimed that the
  bug is actually in the linux-kernel. The linux-kernel folks do not intend
  to change the data of their 'struct dirent'. They want it just the way it
  is and don't care if the glibc folks "wrongly" interpret the `d_off' field.

- The bug in glibc only occurs when an "overflow" is *wrongly* detecte...

- This bug report was already posted in libc-alpha@sources.redhat.com
  I'm resubmitting it here at the request of Andreas Jaeger.

- All the following relates to the file:

glibc-2.3.2/sysdeps/unix/sysv/linux/getdents.c

I report more than one bug in getdents() so please bare with me :> (the
  description is actually much longer than the patch). In a nutshell, the
  following program might (nondeterministically) enter an endless-loop:

DIR           *dir = opendir("some-autofs-directory");
      struct dirent *e;
      while( (e = readdir(dir)) != NULL ) // might never return null
          printf("ino=%d name=%s\n", e->d_ino, e->d_name)

- In the implementation of getdents(), the `d_off' field (belonging to the
  linux kernel's dirent structure) is falsely assumed to contain the *byte*
  offset to the next dirent.
  Note that the linux manual of the readdir system-call states that d_off
  is the "offset to *this* dirent" while glibc's getdents treats it as the
  offset to the *next* dirent.

- In practice, both of the above are wrong/misleading. The `d_off' field
  may contain illegal negative values, 0 (should also never happen as the
  "next" dirent's offset must always be bigger then 0), or positive values
  that are bigger than the size of the directory-file itself:

o We're not sure what the Linux kernel intended to place in this field,
    but our experience shows that on "real" file systems (that actually
    reside on some disk) the offset seems to be a simple (not necessarily
    continuous) counter: e.g. first entry may have d_off=1, second: d_off=2,
    third: d_off=4096, fourth=d_off=4097 etc. We conjecture this is the
    serial of the dirent record within the directory (and so, this is indeed
    the "offset", but counted in records out of which some were already
    removed).

o For file systems that are maintained by the amd automounter (automount,
    directories) the d_off seems to be arbitrary (and may be negative, zero
    or beyond the scope of a 32bit integer). We conjecture the amd doesn't
    assign this field and the received values are simply garbage.

- Fortunately, there is actually no need to use d_off as all the needed
  information is embedded in the d_reclen field (dirents are consecutive
  and the "next" dirent is found exactly after the "current" dirent).
  And indeed, glibc's getdents() uses the above technique most of the time.
  But, there are certain exceptional-conditions that might trigger
  getdents() to stop using on the dependable d_reclen and start using
  the unreliable d_off (Furthermore, these exceptional-conditions are
  identified based on the unreliable information held by d_off).

- One aspect of this bug was actually reported in 2001:
      http://mail.gnu.org/archive/html/bug-glibc/2001-03/msg00048.html
  but was wrongly (IMHO) dismissed by Ulrich Drepper who claimed that the
  bug is actually in the linux-kernel. The linux-kernel folks do not intend
  to change the data of their 'struct dirent'. They want it just the way it
  is and don't care if the glibc folks "wrongly" interpret the `d_off' field.

- The bug in glibc only occurs when an "overflow" is *wrongly* detected
  (see lines: 176-183 in getdents.c).
  Overflow might happen when the kernel uses `kernel_dirent64' that contains
  64bit wide fields (d_ino & d_off), while the user uses `struct dirent'
  through readdir(3) (rather than `struct dirent64' through readdir64).
  in which the corresponding fields are 32bit wide.
  [ This is of course a legal scenario: the user shouldn't know nor care
    how the kernel implements its readdir ]

- In such an "overflow" situation, getdents() tries to lseek (the
  directory-fd) to the end of the last-legal-dirent that getdents()
  has successfully read.
  This offset is supposedly held by the local variable `last_offset'.
  But, since `last_offset' is assigned with `d_off' on each iteration,
  and since as mentioned above `d_off' usually holds an incorrect value,
  the lseek is not performed to the correct place.
  getdents() then returns the number of bytes successfully read.

- This situation of course causes the next invocation of readdir() to
  return erroneous data due to the incorrect offset associated with
  the fd.
  Furthermore, it often happens (for autofs directories that serve as
  automount points and are maintained by the amd) that the first dirent-entry
  has d_off=0 and the second dirent causes a wrong "overflow".
  In this case getdents() will return the number of bytes composing
  the first successfully read dirent, and will wrongly lseek to 0: the
  beginning of the file.
  This means that the next invocation of readdir(3) will return (again)
  the first dirent, and so on (never returning NULL), causing any
  program that uses readdir(3) to enter into an endless loop!

How to fix (the above bug + another two):
########################################

1:---------------------------------------------------------------------
Instead of line 193:
        last_offset = d_off;
write:
        if( last_offset == -1 )
                last_offset = 0;
        last_offset += old_reclen;

[  This makes `last_offset' hold the *real* last offset, and prevents
       the wrong lseek() when an "overflow" occurs. It works since the
       `d_reclen' is reliable and correct, as reflected by the manner
       getdents() uses `old_reclen'  ]

2:---------------------------------------------------------------------
The condition that identifies an "overflow" situation should also be
changed:
Instead of lines 176-179:
        if ((sizeof (outp->u.d_ino) != sizeof (inp->k.d_ino)
             && outp->u.d_ino != d_ino)
            || (sizeof (outp->u.d_off) != sizeof (inp->k.d_off)
                && outp->u.d_off != d_off))
only write:
        if ( sizeof (outp->u.d_ino) != sizeof (inp->k.d_ino)
             && outp->u.d_ino != d_ino )

[  I removed the right-side of the `or': since the `d_off' field is
       buggy and unusable, don't include it in the check for overflow.
       The alterative is that programs reading the content of a directory
       maintained by the amd using readdir(3), will usually fail due to
       overflow, even though the directory is prefectly fine.  ]

3:---------------------------------------------------------------------
The following is another bug I think you have (which is unrelated
to the above).
Instead of line 120:
        && nbytes <= sizeof (DIRENT_TYPE))
write:
        && nbytes <= sizeof (struct kernel_dirent64))

[  The above condition decides whether the user gave a large enough
       buffer in which `struct kernel_dirent64' will fit (`nbytes' is the
       size of the user's `buf'). If `buf' is too small, a bigger buffer
       is allocated (lines 122-124) so as to be able to hold `struct
       kernel_dirent64'  ]

4:---------------------------------------------------------------------
I think the following is also an (unrelated) bug:
In lines 216-221:

red_nbytes = MIN (nbytes
                      - ((nbytes / (offsetof (DIRENT_TYPE, d_name) + 14))
                         * size_diff),
                      nbytes - size_diff);

skdp = kdp = __alloca (red_nbytes);

It is obvious that `red_nbytes' may be negative (or rather, wrapped-around),
since `nbytes' is a value given by the user (and may even be 0).
-----------------------------------------------------------------------

The above patch is given below (your interface doesn't allow attachments),
it was generated using diff -u.
This patch was applied to our installation of glibc and is successfully
used for a few months now by hundreds of of machines.
Thanks,
    --Dan

--- getdents.c.orig	2003-12-04 19:11:38.000000000 +0200
+++ getdents.c	2003-12-04 19:38:06.000000000 +0200
@@ -117,7 +117,7 @@
       size_t kbytes = nbytes;
       if (offsetof (DIRENT_TYPE, d_name)
 	  < offsetof (struct kernel_dirent64, d_name)
-	  && nbytes <= sizeof (DIRENT_TYPE))
+	  && nbytes <= sizeof (kernel_dirent64))
 	{
 	  kbytes = nbytes + offsetof (struct kernel_dirent64, d_name)
 		   - offsetof (DIRENT_TYPE, d_name);
@@ -175,8 +175,7 @@
 	      outp->u.d_off = d_off;
 	      if ((sizeof (outp->u.d_ino) != sizeof (inp->k.d_ino)
 		   && outp->u.d_ino != d_ino)
-		  || (sizeof (outp->u.d_off) != sizeof (inp->k.d_off)
-		      && outp->u.d_off != d_off))
+		  )
 		{
 		  /* Overflow.  If there was at least one entry
 		     before this one, return them without error,
@@ -190,7 +189,10 @@
 		  return -1;
 		}
 
-	      last_offset = d_off;
+	      if( last_offset == -1 )
+		last_offset = 0;
+	      last_offset += old_reclen;
+
 	      outp->u.d_reclen = new_reclen;
 	      outp->u.d_type = d_type;
 
@@ -213,6 +215,7 @@
     const size_t size_diff = (offsetof (DIRENT_TYPE, d_name)
 			      - offsetof (struct kernel_dirent, d_name));
 
+    /* bug? (nbytes might be smaller than right side of minus) */
     red_nbytes = MIN (nbytes
 		      - ((nbytes / (offsetof (DIRENT_TYPE, d_name) + 14))
 			 * size_diff),

Revision history for this message

Debian Bug Importer (debzilla) wrote on 2005-01-06:

#1

Message-Id: <email address hidden>
Date: Thu, 06 Jan 2005 16:19:58 +0200
From: Ely Levy <email address hidden>
To: Debian Bug Tracking System <email address hidden>
Subject: libc6: Bug (+fix) in readdir() due to getdents()

Package: libc6
Version: 2.3.2.ds1-13hujics
Severity: critical
Tags: patch
Justification: breaks the whole system

The full bug report/patch by Dan Tzafrir are in
http://sources.redhat.com/bugzilla/show_bug.cgi?id=14

-- System Information:
Debian Release: 3.1
APT prefers testing
APT policy: (500, 'testing')
Architecture: i386 (i686)
Kernel: Linux 2.6.10
Locale: LANG=C, LC_CTYPE=C (ignored: LC_ALL set to he_IL)

Versions of packages libc6 depends on:
ii libdb1-compat 2.1.3-7 The Berkeley database routines [gl

-- no debconf information

Revision history for this message

Debian Bug Importer (debzilla) wrote on 2005-01-13:

#2

Message-ID: <email address hidden>
Date: Thu, 13 Jan 2005 11:46:01 +0900
From: GOTO Masanori <email address hidden>
To: Ely Levy <email address hidden>, <email address hidden>,
<email address hidden>
Subject: Re: Bug#288948: libc6: Bug (+fix) in readdir() due to getdents()

severity 288948 important
merge 180065 288948
thanks

At Thu, 06 Jan 2005 16:19:58 +0200,
Ely Levy wrote:
> Version: 2.3.2.ds1-13hujics

What is this version? We don't have it.

> Justification: breaks the whole system

I don't think so. Almost all user don't hit this problem. I've
merged this bug to #180065.

> The full bug report/patch by Dan Tzafrir are in
> http://sources.redhat.com/bugzilla/show_bug.cgi?id=14

This is IRIX NFS server specific problem, and IIRC nowadays linux
kernel nfs client can work with a workaround patch by Trond.
I'll follow up to those bugs.

Regards,
-- gotom

Revision history for this message

Debian Bug Importer (debzilla) wrote on 2005-01-13:

#3

Message-ID: <email address hidden>
Date: Thu, 13 Jan 2005 13:37:54 +0200 (IST)
From: Ely Levy <email address hidden>
To: GOTO Masanori <email address hidden>
cc: <email address hidden>, <email address hidden>
Subject: Re: Bug#288948: libc6: Bug (+fix) in readdir() due to getdents()

On Thu, 13 Jan 2005, GOTO Masanori wrote:

> severity 288948 important
> merge 180065 288948
> thanks
>
> At Thu, 06 Jan 2005 16:19:58 +0200,
> Ely Levy wrote:
> > Version: 2.3.2.ds1-13hujics

It's 2.3.2.ds1 with a small patch to the hesiod system we do to make
it fit our system.

> What is this version? We don't have it.
>
> > Justification: breaks the whole system
>
> I don't think so. Almost all user don't hit this problem. I've
> merged this bug to #180065.
>
> > The full bug report/patch by Dan Tzafrir are in
> > http://sources.redhat.com/bugzilla/show_bug.cgi?id=14
>
> This is IRIX NFS server specific problem, and IIRC nowadays linux
> kernel nfs client can work with a workaround patch by Trond.
> I'll follow up to those bugs.

The bug is confirmed with kernel 2.6.10.
And the patch was tested on few libc versions and being used on wide
spread in our system for almost 2 years now with no problems.

> Regards,
> -- gotom
>
Ely

Revision history for this message

Matt Zimmerman (mdz) wrote on 2005-03-05:

#4

Jeff, is this something we should be concerned about for Hoary?

Revision history for this message

Jeff Bailey (jbailey) wrote on 2005-03-09:

#5

We had a version of this patch before we bumped up from 2.2 to 2.3 in Debian.
We dropped it because it wasn't clear what problem it was actually solving - and
we've so far avoided integrating it, since the kernel folks and glibc couldn't
agree on the right solution.

I suspect we should put it in, but since it's a corner case (only affects one
type of NFS server), I'd be inclined to leave it until after the release.

Resolving bug as 'remind' so that I see it post-hoary.

Revision history for this message

Jeff Bailey (jbailey) wrote on 2005-04-06:

#6

Reopening bug, setting target milestone to 5.10

Revision history for this message

Jeff Bailey (jbailey) wrote on 2005-05-18:

#7

Took a look at this, and the patch isn't 64 bit clean. Applies cleanly to 2.3.5
otherwise, though.

Revision history for this message

Jeff Bailey (jbailey) wrote on 2005-09-20:

#8

Dropping severity on this bug. It's existed for quite a while, and only hits
Irix servers.

Also, it looks like the patch
http://client.linux-nfs.org/Linux-2.6.x/2.6.12/linux-2.6.12-43-dirent_fix.diff
is intended to fix the problem, and
has been integrated into 2.6.13.

Marking bug as UPSTREAM, will close when >=2.6.13 hits dapper.

Revision history for this message

Jeff Bailey (jbailey) wrote on 2006-02-07:

#9

2.6.15 is now in the archive, which should also solve this problem. I don't have an IRIX NFS server here to test it, so if I'm wrong, please reopen. =)

Tks,
Jeff Bailey

Changed in glibc:
status:	Unconfirmed → Fix Released

Revision history for this message

In Sourceware.org Bugzilla #14, Decimal (decimal) wrote on 2006-02-21:

#11

This patch applies cleanly to sysdeps/unix/sysv/linux/getdents.c in the trunk.

I have created a first pass at a testcase but it requires an autofs filesystem
and is non-deterministic according to the bug comments so I have not actually
reproduced yet.

Revision history for this message

In Sourceware.org Bugzilla #14, Decimal (decimal) wrote on 2006-02-21:

#12

Created attachment 876
Patch taken from bug comments.

Revision history for this message

In Sourceware.org Bugzilla #14, Decimal (decimal) wrote on 2006-02-21:

#13

Created attachment 877
Preliminary testcase adapted from bug comments.

Revision history for this message

In Sourceware.org Bugzilla #14, Decimal (decimal) wrote on 2006-02-21:

#14

Created attachment 878
Standalone preliminary testcase adapted from bug comments.

Here is a standalone testcase as well if someone happens to have a system with
autofs to reproduce this and to refine the testcase.

Bug Watch Updater (bug-watch-updater) on 2006-06-15

Changed in glibc:
status:	Unconfirmed → Confirmed

Revision history for this message

In Sourceware.org Bugzilla #14, Petr Baudis (pasky) wrote on 2010-06-01:

#15

AFAICS, current kernel does put_user(file->f_pos, &lastdirent->d_off), thus
d_off should have well-defined meaning nowadays.

Revision history for this message

In Sourceware.org Bugzilla #14, Dan-tsafrir (dan-tsafrir) wrote on 2010-06-01:

#16

Petr, your reason for closing this bug is invalid:

(1) If you read the original bug report you'll notice that it's not
enough for d_off to have a "well defined meaning". What you really
need is for that meaning to be the same in both the kernel and in
libc. This bug report specifically points out the fact that the
meaning is different.

(2) Also, if you search for put_user(file->f_pos,&lastdirent->d_off)
in the kernel code you'd see that it existed many, many years before
this bug was submitted (the line is there since Linux-1.3.0, no
less). Thus, having this line in the kernel doesn't prove or disprove
anything.

Bug Watch Updater (bug-watch-updater) on 2011-05-26

Changed in glibc:
importance:	Unknown → Critical

Bug Watch Updater (bug-watch-updater) on 2011-08-11

Changed in glibc (Debian):
status:	New → Fix Released

Revision history for this message

In Sourceware.org Bugzilla #14, Jsm28 (jsm28) wrote on 2012-02-18:

#17

Download full text (3.3 KiB)

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 an...

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.

Revision history for this message

In Sourceware.org Bugzilla #14, Dan-tsafrir (dan-tsafrir) wrote on 2012-02-19:

#18

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.

Revision history for this message

In Sourceware.org Bugzilla #14, Jsm28 (jsm28) wrote on 2012-02-19:

#19

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.

Bug Watch Updater (bug-watch-updater) on 2012-02-24

Changed in glibc:
status:	Confirmed → Invalid

Revision history for this message

In Sourceware.org Bugzilla #14, Jackie-rosen (jackie-rosen) wrote on 2014-02-16:

#20

*** 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.

	Status	Importance	Assigned to	Milestone
GLibC	Invalid	Critical	sourceware-bugs #14
glibc (Debian)	Fix Released	Unknown	debbugs #288948
glibc (Ubuntu)	Fix Released	Medium	Jeff Bailey	Ubuntu ubuntu-6.06

Ubuntu
glibc package

libc6: Bug (+fix) in readdir() due to getdents()

Bug Description

Other bug subscribers

Remote bug watches

Ubuntuglibc package

libc6: Bug (+fix) in readdir() due to getdents()

Bug Description

Other bug subscribers

Remote bug watches

Ubuntu
glibc package