util-linux/patches/util-linux-2.12q-update_mtab-fixes.patch

This fixes a few issues with update_mtab():
- If it is a remount, and only mnt_opts needs updating, mc->m.mnt_opts is set
  to point to instead->mnt_opts, rather than allocating a new string, which
  would cause a double free if the caller actually freed the passed mnt_opts,
  as we free mc->m.mnt_opts before returning to the caller.
- Mostly the same issue as above.  If mtab does not contain the new entry, then
  absent->m is set to point to instead, which would have cause a double free
  if absent was inserted properly into the linked list, since we free all
  elements of absent before returning to the caller.
- If mtab does not contain the new entry, then only mc0->prev is updated to
  point to absent, but not the old mc0->prev's nxt pointer.  Because we then
  use the nxt pointers to write the new mtab, absent is not added to the new
  mtab.
- If mtab is empty, absent->prev should be set to mc0, and not mc0->prev, as
  it will be NULL.
- Memory leak if we have to abort before mc0 and co are freed.

Patch by Martin Schlemmer <azarah@gentoo.org>


--- util-linux-2.12q/mount/fstab.c	2005-09-14 15:30:10.000000000 +0200
+++ util-linux-2.12q.az/mount/fstab.c	2005-09-14 15:31:48.000000000 +0200
@@ -604,15 +604,32 @@ update_mtab (const char *dir, struct my_
 				free(mc);
 			}
 		} else {
-			/* A remount */
-			mc->m.mnt_opts = instead->mnt_opts;
+			/* A remount. */
+			my_free(mc->m.mnt_opts);
+			/* Need to alloc memory, else we might
+			 * run into issues if both we and the caller frees
+			 * mnt_opts ... */
+			mc->m.mnt_opts = xstrdup(instead->mnt_opts);
 		}
 	} else if (instead) {
 		/* not found, add a new entry */
 		absent = xmalloc(sizeof(*absent));
-		absent->m = *instead;
+		/* Cannot just set absent->m to instead, as we free absent
+		 * below, and the caller might free 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;
+		if (mc0->prev != NULL) {
+			absent->prev = mc0->prev;
+			mc0->prev->nxt = absent;
+		} else {
+			absent->prev = mc0;
+		}
 		mc0->prev = absent;
 		if (mc0->nxt == NULL)
 			mc0->nxt = absent;
@@ -624,6 +641,8 @@ update_mtab (const char *dir, struct my_
 		int errsv = errno;
 		error (_("cannot open %s (%s) - mtab not updated"),
 		       MOUNTED_TEMP, strerror (errsv));
+		/* Do not leak memory */
+		discard_mntentchn(mc0);
 		goto leave;
 	}
 
1	niro	153	This fixes a few issues with update_mtab():
2			- If it is a remount, and only mnt_opts needs updating, mc->m.mnt_opts is set
3			to point to instead->mnt_opts, rather than allocating a new string, which
4			would cause a double free if the caller actually freed the passed mnt_opts,
5			as we free mc->m.mnt_opts before returning to the caller.
6			- Mostly the same issue as above. If mtab does not contain the new entry, then
7			absent->m is set to point to instead, which would have cause a double free
8			if absent was inserted properly into the linked list, since we free all
9			elements of absent before returning to the caller.
10			- If mtab does not contain the new entry, then only mc0->prev is updated to
11			point to absent, but not the old mc0->prev's nxt pointer. Because we then
12			use the nxt pointers to write the new mtab, absent is not added to the new
13			mtab.
14			- If mtab is empty, absent->prev should be set to mc0, and not mc0->prev, as
15			it will be NULL.
16			- Memory leak if we have to abort before mc0 and co are freed.
17
18			Patch by Martin Schlemmer <azarah@gentoo.org>
19
20
21			--- util-linux-2.12q/mount/fstab.c 2005-09-14 15:30:10.000000000 +0200
22			+++ util-linux-2.12q.az/mount/fstab.c 2005-09-14 15:31:48.000000000 +0200
23			@@ -604,15 +604,32 @@ update_mtab (const char *dir, struct my_
24			free(mc);
25			}
26			} else {
27			- /* A remount */
28			- mc->m.mnt_opts = instead->mnt_opts;
29			+ /* A remount. */
30			+ my_free(mc->m.mnt_opts);
31			+ /* Need to alloc memory, else we might
32			+ * run into issues if both we and the caller frees
33			+ * mnt_opts ... */
34			+ mc->m.mnt_opts = xstrdup(instead->mnt_opts);
35			}
36			} else if (instead) {
37			/* not found, add a new entry */
38			absent = xmalloc(sizeof(*absent));
39			- absent->m = *instead;
40			+ /* Cannot just set absent->m to instead, as we free absent
41			+ * below, and the caller might free instead */
42			+ absent->m.mnt_fsname = xstrdup(instead->mnt_fsname);
43			+ absent->m.mnt_dir = xstrdup(instead->mnt_dir);
44			+ absent->m.mnt_type = xstrdup(instead->mnt_type);
45			+ absent->m.mnt_opts = xstrdup(instead->mnt_opts);
46			+ absent->m.mnt_freq = instead->mnt_freq;
47			+ absent->m.mnt_passno = instead->mnt_passno;
48			+
49			absent->nxt = mc0;
50			- absent->prev = mc0->prev;
51			+ if (mc0->prev != NULL) {
52			+ absent->prev = mc0->prev;
53			+ mc0->prev->nxt = absent;
54			+ } else {
55			+ absent->prev = mc0;
56			+ }
57			mc0->prev = absent;
58			if (mc0->nxt == NULL)
59			mc0->nxt = absent;
60			@@ -624,6 +641,8 @@ update_mtab (const char *dir, struct my_
61			int errsv = errno;
62			error (_("cannot open %s (%s) - mtab not updated"),
63			MOUNTED_TEMP, strerror (errsv));
64			+ /* Do not leak memory */
65			+ discard_mntentchn(mc0);
66			goto leave;
67			}
68