[Bug-gnulib] mountlist patch for buffer size overflow on OSF/1 Alphas, etc.

[Paul Eggert]

I noticed a potential (albeit highly unlikely) size overflow in
calculating a buffer size on OSF/1 Alphas in mountlist.c.  While I was
fixing this, I also fixed a FIXME (me_type wasn't freed) and used a
brief style for mallocs; if this is not the style you like please let
me know and I can back out that part of the change.  I installed the
following:

2003-10-31  Paul Eggert  <address@hidden>

        * mountlist.h (struct mount_entry.me_type_malloced): New member.
        * mountlist.c (SIZE_MAX): Define if not defined already.
        (read_filesystem_list): Set and use me_type_malloced.
        Use "sizeof *me" rather than "sizeof (struct mount_entry)" (or
        whatever the type happens to be), for brevity and consistency.
        Check for size calculation overflow on Alphas running OSF/1.

Index: lib/mountlist.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/mountlist.h,v
retrieving revision 1.10
diff -p -u -r1.10 mountlist.h
--- lib/mountlist.h     18 Jun 2003 05:52:19 -0000      1.10
+++ lib/mountlist.h     1 Nov 2003 05:52:14 -0000
@@ -26,6 +26,7 @@ struct mount_entry
   dev_t me_dev;                        /* Device number of me_mountdir. */
   unsigned int me_dummy : 1;   /* Nonzero for dummy filesystems. */
   unsigned int me_remote : 1;  /* Nonzero for remote fileystems. */
+  unsigned int me_type_malloced : 1; /* Nonzero if me_type was malloced. */
   struct mount_entry *me_next;
 };
 
Index: lib/mountlist.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/mountlist.c,v
retrieving revision 1.43
diff -p -u -r1.43 mountlist.c
--- lib/mountlist.c     17 Sep 2003 12:46:33 -0000      1.43
+++ lib/mountlist.c     1 Nov 2003 05:52:14 -0000
@@ -26,6 +26,10 @@
 
 #include "xalloc.h"
 
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+
 #ifndef strstr
 char *strstr ();
 #endif
@@ -317,10 +321,11 @@ read_filesystem_list (int need_fs_type)
       return NULL;
     for (p = mntlist; p; p = p->next) {
       mnt = p->ment;
-      me = xmalloc (sizeof (struct mount_entry));
+      me = xmalloc (sizeof *me);
       me->me_devname = xstrdup (mnt->mnt_fsname);
       me->me_mountdir = xstrdup (mnt->mnt_dir);
       me->me_type = xstrdup (mnt->mnt_type);
+      me->me_type_malloced = 1;
       me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
       me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
       me->me_dev = -1;
@@ -344,10 +349,11 @@ read_filesystem_list (int need_fs_type)
 
     while ((mnt = getmntent (fp)))
       {
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
        me->me_devname = xstrdup (mnt->mnt_fsname);
        me->me_mountdir = xstrdup (mnt->mnt_dir);
        me->me_type = xstrdup (mnt->mnt_type);
+       me->me_type_malloced = 1;
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
        me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
        devopt = strstr (mnt->mnt_opts, "dev=");
@@ -383,10 +389,11 @@ read_filesystem_list (int need_fs_type)
       {
        char *fs_type = fsp_to_string (fsp);
 
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
        me->me_devname = xstrdup (fsp->f_mntfromname);
        me->me_mountdir = xstrdup (fsp->f_mntonname);
        me->me_type = fs_type;
+       me->me_type_malloced = 0;
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
        me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
        me->me_dev = (dev_t) -1;        /* Magic; means not known yet. */
@@ -408,10 +415,11 @@ read_filesystem_list (int need_fs_type)
           0 < (val = getmnt (&offset, &fsd, sizeof (fsd), NOSTAT_MANY,
                              (char *) 0)))
       {
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
        me->me_devname = xstrdup (fsd.fd_req.devname);
        me->me_mountdir = xstrdup (fsd.fd_req.path);
        me->me_type = gt_names[fsd.fd_req.fstype];
+       me->me_type_malloced = 0;
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
        me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
        me->me_dev = fsd.fd_req.dev;
@@ -478,9 +486,7 @@ read_filesystem_list (int need_fs_type)
 
             if (lstat (name, &statbuf) >= 0 && S_ISDIR (statbuf.st_mode))
               {
-                struct rootdir_entry *re;
-
-                re = xmalloc (sizeof (struct rootdir_entry));
+               struct rootdir_entry *re = xmalloc (sizeof *re);
                 re->name = name;
                 re->dev = statbuf.st_dev;
                 re->ino = statbuf.st_ino;
@@ -506,10 +512,11 @@ read_filesystem_list (int need_fs_type)
             if (re->dev == fi.dev && re->ino == fi.root)
               break;
 
-          me = xmalloc (sizeof (struct mount_entry));
+         me = xmalloc (sizeof *me);
           me->me_devname = xstrdup (fi.device_name[0] != '\0' ? fi.device_name 
: fi.fsh_name);
           me->me_mountdir = xstrdup (re != NULL ? re->name : fi.fsh_name);
           me->me_type = xstrdup (fi.fsh_name);
+         me->me_type_malloced = 1;
           me->me_dev = fi.dev;
           me->me_dummy = 0;
           me->me_remote = (fi.flags & B_FS_IS_SHARED) != 0;
@@ -532,14 +539,17 @@ read_filesystem_list (int need_fs_type)
 
 #if defined MOUNTED_GETFSSTAT  /* __alpha running OSF_1 */
   {
-    int numsys, counter, bufsize;
+    int numsys, counter;
+    size_t bufsize;
     struct statfs *stats;
 
     numsys = getfsstat ((struct statfs *)0, 0L, MNT_NOWAIT);
     if (numsys < 0)
       return (NULL);
+    if (SIZE_MAX / sizeof *stats <= numsys)
+      xalloc_die ();
 
-    bufsize = (1 + numsys) * sizeof (struct statfs);
+    bufsize = (1 + numsys) * sizeof *stats;
     stats = xmalloc (bufsize);
     numsys = getfsstat (stats, bufsize, MNT_NOWAIT);
 
@@ -551,10 +561,11 @@ read_filesystem_list (int need_fs_type)
 
     for (counter = 0; counter < numsys; counter++)
       {
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
        me->me_devname = xstrdup (stats[counter].f_mntfromname);
        me->me_mountdir = xstrdup (stats[counter].f_mntonname);
        me->me_type = xstrdup (FS_TYPE (stats[counter]));
+       me->me_type_malloced = 1;
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
        me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
        me->me_dev = (dev_t) -1;        /* Magic; means not known yet. */
@@ -580,7 +591,7 @@ read_filesystem_list (int need_fs_type)
 
     while (fread (&mnt, sizeof mnt, 1, fp) > 0)
       {
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
 # ifdef GETFSTYP                       /* SVR3.  */
        me->me_devname = xstrdup (mnt.mt_dev);
 # else
@@ -591,6 +602,7 @@ read_filesystem_list (int need_fs_type)
        me->me_mountdir = xstrdup (mnt.mt_filsys);
        me->me_dev = (dev_t) -1;        /* Magic; means not known yet. */
        me->me_type = "";
+       me->me_type_malloced = 0;
 # ifdef GETFSTYP                       /* SVR3.  */
        if (need_fs_type)
          {
@@ -599,7 +611,10 @@ read_filesystem_list (int need_fs_type)
 
            if (statfs (me->me_mountdir, &fsd, sizeof fsd, 0) != -1
                && sysfs (GETFSTYP, fsd.f_fstyp, typebuf) != -1)
-             me->me_type = xstrdup (typebuf);
+             {
+               me->me_type = xstrdup (typebuf);
+               me->me_type_malloced = 1;
+             }
          }
 # endif
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
@@ -629,10 +644,11 @@ read_filesystem_list (int need_fs_type)
     struct mntent **mnttbl = getmnttbl (), **ent;
     for (ent=mnttbl;*ent;ent++)
       {
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
        me->me_devname = xstrdup ( (*ent)->mt_resource);
        me->me_mountdir = xstrdup ( (*ent)->mt_directory);
-       me->me_type =  xstrdup ((*ent)->mt_fstype);
+       me->me_type = xstrdup ((*ent)->mt_fstype);
+       me->me_type_malloced = 1;
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
        me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
        me->me_dev = (dev_t) -1;        /* Magic; means not known yet. */
@@ -690,10 +706,11 @@ read_filesystem_list (int need_fs_type)
       {
        while ((ret = getmntent (fp, &mnt)) == 0)
          {
-           me = xmalloc (sizeof (struct mount_entry));
+           me = xmalloc (sizeof *me);
            me->me_devname = xstrdup (mnt.mnt_special);
            me->me_mountdir = xstrdup (mnt.mnt_mountp);
            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 = (dev_t) -1;    /* Magic; means not known yet. */
@@ -747,7 +764,7 @@ read_filesystem_list (int need_fs_type)
        char *options, *ignore;
 
        vmp = (struct vmount *) thisent;
-       me = xmalloc (sizeof (struct mount_entry));
+       me = xmalloc (sizeof *me);
        if (vmp->vmt_flags & MNT_REMOTE)
          {
            char *host, *path;
@@ -769,6 +786,7 @@ read_filesystem_list (int need_fs_type)
          }
        me->me_mountdir = xstrdup (thisent + vmp->vmt_data[VMT_STUB].vmt_off);
        me->me_type = xstrdup (fstype_to_string (vmp->vmt_gfstype));
+       me->me_type_malloced = 1;
        options = thisent + vmp->vmt_data[VMT_ARGS].vmt_off;
        ignore = strstr (options, "ignore");
        me->me_dummy = (ignore
@@ -799,7 +817,8 @@ read_filesystem_list (int need_fs_type)
        me = mount_list->me_next;
        free (mount_list->me_devname);
        free (mount_list->me_mountdir);
-       /* FIXME: me_type is not always malloced.  */
+       if (mount_list->me_type_malloced)
+         free (mount_list->me_type);
        free (mount_list);
        mount_list = me;
       }