From: Karel Zak <kzak@redhat.com>
Date: Tue, 12 Apr 2011 20:52:33 +0000 (+0200)
Subject: mount: use fflush() and temporary file for mtab updates (CVE-2011-1089)
X-Git-Tag: v2.20-rc1~349
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ceb012522c6c767a9c072705dd7b245cc696d1db;p=thirdparty%2Futil-linux.git

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 fd53ca4beb..c04e9733e6 100644
--- 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 b508de674d..aa59304bc3 100644
--- a/mount/mount.c
+++ b/mount/mount.c
@@ -1443,25 +1443,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);