mount: fix memory usage in update_mtab

The update_mtab deallocates memory which was allocated by caller. It's nice
opportunity for double-free errors.

The patch fix a memory leak if we have to abort before mc0 are freed.  The
patch also fix a memory leak when we deallocate old (umounted) entry.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Martin Schlemmer <azarah@gentoo.org>
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/mount/fstab.c b/mount/fstab.c
index 1e186c194545fea979c58b46d9a0a6919130a799..8e59c8a02b67c65151b697ef42195cb42dd68197 100644 (file)
--- a/mount/fstab.c
+++ b/mount/fstab.c
@@ -102,11 +102,8 @@ my_free(const void *s) {
 }
 
 static void
-discard_mntentchn(struct mntentchn *mc0) {
-       struct mntentchn *mc, *mc1;
-
-       for (mc = mc0->nxt; mc && mc != mc0; mc = mc1) {
-               mc1 = mc->nxt;
+my_free_mc(struct mntentchn *mc) {
+       if (mc) {
                my_free(mc->m.mnt_fsname);
                my_free(mc->m.mnt_dir);
                my_free(mc->m.mnt_type);
@@ -115,6 +112,17 @@ discard_mntentchn(struct mntentchn *mc0) {
        }
 }
 
+
+static void
+discard_mntentchn(struct mntentchn *mc0) {
+       struct mntentchn *mc, *mc1;
+
+       for (mc = mc0->nxt; mc && mc != mc0; mc = mc1) {
+               mc1 = mc->nxt;
+               my_free_mc(mc);
+       }
+}
+
 static void
 read_mntentchn(mntFILE *mfp, const char *fnam, struct mntentchn *mc0) {
        struct mntentchn *mc = mc0;
@@ -636,11 +644,12 @@ update_mtab (const char *dir, struct my_mntent *instead) {
                        if (mc && mc != mc0) {
                                mc->prev->nxt = mc->nxt;
                                mc->nxt->prev = mc->prev;
-                               free(mc);
+                               my_free_mc(mc);
                        }
                } else if (!strcmp(mc->m.mnt_dir, instead->mnt_dir)) {
                        /* A remount */
-                       mc->m.mnt_opts = instead->mnt_opts;
+                       my_free(mc->m.mnt_opts);
+                       mc->m.mnt_opts = xstrdup(instead->mnt_opts);
                } else {
                        /* A move */
                        my_free(mc->m.mnt_dir);
@@ -649,7 +658,12 @@ update_mtab (const char *dir, struct my_mntent *instead) {
        } else if (instead) {
                /* not found, add a new entry */
                absent = xmalloc(sizeof(*absent));
-               absent->m = *instead;
+               absent->m.mnt_fsname = xstrdup(instead->mnt_fsname);
+               absent->m.mnt_dir = xstrdup(instead->mnt_dir);
+               absent->m.mnt_type = xstrdup(instead->mnt_type);
+               absent->m.mnt_opts = xstrdup(instead->mnt_opts);
+               absent->m.mnt_freq = instead->mnt_freq;
+               absent->m.mnt_passno = instead->mnt_passno;
                absent->nxt = mc0;
                absent->prev = mc0->prev;
                mc0->prev = absent;
@@ -663,6 +677,7 @@ update_mtab (const char *dir, struct my_mntent *instead) {
                int errsv = errno;
                error (_("cannot open %s (%s) - mtab not updated"),
                       MOUNTED_TEMP, strerror (errsv));
+               discard_mntentchn(mc0);
                goto leave;
        }