Bug in coreutils-4.5.9 "df" under AIX 5.1 -- found and fixed

[Mike Jetzer]

I'd like to report several problems I encountered while
compiling coreutils-4.5.9 under AIX 5.1.  The company for
which I work has a product which works under various
versions of Unix.  For one support utility, we use the GNU
version of "df", as df's output varies from one vendor to
another.

The actual bug to report is that, under certain
installations of AIX 5.1, df will hang for  a period of
time (about 4 1/2 minutes on one machine) and then report
"memory exhausted."  This behavior is exhibited on at least
two machines (one in-house machine and one customer
machine), but is not exhibited on another machine we have
in-house.

The version we were using was somewhat outdated
(fileutils-4.1), so when we ran into this bug during a
customer go-live, I fetched coreutils-4.5.9 to see if the
bug was still present.  It was, so I did sufficient
investigation to identify and correct the bug.

First, I can report a few problems compiling coreutils (I'm
compiling with IBM's xlc "C for AIX Compiler, Version 5",
with "export CC=xlc"):
    - Apparently, configure does its test for getmntent
      before it does its test for vmount.  Since AIX 5.1
      has both getmntent and vmount, config.status winds up
      with "MOUNTED_GETMNTENT1" defined rather than
      "MOUNTED_VMOUNT" (when compiled with
      MOUNTED_GETMNTENT1, df "mostly" works, but reports
      NFS-mounted filesystems as though they were locally
      mounted).  So, when we compiled, I manually edited
      config.status, changed MOUNTED_GETMENTENT1 to
      MOUNTED_VMOUNT on the two lines it occurred, and
      continued from there.

      I have no idea how to correct this deficiency.

    - lib/ftw.c #defines STAT; under AIX 5.1, <sys/dir.h>
      also #defines STAT, to an incompatible value.  I
      changed all occurrences of STAT in lib/ftw.c to be
      "FTW_STAT", which seems to have solved the problem.

      This same change also had to be applied to get
      lib/ftw.c to compile under AIX 4.1 (using xlc "C for
      AIX Compiler").

    - src/fmt.c gets an error of the following form at
      lines 137, 138, 139, and 140 (with VARIABLE taking
      the values of "paren", "period", "punct", and
      "final"):
          Bit-field VARIABLE must be of type signed int,
          unsigned int or int.
      These variables are defined as
          bool paren:1;
      Since src/system.h defines "bool" as
          typedef enum {false = 0, true = 1} bool;
      it appears that the c compiler doesn't treat enums as
      ints for the purpose of bit fields.

      Again, I don't know how to solve this; since the
      company for which I work is only interested in "df",
      I simply edited the src/Makefile to only compile
      "df".


Which brings me to the bug we observed in "df".

The bug is actually in lib/mountlist.c'c
read_filesystem_list() implementation when MOUNTED_VMOUNT
is defined.

According to
http://publibn.boulder.ibm.com/doc_link/en_US/a_doc_lib/libs/basetrf1/mntctl
.htm
mntctl() returns the number of structures copied into the
passed buffer if the passed buffer is large enough to hold
the structures.

However, the code ignores this documented behavior and
assumes that a pointer to the last structure, plus the
contents of the last structure's vmt_length, will be equal
to the end of the buffer.

For some machines, this is not the case.  Rather, there are
(in at least one test) 200-some-odd-bytes of zeros at the
end of the buffer.  This causes the "for" loop to never
terminate, eventually exhausting memory.

The solution is simply enough: remove the pointer
arithmetic as the loop's controlling expression and replace
it with a simple count.

Here's the modified code which seems to work correctly
(sorry, not familiar with making patch-style diffs):


#ifdef MOUNTED_VMOUNT           /* AIX.  */
  {
    int bufsize;
    char *entries, *thisent;
    struct vmount *vmp;
    int total_num, num_processed;

    /* Ask how many bytes to allocate for the mounted filesystem info.  */
    mntctl (MCTL_QUERY, sizeof bufsize, (struct vmount *) &bufsize);
    entries = xmalloc (bufsize);

    /* Get the list of mounted filesystems.  */
    total_num = mntctl (MCTL_QUERY, bufsize, (struct vmount *) entries);

    num_processed = 0;
    for (thisent = entries; num_processed < total_num;
         thisent += vmp->vmt_length)
      {
        char *options, *ignore;

        num_processed++;

        vmp = (struct vmount *) thisent;
        me = (struct mount_entry *) xmalloc (sizeof (struct mount_entry));
        if (vmp->vmt_flags & MNT_REMOTE)
          {
            char *host, *path;

            me->me_remote = 1;
            /* Prepend the remote pathname.  */
            host = thisent + vmp->vmt_data[VMT_HOSTNAME].vmt_off;
            path = thisent + vmp->vmt_data[VMT_OBJECT].vmt_off;
            me->me_devname = xmalloc (strlen (host) + strlen (path) + 2);
            strcpy (me->me_devname, host);
            strcat (me->me_devname, ":");
            strcat (me->me_devname, path);
          }
        else
          {
            me->me_remote = 0;
            me->me_devname = xstrdup (thisent +
                                      vmp->vmt_data[VMT_OBJECT].vmt_off);
          }
        me->me_mountdir = xstrdup (thisent +
vmp->vmt_data[VMT_STUB].vmt_off);
        me->me_type = xstrdup (fstype_to_string (vmp->vmt_gfstype));
        options = thisent + vmp->vmt_data[VMT_ARGS].vmt_off;
        ignore = strstr (options, "ignore");
        me->me_dummy = (ignore
                        && (ignore == options || ignore[-1] == ',')
                        && (ignore[sizeof "ignore" - 1] == ','
                            || ignore[sizeof "ignore" - 1] == '\0'));
        me->me_dev = (dev_t) -1; /* vmt_fsid might be the info we want.  */

        /* Add to the linked list. */
        *mtail = me;
        mtail = &me->me_next;
      }
    free (entries);
  }
#endif /* MOUNTED_VMOUNT. */

Here's more information regarding mntctl() (from the link above):
    Return Values

    If the mntctl subroutine is successful, the number of
    vmount structures copied into the Buffer parameter is
    returned. If the Size parameter indicates the supplied
    buffer is too small to hold the vmount structures for
    all the current VFSs, the mntctl subroutine sets the
    first word of the Buffer parameter to the required size
    (in bytes) and returns the value 0. If the mntctl
    subroutine otherwise fails, a value of -1 is returned,
    and the errno global variable is set to indicate the
    error.

    Error Codes

    The mntctl subroutine fails and the requested operation
    is not performed if one or both of the following are
    true:

    EINVAL The Command parameter is not MCTL_QUERY, or the
    Size parameter is not a positive value.

    EFAULT The Buffer parameter points to a location
    outside of the allocated address space of the process.

So, strictly speaking, mntctl() may return an error, but since
the original code happily ignored any errors, my change follows
in that fine tradition :-)


When compiling under HP-UX B.11.11 (using the ANSI c
compiler -- I can't determine its version using "-v" or
"-V"; the exact command line is "cc -Aa +DAportable
-D_HPUX_SOURCE", which is what we typically use for GNU
packages), the following error is returned
    cc: "exclude.c", line 177: error 1552: Incompatible
    types in second and third operands of conditional
    expression (?:).

This appears to be because the fnmatch() prototype in
libfnmatch.h is getting its __const keywords replaced with
the empty string, and thus does not match
fnmatch_no_wildcards() (even if __const were replaced with
"const", I don't know if the compiler would have complained
because fnmatch() takes two "const char *" arguments, while
fnmatch_no_wildcards() takes two "char const *".  To get
around this, I simply removed the "const" qualifiers from
fnmatch_no_wildcards().