Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)

From: Bruno Haible <[email-masked]>
Date: Fri, 12 Oct 2018 18:17:52 +0200

David Wood wrote:
> >> At this point, me->me_dev contains a wrongly packed (32-bit) device 
> >> number, which forces the find_mount_point() code path (causing other 
> >> unpleasantries).  The following patch against coreutils v8.5 fixes the 
> >> problem:

This problem description was to the point, but I needed a bit of time to
understand the issue entirely.

On Solaris, gnulib's mountlist module proceeds by reading the /etc/mnttab
file.

https://docs.oracle.com/cd/E86824_01/html/E54775/mnttab-4.html says:
  "The mnttab file system provides the previously undocumented dev=xxx option
   in the option string for each mounted file system. This is provided for
   legacy applications that might have been using the dev=information option.

   Using dev=option in applications is strongly discouraged. The device number
   string represents a 32-bit quantity and might not contain correct
   information in 64-bit environments.

   Applications requiring device number information for mounted file systems
   should use the getextmntent(3C) interface, which functions properly in
   either 32- or 64-bit environments."

The 'stat' program displays a dev_t. For example, for '/':
A 32-bit 'stat': 4750002     = (0x11d << 18) + (0x10002 << 0)
A 64-bit 'stat': 11d00010002 = (0x11d << 32) + (0x10002 << 0)

So, device numbers in a 32-bit program and in a 64-bit program are
different!

Additionally, reading /etc/mnttab produces the same(!) result when
done by a 64-bit program as by a 32-bit program. The approach that
converts the dev=... strings found in /etc/mnttab therefore produces
dev_t values according to 32-bit programs, even in a 64-bit program.

Now comes GNU 'df' which, as David noted, has logic to compare the
two dev_t values:
./src/df.c:1371:              me->me_dev = disk_stats.st_dev;
./src/df.c:1388:        if (statp->st_dev == me->me_dev
./src/df.c:1394:                || disk_stats.st_dev != me->me_dev)

So, really, we need to avoid the wrongly encoded dev_t values, and
this means to follow the advice from the mnttab.4 man page.

2018-10-12  Bruno Haible  <[email-masked]>

	mountlist: Improve support for Solaris in 64-bit mode.
	Reported by David Wood <[email-masked]> in
	<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=6816>.
	* m4/ls-mntd-fs.m4 (gl_LIST_MOUNTED_FILE_SYSTEMS): On Solaris 8 or
	newer, define MOUNTED_GETEXTMNTENT instead of MOUNTED_GETMNTENT2.
	* lib/mountlist.c: Add code for MOUNTED_GETEXTMNTENT case.

diff --git a/lib/mountlist.c b/lib/mountlist.c
index 970c611..845c348 100644
--- a/lib/mountlist.c
+++ b/lib/mountlist.c
@@ -111,7 +111,11 @@
 # include <mntent.h>
 #endif
 
-#ifdef MOUNTED_GETMNTENT2       /* Solaris, also (obsolete) SVR4 */
+#ifdef MOUNTED_GETEXTMNTENT     /* Solaris >= 8 */
+# include <sys/mnttab.h>
+#endif
+
+#ifdef MOUNTED_GETMNTENT2       /* Solaris < 8, also (obsolete) SVR4 */
 # include <sys/mnttab.h>
 #endif
 
@@ -918,10 +922,55 @@ read_file_system_list (bool need_fs_type)
   }
 #endif /* MOUNTED_GETMNTTBL */
 
-#ifdef MOUNTED_GETMNTENT2       /* Solaris, also (obsolete) SVR4 */
+#ifdef MOUNTED_GETEXTMNTENT     /* Solaris >= 8 */
+  {
+    struct extmnttab mnt;
+    const char *table = MNTTAB;
+    FILE *fp;
+    int ret;
+
+    /* No locking is needed, because the contents of /etc/mnttab is generated
+       by the kernel.  */
+
+    errno = 0;
+    fp = fopen (table, "r");
+    if (fp == NULL)
+      ret = errno;
+    else
+      {
+        while ((ret = getextmntent (fp, &mnt, 1)) == 0)
+          {
+            me = xmalloc (sizeof *me);
+            me->me_devname = xstrdup (mnt.mnt_special);
+            me->me_mountdir = xstrdup (mnt.mnt_mountp);
+            me->me_mntroot = NULL;
+            me->me_type = xstrdup (mnt.mnt_fstype);
+            me->me_type_malloced = 1;
+            me->me_dummy = MNT_IGNORE (&mnt) != 0;
+            me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
+            me->me_dev = makedev (mnt.mnt_major, mnt.mnt_minor);
+
+            /* Add to the linked list. */
+            *mtail = me;
+            mtail = &me->me_next;
+          }
+
+        ret = fclose (fp) == EOF ? errno : 0 < ret ? 0 : -1;
+        /* Here ret = -1 means success, ret >= 0 means failure.  */
+      }
+
+    if (0 <= ret)
+      {
+        errno = ret;
+        goto free_then_fail;
+      }
+  }
+#endif /* MOUNTED_GETMNTTBL */
+
+#ifdef MOUNTED_GETMNTENT2       /* Solaris < 8, also (obsolete) SVR4 */
   {
     struct mnttab mnt;
-    char *table = MNTTAB;
+    const char *table = MNTTAB;
     FILE *fp;
     int ret;
     int lockfd = -1;
@@ -979,6 +1028,7 @@ read_file_system_list (bool need_fs_type)
           }
 
         ret = fclose (fp) == EOF ? errno : 0 < ret ? 0 : -1;
+        /* Here ret = -1 means success, ret >= 0 means failure.  */
       }
 
     if (0 <= lockfd && close (lockfd) != 0)
diff --git a/m4/ls-mntd-fs.m4 b/m4/ls-mntd-fs.m4
index ff688f5..643d0ce 100644
--- a/m4/ls-mntd-fs.m4
+++ b/m4/ls-mntd-fs.m4
@@ -158,7 +158,23 @@ yes
     fi
 
     if test -z "$ac_list_mounted_fs"; then
-      # Solaris, also (obsolete) SVR4.
+      # Solaris >= 8.
+      AC_CACHE_CHECK([for getextmntent function],
+        [fu_cv_sys_mounted_getextmntent],
+        [AC_EGREP_HEADER([getextmntent], [sys/mnttab.h],
+           [fu_cv_sys_mounted_getextmntent=yes],
+           [fu_cv_sys_mounted_getextmntent=no])])
+      if test $fu_cv_sys_mounted_getextmntent = yes; then
+        ac_list_mounted_fs=found
+        AC_DEFINE([MOUNTED_GETEXTMNTENT], [1],
+          [Define if there is a function named getextmntent for reading the list
+           of mounted file systems.  (Solaris)])
+      fi
+    fi
+
+    if test -z "$ac_list_mounted_fs"; then
+      # Solaris < 8, also (obsolete) SVR4.
+      # Solaris >= 8 has the two-argument getmntent but is already handled above.
       AC_CACHE_CHECK([for two-argument getmntent function],
         [fu_cv_sys_mounted_getmntent2],
         [AC_EGREP_HEADER([getmntent], [sys/mnttab.h],