[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#916276: glibc: Please add prelimenary patch to fix regression on qemu-user



Source: glibc
Version: 2.28-2
Severity: important
Tags: patch
User: debian-68k@lists.debian.org
Usertags: m68k

Hi!

In 298d0e3129c0b5137f4989275b13fe30d0733c4d ("Consolidate Linux getdents{64}
implementation"), upstream made changes to the getdents{64} implementation
which breaks glibc on qemu-user.

The result of that is that applications like dash behave erratically, such that
for example pattern expansion no longer works. This means that "*" is not expanded
to filenames:

$ ls -l old/
total 148
-rw-r--r-- 1 glaubitz fbedv 16188 Jun 21  2003 chardraw.c
-rw-r--r-- 1 glaubitz fbedv  7922 Jun 21  2003 fillpoly.c
-rw-r--r-- 1 glaubitz fbedv   948 Jun 21  2003 readme
-rw-r--r-- 1 glaubitz fbedv 31860 Jun 21  2003 to_atari.c
-rw-r--r-- 1 glaubitz fbedv 13093 Jun 21  2003 to_eps.c
-rw-r--r-- 1 glaubitz fbedv  6631 Jun 21  2003 to_mf.c
-rw-r--r-- 1 glaubitz fbedv  3261 Jun 21  2003 to_pbm.c
-rw-r--r-- 1 glaubitz fbedv 12854 Jun 21  2003 to_pcx.c
-rw-r--r-- 1 glaubitz fbedv 10657 Jun 21  2003 to_pdf.c
-rw-r--r-- 1 glaubitz fbedv  6697 Jun 21  2003 to_pm.c
-rw-r--r-- 1 glaubitz fbedv  9463 Jun 21  2003 to_x11a.c
-rw-r--r-- 1 glaubitz fbedv 10405 Jun 21  2003 to_x11.c
$ ls -l old/*
/bin/ls: cannot access 'old/*': No such file or directory
$

dash is just one example. Other affected applications are cmake or qmake.
The attached prelimenary patch by James Clarke fixes the problem for me.

Thanks,
Adrian

> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=23960

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Description: Fix regression of glibc 2.28 on qemu-user
 The new getdents{64} implementation in glibc 2.28 breaks
 qemu-user making certain applications like dash fail to
 work properly.
 .
Author: James Clarke <jrtc27@jrtc27.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23960
Last-Update: 2018-12-11

Index: glibc-2.28/sysdeps/unix/sysv/linux/alpha/getdents.c
===================================================================
--- glibc-2.28.orig/sysdeps/unix/sysv/linux/alpha/getdents.c
+++ glibc-2.28/sysdeps/unix/sysv/linux/alpha/getdents.c
@@ -1,11 +1,13 @@
 /* Although Alpha defines _DIRENT_MATCHES_DIRENT64, 'struct dirent' and
    'struct dirent64' have slight different internal layout with d_ino
    being a __ino_t on non-LFS version with an extra __pad field which should
-   be zeroed.  */
+   be zero.  */
 
 #include <dirent.h>
 #undef _DIRENT_MATCHES_DIRENT64
 #define _DIRENT_MATCHES_DIRENT64 0
-#define DIRENT_SET_DP_INO(dp, value) \
-  do { (dp)->d_ino = (value); (dp)->__pad = 0; } while (0)
+#define DIRENT_CHECK_DP_INO_SIZE(kdp, dp) \
+  (sizeof ((kdp)->d_ino) == sizeof ((dp)->d_ino) + sizeof ((dp)->__pad))
+#define DIRENT_CHECK_DP_INO(dp) \
+  (dp)->__pad == 0
 #include <sysdeps/unix/sysv/linux/getdents.c>
Index: glibc-2.28/sysdeps/unix/sysv/linux/getdents.c
===================================================================
--- glibc-2.28.orig/sysdeps/unix/sysv/linux/getdents.c
+++ glibc-2.28/sysdeps/unix/sysv/linux/getdents.c
@@ -24,92 +24,85 @@
 # include <string.h>
 # include <errno.h>
 
-# ifndef DIRENT_SET_DP_INO
-#  define DIRENT_SET_DP_INO(dp, value) (dp)->d_ino = (value)
+/* For Linux we need a special version of this file since the
+   definition of `struct dirent' is not the same for the kernel and
+   the libc; the kernel places d_type after the variable-length d_name,
+   but we place it at a fixed offset just before d_name.  */
+
+struct kernel_dirent
+  {
+    __syscall_ulong_t d_ino;
+    __syscall_ulong_t d_off;
+    unsigned short d_reclen;
+    char d_name[256];
+  };
+
+# ifndef DIRENT_CHECK_DP_INO_SIZE
+#  define DIRENT_CHECK_DP_INO_SIZE(kdp, dp) \
+    (sizeof ((kdp)->d_ino) == sizeof ((dp)->d_ino))
 # endif
 
-/* Pack the dirent64 struct down into 32-bit offset/inode fields, and
-   ensure that no overflow occurs.  */
 ssize_t
 __getdents (int fd, char *buf, size_t nbytes)
 {
-  union
-  {
-    /* For !_DIRENT_MATCHES_DIRENT64 kernel 'linux_dirent64' has the same
-       layout of 'struct dirent64'.  */
-    struct dirent64 k;
-    struct dirent u;
-    char b[1];
-  } *kbuf = (void *) buf, *outp, *inp;
-  size_t kbytes = nbytes;
-  off64_t last_offset = -1;
   ssize_t retval;
+# ifdef DIRENT_CHECK_DP_INO
+  off64_t last_offset = -1;
+# endif
 
-# define size_diff (offsetof (struct dirent64, d_name) \
-		    - offsetof (struct dirent, d_name))
-  char kbuftmp[sizeof (struct dirent) + size_diff];
-  if (nbytes <= sizeof (struct dirent))
-    kbuf = (void*) kbuftmp;
-
-  retval = INLINE_SYSCALL_CALL (getdents64, fd, kbuf, kbytes);
-  if (retval == -1)
-    return -1;
-
-  /* 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)
+  /* The d_ino and d_off fields in kernel_dirent and dirent must have
+     the same sizes and alignments.  */
+  _Static_assert (DIRENT_CHECK_DP_INO_SIZE ((struct kernel_dirent *) 0,
+					    (struct dirent *) 0), "size and alignment");
+  _Static_assert (sizeof (((struct kernel_dirent *) 0)->d_off)
+	  == sizeof (((struct dirent *) 0)->d_off), "size and alignment");
+  _Static_assert (offsetof (struct kernel_dirent, d_off)
+	  == offsetof (struct dirent, d_off), "size and alignment");
+  _Static_assert (offsetof (struct kernel_dirent, d_reclen)
+	  == offsetof (struct dirent, d_reclen), "size and alignment");
+
+  retval = INLINE_SYSCALL (getdents, 3, fd, buf, nbytes);
+
+  /* The kernel added the d_type value after the name.  Change
+     this now. Also fix up d_ino as required for e.g. Alpha.  */
+  if (retval != -1)
     {
-      const size_t alignment = _Alignof (struct dirent);
-      /* 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 = inp->k.d_reclen;
-      size_t new_reclen = ((old_reclen - size_diff + alignment - 1)
-                           & ~(alignment - 1));
-
-      /* 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 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 (last_offset != -1)
-            {
-              __lseek64 (fd, last_offset, SEEK_SET);
-              return outp->b - buf;
-            }
-	  return INLINE_SYSCALL_ERROR_RETURN_VALUE (EOVERFLOW);
-        }
-
-      last_offset = d_off;
-      outp->u.d_reclen = new_reclen;
-      outp->u.d_type = d_type;
+      union
+      {
+	struct kernel_dirent k;
+	struct dirent u;
+      } *kbuf = (void *) buf;
+
+      while ((char *) kbuf < buf + retval)
+	{
+	  char d_type = *((char *) kbuf + kbuf->k.d_reclen - 1);
+	  memmove (kbuf->u.d_name, kbuf->k.d_name,
+		   strlen (kbuf->k.d_name) + 1);
+	  kbuf->u.d_type = d_type;
+
+	  /* Alpha only has a 32-bit d_ino, followed by a 32-bit __pad field,
+	   * so check we haven't overflowed.  */
+# ifdef DIRENT_CHECK_DP_INO
+	  if (DIRENT_CHECK_DP_INO (&kbuf->u))
+	    {
+	      /* Overflow.  If there was at least one entry before this one,
+		 return them without error, otherwise signal overflow.  */
+	      if (last_offset != -1)
+		{
+		    __lseek64 (fd, last_offset, SEEK_SET);
+		    return (char *) kbuf - buf;
+		}
+	      return INLINE_SYSCALL_ERROR_RETURN_VALUE (EOVERFLOW);
+	    }
 
-      inp = (void *) inp + old_reclen;
-      outp = (void *) outp + new_reclen;
+	  last_offset = kbuf->u.d_off;
+#endif
+
+	  kbuf = (void *) ((char *) kbuf + kbuf->k.d_reclen);
+	}
     }
 
-  return outp->b - buf;
+  return retval;
 }
 
-# undef DIRENT_SET_DP_INO
-
-#endif /* _DIRENT_MATCHES_DIRENT64  */
+#endif /* !_DIRENT_MATCHES_DIRENT64  */

Reply to: