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.
---
mount/fstab.c | 27 +++++++++++++++++++++++----
1 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/mount/fstab.c b/mount/fstab.c
index bdcc7a6..845f577 100644
--- a/mount/fstab.c
+++ b/mount/fstab.c
@@ -639,15 +639,32 @@ update_mtab (const char *dir, struct my_mntent *instead) {
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;
@@ -659,6 +676,8 @@ update_mtab (const char *dir, struct my_mntent *instead) {
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.5.1
-
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
