mount: use fflush() and temporary file for mtab updates (CVE-2011-1089)

http://thread.gmane.org/gmane.comp.security.oss.general/4374

Changes:

  - force mount(8) to use /etc/mtab.tmp file every time. The original
    code used the tmp file for remount/move operations only.

  - call and check fflush() return code for the tmp file

Note mount(8) blocks all signals when writing to mtab, so it's not
affected by SIGXFSZ and the mtab lock file is always removed.

This patch does not fix the same issue in umount(8) and libmount.

Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/mount/fstab.c b/mount/fstab.c
index fd53ca4beb4e17ec1aee7555e94b280d3f180b4d..c04e9733e658ced4f5f71566a6d833c47557dd43 100644 (file)
--- a/mount/fstab.c
+++ b/mount/fstab.c
@@ -890,7 +890,7 @@ update_mtab (const char *dir, struct my_mntent *instead) {
        mntFILE *mfp, *mftmp;
        const char *fnam = _PATH_MOUNTED;
        struct mntentchn mtabhead;      /* dummy */
-       struct mntentchn *mc, *mc0, *absent = NULL;
+       struct mntentchn *mc, *mc0 = NULL, *absent = NULL;
        struct stat sbuf;
        int fd;
 
@@ -914,10 +914,12 @@ update_mtab (const char *dir, struct my_mntent *instead) {
        read_mntentchn(mfp, fnam, mc);
 
        /* find last occurrence of dir */
-       for (mc = mc0->prev; mc && mc != mc0; mc = mc->prev)
-               if (streq(mc->m.mnt_dir, dir))
-                       break;
-       if (mc && mc != mc0) {
+       if (dir) {
+               for (mc = mc0->prev; mc && mc != mc0; mc = mc->prev)
+                       if (streq(mc->m.mnt_dir, dir))
+                               break;
+       }
+       if (dir && mc && mc != mc0) {
                if (instead == NULL) {
                        /* An umount - remove entry */
                        if (mc && mc != mc0) {
@@ -963,24 +965,37 @@ update_mtab (const char *dir, struct my_mntent *instead) {
                int errsv = errno;
                error (_("cannot open %s (%s) - mtab not updated"),
                       _PATH_MOUNTED_TMP, strerror (errsv));
-               discard_mntentchn(mc0);
                goto leave;
        }
 
        for (mc = mc0->nxt; mc && mc != mc0; mc = mc->nxt) {
                if (my_addmntent(mftmp, &(mc->m)) == 1) {
                        int errsv = errno;
-                       die (EX_FILEIO, _("error writing %s: %s"),
+                       error(_("error writing %s: %s"),
                             _PATH_MOUNTED_TMP, strerror (errsv));
+                       goto leave;
                }
        }
 
        discard_mntentchn(mc0);
+       mc0 = NULL;
+
+       /*
+        * We have to be paranoid with write() to avoid incomplete
+        * /etc/mtab. Users are able to control writing by RLIMIT_FSIZE.
+        */
+       if (fflush(mftmp->mntent_fp) != 0) {
+               int errsv = errno;
+               error (_("%s: cannot fflush changes: %s"),
+                               _PATH_MOUNTED_TMP, strerror (errsv));
+               goto leave;
+       }
+
        fd = fileno(mftmp->mntent_fp);
 
        /*
-        * It seems that better is incomplete and broken /mnt/mtab that
-        * /mnt/mtab that is writeable for non-root users.
+        * It seems that better is incomplete and broken /etc/mtab that
+        * /etc/mtab that is writeable for non-root users.
         *
         * We always skip rename() when chown() and chmod() failed.
         * -- kzak, 11-Oct-2007
@@ -1017,6 +1032,9 @@ update_mtab (const char *dir, struct my_mntent *instead) {
        }
 
  leave:
+       if (mc0)
+               discard_mntentchn(mc0);
+       unlink(_PATH_MOUNTED_TMP);
        unlock_mtab();
 }
 

diff --git a/mount/mount.c b/mount/mount.c
index 1c5fe5a3a4aaa02ece6f827775f7260c67eeb4a3..8af218baf72636ce2699c0454fbafff97e1753b3 100644 (file)
--- a/mount/mount.c
+++ b/mount/mount.c
@@ -1442,25 +1442,8 @@ update_mtab_entry(const char *spec, const char *node, const char *type,
                        update_mtab (mnt.mnt_dir, &mnt);
                else if (flags & MS_MOVE)
                        update_mtab(mnt.mnt_fsname, &mnt);
-               else {
-                       mntFILE *mfp;
-
-                       lock_mtab();
-                       mfp = my_setmntent(_PATH_MOUNTED, "a+");
-                       if (mfp == NULL || mfp->mntent_fp == NULL) {
-                               int errsv = errno;
-                               error(_("mount: can't open %s: %s"), _PATH_MOUNTED,
-                                     strerror (errsv));
-                       } else {
-                               if ((my_addmntent (mfp, &mnt)) == 1) {
-                                       int errsv = errno;
-                                       error(_("mount: error writing %s: %s"),
-                                             _PATH_MOUNTED, strerror (errsv));
-                               }
-                       }
-                       my_endmntent(mfp);
-                       unlock_mtab();
-               }
+               else
+                       update_mtab(NULL, &mnt);
        }
        my_free(mnt.mnt_fsname);
        my_free(mnt.mnt_dir);