From ceb012522c6c767a9c072705dd7b245cc696d1db Mon Sep 17 00:00:00 2001 From: Karel Zak Date: Tue, 12 Apr 2011 22:52:33 +0200 Subject: [PATCH] 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 --- mount/fstab.c | 36 +++++++++++++++++++++++++++--------- mount/mount.c | 21 ++------------------- 2 files changed, 29 insertions(+), 28 deletions(-) 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); -- 2.47.3