libc6: i386: readdir() occasionally fails with EOVERFLOW with cfs (cfs: CFS only gives 58-63 files with some programs)

[Gerrit Pape <pape-dbnbgs@smarden.org>]

reassign 175260 libc6
retitle 175260 libc6: i386: readdir() occasionally fails with EOVERFLOW with cfs
submitter 175260 !
thanks

Hello,

this problem shows up on some systems only, I wasn't able to reproduce
it on my systems, but I got an account on a machine of the OP, and was
able to track down the problem to some changes in libc6_2.3.1-6 in
sysdeps/unix/sysv/linux/getdents.c.

The bug doesn't happen with libc6_2.3.1-5 installed on the OP's system.
It does show up with libc6_2.3.1-6 and later versions.  As you can see
from the bug log, the impact is that the following test program (run in
a cfs directory) stops the readdir() loop due to readdir() returning -1,
and setting errno to EOVERFLOW (BTW: the Debian readdir(3) manpage
doesn't mention EOVERFLOW at all).

#include <stdio.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>
int main() {
   DIR *dp;
   struct dirent *dir;
   if ((dp = opendir(".")) == NULL) perror("opendir");
   errno =0;
   while ((dir = readdir(dp)) != NULL)
      printf("File: %s\n", dir->d_name);
   if (errno) perror("readdir");
   closedir(dp);
   _exit(0);
}

I attach the diff (from libc6_2.3.1-5 to libc6_2.3.1-6) I suppose is
causing the problem.  It's definitely the place where EOVERFLOW is set.

I'm not completely sure that this isn't a bug in cfs, but if it is, it
must be in there for years, since the source code didn't change that
much lately (cfs was initially written in 1993, updated in 1997, only
slightly patched since then).  If this indeed is a cfs bug, please
reassign this bug back, and I would appreciate a hint on what the
problem in cfs could be.

It also seems possible that the kernel version is involved, but I got
another report of the same bug from another user, using another kernel.

Since the changes in sysdeps/unix/sysv/linux/getdents.c trigger this
bug, please review these changes.

I still have a login on the machine having this bug, so if you need more
details, please ask, I should be able to provide them.

Thanks, Gerrit.

--- glibc-2.3.1/sysdeps/unix/sysv/linux/getdents.c	2003-07-06 18:06:15.000000000 +0200
+++ ../../t3/glibc-2.3.1/glibc-2.3.1/sysdeps/unix/sysv/linux/getdents.c	2003-07-06 18:20:24.000000000 +0200
@@ -47,8 +47,6 @@
 #endif
 
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
-#define test_overflow(VAR, SIZE) ((((VAR) < 0 ? -(VAR):(VAR)) >> 8*(SIZE)) != 0)
-
 
 extern int __syscall_getdents (int fd, char *__unbounded buf, unsigned int nbytes);
 extern int __syscall_getdents64 (int fd, char *__unbounded buf, unsigned int nbytes);
@@ -99,7 +97,7 @@
 internal_function
 __GETDENTS (int fd, char *buf, size_t nbytes)
 {
-  DIRENT_TYPE *dp;
+  off64_t last_offset = -1;
   ssize_t retval;
 
 #ifdef __NR_getdents64
@@ -110,7 +108,12 @@
 # ifndef __ASSUME_GETDENTS64_SYSCALL
       int saved_errno = errno;
 # endif
-      char *kbuf = buf;
+      union
+      {
+	struct kernel_dirent64 k;
+	DIRENT_TYPE u;
+	char b[1];
+      } *kbuf = (void *) buf, *outp, *inp;
       size_t kbytes = nbytes;
       if (offsetof (DIRENT_TYPE, d_name)
 	  < offsetof (struct kernel_dirent64, d_name)
@@ -123,11 +126,9 @@
       retval = INLINE_SYSCALL (getdents64, 3, fd, CHECK_N(kbuf, kbytes),
 			       kbytes);
 # ifndef __ASSUME_GETDENTS64_SYSCALL
-      if (retval != -1 && errno != -EINVAL)
+      if (retval != -1 || errno != EINVAL)
 # endif
 	{
-	  struct kernel_dirent64 *kdp;
-	  int64_t last_offset = 0;
 	  const size_t size_diff = (offsetof (struct kernel_dirent64, d_name)
 				    - offsetof (DIRENT_TYPE, d_name));
 
@@ -139,55 +140,65 @@
 	     need, don't do any conversions.  */
 	  if (offsetof (DIRENT_TYPE, d_name)
 	      == offsetof (struct kernel_dirent64, d_name)
-	      && sizeof (dp->d_ino) == sizeof (kdp->d_ino)
-	      && sizeof (dp->d_off) == sizeof (kdp->d_off))
+	      && sizeof (outp->u.d_ino) == sizeof (inp->k.d_ino)
+	      && sizeof (outp->u.d_off) == sizeof (inp->k.d_off))
 	    return retval;
 
-	  dp = (DIRENT_TYPE *)buf;
-	  kdp = (struct kernel_dirent64 *) kbuf;
-	  while ((char *) kdp < kbuf + retval)
+	  /* These two pointers might alias the same memory buffer.
+	     Standard C requires that we always use the same type for them,
+	     so we must use the union type.  */
+	  inp = kbuf;
+	  outp = (void *) buf;
+
+	  while (&inp->b < &kbuf->b + retval)
 	    {
 	      const size_t alignment = __alignof__ (DIRENT_TYPE);
-	      /* Since kdp->d_reclen is already aligned for the kernel
+	      /* Since inp->k.d_reclen is already aligned for the kernel
 		 structure this may compute a value that is bigger
 		 than necessary.  */
-	      size_t old_reclen = kdp->d_reclen;
+	      size_t old_reclen = inp->k.d_reclen;
 	      size_t new_reclen = ((old_reclen - size_diff + alignment - 1)
 				  & ~(alignment - 1));
-	      uint64_t d_ino = kdp->d_ino;
-	      int64_t d_off = kdp->d_off;
-	      unsigned char d_type = kdp->d_type;
-
-	      DIRENT_SET_DP_INO (dp, d_ino);
-	      dp->d_off = d_off;
-	      if ((sizeof (dp->d_ino) != sizeof (kdp->d_ino)
-		   && dp->d_ino != d_ino)
-		  || (sizeof (dp->d_off) != sizeof (kdp->d_off)
-		      && test_overflow(d_off, sizeof(dp->d_off))))
+
+	      /* Copy the data out of the old structure into temporary space.
+		 Then copy the name, which may overlap if BUF == KBUF.  */
+	      const uint64_t d_ino = inp->k.d_ino;
+	      const int64_t d_off = inp->k.d_off;
+	      const uint8_t d_type = inp->k.d_type;
+
+	      memmove (outp->u.d_name, inp->k.d_name,
+		       old_reclen - offsetof (struct kernel_dirent64, d_name));
+
+	      /* Now we have copied the data from INP and access only OUTP.  */
+
+	      DIRENT_SET_DP_INO (&outp->u, d_ino);
+	      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,
 		     otherwise signal overflow.  */
-		  if (dp != buf)
+		  if (last_offset != -1)
 		    {
 		      __lseek64 (fd, last_offset, SEEK_SET);
-		      return (char *) dp - buf;
+		      return outp->b - buf;
 		    }
 		  __set_errno (EOVERFLOW);
 		  return -1;
 		}
 
 	      last_offset = d_off;
-	      dp->d_reclen = new_reclen;
-	      dp->d_type = d_type;
-	      memmove (dp->d_name, kdp->d_name,
-		       old_reclen - offsetof (struct kernel_dirent64, d_name));
+	      outp->u.d_reclen = new_reclen;
+	      outp->u.d_type = d_type;
 
-	      dp = (DIRENT_TYPE *) ((char *) dp + new_reclen);
-	      kdp = (struct kernel_dirent64 *) ((char *) kdp + old_reclen);
+	      inp = (void *) inp + old_reclen;
+	      outp = (void *) outp + new_reclen;
 	    }
 
-	  return (char *) dp - buf;
+	  return outp->b - buf;
 	}
 
 # ifndef __ASSUME_GETDENTS64_SYSCALL
@@ -199,7 +210,6 @@
   {
     size_t red_nbytes;
     struct kernel_dirent *skdp, *kdp;
-    __kernel_off_t last_offset = 0;
     const size_t size_diff = (offsetof (DIRENT_TYPE, d_name)
 			      - offsetof (struct kernel_dirent, d_name));
 
@@ -208,7 +218,6 @@
 			 * size_diff),
 		      nbytes - size_diff);
 
-    dp = (DIRENT_TYPE *) buf;
     skdp = kdp = __alloca (red_nbytes);
 
     retval = INLINE_SYSCALL (getdents, 3, fd,
@@ -217,6 +226,7 @@
     if (retval == -1)
       return -1;
 
+    DIRENT_TYPE *dp = (DIRENT_TYPE *) buf;
     while ((char *) kdp < (char *) skdp + retval)
       {
 	const size_t alignment = __alignof__ (DIRENT_TYPE);
@@ -228,7 +238,7 @@
 	  {
 	    /* Our heuristic failed.  We read too many entries.  Reset
 	       the stream.  */
-	    assert (dp != buf);
+	    assert (last_offset != -1);
 	    __lseek64 (fd, last_offset, SEEK_SET);
 
 	    if ((char *) dp == buf)
@@ -253,7 +263,7 @@
 	dp = (DIRENT_TYPE *) ((char *) dp + new_reclen);
 	kdp = (struct kernel_dirent *) (((char *) kdp) + kdp->d_reclen);
       }
-    }
 
-  return (char *) dp - buf;
+    return (char *) dp - buf;
+  }
 }