GNU bug report logs - #6816
df bug on 64-bit Solaris (need to use getextmntent)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6816 in the body.
You can then email your comments to 6816 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Wood, David" <David.Wood <at> deshaw.com>
To: "bug-coreutils <at> gnu.org" <bug-coreutils <at> gnu.org>
Subject: df bug on 64-bit Solaris (need to use getextmntent)
Date: Fri, 6 Aug 2010 15:56:07 -0400

Hello,

From mnttab(4) on Solaris 10:

     ...

     WARNINGS

     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 environ-
     ments.

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

Indeed, if one compiles coreutils 64-bit, the logic breaks down around line 718 of df.c:

        if (statp->st_dev == me->me_dev
            && !STREQ (me->me_type, "lofs")
            && (!best_match || best_match->me_dummy || !me->me_dummy))
          {

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:

--- mountlist.c 3 Aug 2010 21:53:14 -0000       1.1.1.4
+++ mountlist.c 6 Aug 2010 18:16:55 -0000       1.2
@@ -107,6 +107,10 @@
 # include <sys/mnttab.h>
 #endif

+#ifdef MOUNTED_GETEXTMNTENT     /* need makedev when using getextmntent */
+# include <sys/mkdev.h>
+#endif
+
 #ifdef MOUNTED_VMOUNT           /* AIX.  */
 # include <fshelp.h>
 # include <sys/vfs.h>
@@ -125,7 +129,8 @@

 #undef MNT_IGNORE
 #if defined MNTOPT_IGNORE && defined HAVE_HASMNTOPT
-# define MNT_IGNORE(M) hasmntopt (M, MNTOPT_IGNORE)
+/* cast to a mnttab struct pointer to also support extmnttab */
+# define MNT_IGNORE(M) hasmntopt ((struct mnttab *) M, MNTOPT_IGNORE)
 #else
 # define MNT_IGNORE(M) 0
 #endif
@@ -714,7 +719,11 @@

 #ifdef MOUNTED_GETMNTENT2       /* SVR4.  */
   {
+# ifdef MOUNTED_GETEXTMNTENT
+    struct extmnttab mnt;
+# else
     struct mnttab mnt;
+# endif
     char *table = MNTTAB;
     FILE *fp;
     int ret;
@@ -755,7 +764,11 @@
       ret = errno;
     else
       {
+# ifdef MOUNTED_GETEXTMNTENT
+        while ((ret = getextmntent (fp, &mnt, sizeof (struct extmnttab))) == 0)
+# else
         while ((ret = getmntent (fp, &mnt)) == 0)
+# endif
           {
             me = xmalloc (sizeof *me);
             me->me_devname = xstrdup (mnt.mnt_special);
@@ -764,7 +777,11 @@
             me->me_type_malloced = 1;
             me->me_dummy = MNT_IGNORE (&mnt) != 0;
             me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
+# ifdef MOUNTED_GETEXTMNTENT
+            me->me_dev = makedev(mnt.mnt_major, mnt.mnt_minor);
+# else
             me->me_dev = dev_from_mount_options (mnt.mnt_mntopts);
+# endif

             /* Add to the linked list. */
             *mtail = me;

Message #8 received at 6816 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: "Wood, David" <David.Wood <at> deshaw.com>
Cc: bug-gnulib <bug-gnulib <at> gnu.org>, 6816 <at> debbugs.gnu.org
Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)
Date: Wed, 15 Sep 2010 16:18:37 -0600

[digging through older mail, and sorry for the delayed reply]

On 08/06/2010 01:56 PM, Wood, David wrote:
> Hello,
>
>> From mnttab(4) on Solaris 10:
>
>       Applications requiring device number information for mounted
>       file  systems  should  use  the  getextmntent(3C) interface,
>       which functions properly in either 32-  or  64-bit  environ-
>       ments.
>
> Indeed, if one compiles coreutils 64-bit, the logic breaks down around line 718 of df.c:
>
>          if (statp->st_dev == me->me_dev
>              &&  !STREQ (me->me_type, "lofs")
>              &&  (!best_match || best_match->me_dummy || !me->me_dummy))
>            {
>
> 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:

Thanks for the report.  Yes, this needs to be folded into gnulib, but it 
also needs an m4 check for getextmntent, and it would be nice to get by 
with less in-function #ifdefs.

>
> --- mountlist.c 3 Aug 2010 21:53:14 -0000       1.1.1.4
> +++ mountlist.c 6 Aug 2010 18:16:55 -0000       1.2
> @@ -107,6 +107,10 @@
>   # include<sys/mnttab.h>
>   #endif
>
> +#ifdef MOUNTED_GETEXTMNTENT     /* need makedev when using getextmntent */
> +# include<sys/mkdev.h>
> +#endif
> +
>   #ifdef MOUNTED_VMOUNT           /* AIX.  */
>   # include<fshelp.h>
>   # include<sys/vfs.h>
> @@ -125,7 +129,8 @@
>
>   #undef MNT_IGNORE
>   #if defined MNTOPT_IGNORE&&  defined HAVE_HASMNTOPT
> -# define MNT_IGNORE(M) hasmntopt (M, MNTOPT_IGNORE)
> +/* cast to a mnttab struct pointer to also support extmnttab */
> +# define MNT_IGNORE(M) hasmntopt ((struct mnttab *) M, MNTOPT_IGNORE)
>   #else
>   # define MNT_IGNORE(M) 0
>   #endif
> @@ -714,7 +719,11 @@
>
>   #ifdef MOUNTED_GETMNTENT2       /* SVR4.  */
>     {
> +# ifdef MOUNTED_GETEXTMNTENT
> +    struct extmnttab mnt;
> +# else
>       struct mnttab mnt;
> +# endif
>       char *table = MNTTAB;
>       FILE *fp;
>       int ret;
> @@ -755,7 +764,11 @@
>         ret = errno;
>       else
>         {
> +# ifdef MOUNTED_GETEXTMNTENT
> +        while ((ret = getextmntent (fp,&mnt, sizeof (struct extmnttab))) == 0)
> +# else
>           while ((ret = getmntent (fp,&mnt)) == 0)
> +# endif
>             {
>               me = xmalloc (sizeof *me);
>               me->me_devname = xstrdup (mnt.mnt_special);
> @@ -764,7 +777,11 @@
>               me->me_type_malloced = 1;
>               me->me_dummy = MNT_IGNORE (&mnt) != 0;
>               me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
> +# ifdef MOUNTED_GETEXTMNTENT
> +            me->me_dev = makedev(mnt.mnt_major, mnt.mnt_minor);
> +# else
>               me->me_dev = dev_from_mount_options (mnt.mnt_mntopts);
> +# endif
>
>               /* Add to the linked list. */
>               *mtail = me;
>
>
>
>

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Message #11 received at 6816 <at> debbugs.gnu.org (full text, mbox):

From: Assaf Gordon <assafgordon <at> gmail.com>
To: 6816 <at> debbugs.gnu.org
Cc: "bug-gnulib <at> gnu.org List" <bug-gnulib <at> gnu.org>
Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)
Date: Wed, 10 Oct 2018 11:24:22 -0600

(triaging old bugs)

Hello,

On 15/09/10 04:18 PM, Eric Blake wrote:
> On 08/06/2010 01:56 PM, Wood, David wrote:
>>> From mnttab(4) on Solaris 10:
>>
[...]
>>
>> 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:
> 
> Thanks for the report.  Yes, this needs to be folded into gnulib, but it 
> also needs an m4 check for getextmntent, and it would be nice to get by 
> with less in-function #ifdefs.
> 

In 2014 gnulib gained the following commit:
https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=502809019bd2ca3ce3d041d18c35ce9420eedb72
===
commit 502809019bd2ca3ce3d041d18c35ce9420eedb72
Author: Ben Walton <bdwalton <at> gmail.com>
Date:   Tue Jun 3 23:01:14 2014 +0100

    mountlist: avoid hasmntopt const type warning on solaris
===

Which seems to address a similar issue as this bug ( 
https://bugs.gnu.org/6816 ).

I think recent coreutils build well on 64bit solaris.
If there are no objections I will close this bug in couple of days.

regards,
 - assaf

Message #14 received at 6816 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Haible <bruno <at> clisp.org>
To: bug-gnulib <at> gnu.org
Cc: Assaf Gordon <assafgordon <at> gmail.com>, 6816 <at> debbugs.gnu.org
Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)
Date: Thu, 11 Oct 2018 01:54:40 +0200

Hi Assaf,

> In 2014 gnulib gained the following commit:
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=502809019bd2ca3ce3d041d18c35ce9420eedb72
> ===
> commit 502809019bd2ca3ce3d041d18c35ce9420eedb72
> Author: Ben Walton <bdwalton <at> gmail.com>
> Date:   Tue Jun 3 23:01:14 2014 +0100
> 
>      mountlist: avoid hasmntopt const type warning on solaris
> ===
> 
> Which seems to address a similar issue as this bug ( 
> https://bugs.gnu.org/6816 ).

I don't think it's the same bug. Gnulib still does not use getextmntent.

> I think recent coreutils build well on 64bit solaris.

According to the cited man page, the effect of not using getextmntent
is visible at run-time, not at compile-time.

> If there are no objections I will close this bug in couple of days.

Objection.

Bruno

Message #17 received at 6816 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Haible <bruno <at> clisp.org>
To: bug-gnulib <at> gnu.org
Cc: Assaf Gordon <assafgordon <at> gmail.com>, 6816 <at> debbugs.gnu.org,
 David.Wood <at> deshaw.com
Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)
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  <bruno <at> clisp.org>

	mountlist: Improve support for Solaris in 64-bit mode.
	Reported by David Wood <David.Wood <at> deshaw.com> 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],

Message #20 received at 6816 <at> debbugs.gnu.org (full text, mbox):

From: Assaf Gordon <assafgordon <at> gmail.com>
To: Bruno Haible <bruno <at> clisp.org>, bug-gnulib <at> gnu.org
Cc: 6816 <at> debbugs.gnu.org, David.Wood <at> deshaw.com
Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)
Date: Mon, 29 Oct 2018 23:01:44 -0600

Hello Bruno,

On 2018-10-12 10:17 a.m., Bruno Haible wrote:
> 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.
[...]
> 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  <bruno <at> clisp.org>
> 
> 	mountlist: Improve support for Solaris in 64-bit mode.
> 	Reported by David Wood <David.Wood <at> deshaw.com> 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.

Did this solve the issue (any reports from Solaris users)?
can this bug be closed?

-assaf

Message #23 received at 6816 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Haible <bruno <at> clisp.org>
To: Assaf Gordon <assafgordon <at> gmail.com>
Cc: bug-gnulib <at> gnu.org, 6816 <at> debbugs.gnu.org, David.Wood <at> deshaw.com
Subject: Re: bug#6816: df bug on 64-bit Solaris (need to use getextmntent)
Date: Tue, 30 Oct 2018 09:20:26 +0100

Hello Assaf,

> > 2018-10-12  Bruno Haible  <bruno <at> clisp.org>
> > 
> > 	mountlist: Improve support for Solaris in 64-bit mode.
> > 	Reported by David Wood <David.Wood <at> deshaw.com> 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.
> 
> Did this solve the issue (any reports from Solaris users)?
> can this bug be closed?

There was no "how-to-reproduce" recipe, nor did I receive feedback from the
reporter. But in my understanding the issue is fixed in the proper way.
Therefore yes, it can be closed.

Bruno

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.