[PATCH 1/3] lib/nilfs.c: fix potential leak at nilfs_open()
nilfs_open() can exit without closing nilfs-n_devfd and freeing
nilfs-n_dev and nilfs-n_sb if it first initializes a nilfs object in
the code path for NILFS_OPEN_RAW mode and then escapes through
out_nilfs label. This fixes the leak issue.
Signed-off-by: Ryusuke Konishi konishi.ryus...@lab.ntt.co.jp
---
lib/nilfs.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/nilfs.c b/lib/nilfs.c
index 65bf7d5..52ddee9 100644
--- a/lib/nilfs.c
+++ b/lib/nilfs.c
@@ -411,9 +411,9 @@ struct nilfs *nilfs_open(const char *dev, const char *dir,
int flags)
(NILFS_OPEN_RDONLY | NILFS_OPEN_WRONLY | NILFS_OPEN_RDWR)) {
if (nilfs_find_fs(nilfs, dev, dir, MNTOPT_RW) 0) {
if (!(flags NILFS_OPEN_RDONLY))
- goto out_nilfs;
+ goto out_fd;
if (nilfs_find_fs(nilfs, dev, dir, MNTOPT_RO) 0)
- goto out_nilfs;
+ goto out_fd;
}
nilfs-n_iocfd = open(nilfs-n_ioc, O_RDONLY);
if (nilfs-n_iocfd 0)
@@ -442,7 +442,6 @@ out_fd:
if (nilfs-n_sb != NULL)
free(nilfs-n_sb);
-out_nilfs:
free(nilfs);
return NULL;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-nilfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] nilfs-utils: get rid of my_free()
Remove my_free wrapper functions used in fstab.c, mount.nilfs2.c and
umount.nilfs2.c. They are just doing an unnecessary null check before
calling free() and eliminable since free(NULL) is just ignored.
Signed-off-by: Ryusuke Konishi konishi.ryus...@lab.ntt.co.jp
---
sbin/mount/fstab.c | 20 +++-
sbin/mount/mount.nilfs2.c | 38 +++---
sbin/mount/umount.nilfs2.c | 15 ---
3 files changed, 26 insertions(+), 47 deletions(-)
diff --git a/sbin/mount/fstab.c b/sbin/mount/fstab.c
index b0addbe..656a39b 100644
--- a/sbin/mount/fstab.c
+++ b/sbin/mount/fstab.c
@@ -124,19 +124,13 @@ fstab_head() {
return fstab;
}
-static void
-my_free(const void *s) {
- if (s)
- free((void *) s);
-}
-
-static void
-my_free_mc(struct mntentchn *mc) {
+static void 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);
- my_free(mc-m.mnt_opts);
+ free(mc-m.mnt_fsname);
+ free(mc-m.mnt_dir);
+ free(mc-m.mnt_type);
+ free(mc-m.mnt_opts);
free(mc);
}
}
@@ -574,7 +568,7 @@ void update_mtab(const char *dir, struct my_mntent *instead)
}
} else {
/* Replace option strings. (changed for nilfs2) */
- my_free(mc-m.mnt_opts);
+ free(mc-m.mnt_opts);
mc-m.mnt_opts = xstrdup(instead-mnt_opts);
}
} else if (instead) {
diff --git a/sbin/mount/mount.nilfs2.c b/sbin/mount/mount.nilfs2.c
index e9cb25e..e3fc727 100644
--- a/sbin/mount/mount.nilfs2.c
+++ b/sbin/mount/mount.nilfs2.c
@@ -24,7 +24,6 @@
* The following functions are extracted from util-linux-2.12r/mount.c:
* - print_one()
* - update_mtab_entry()
- * - my_free()
*/
#ifdef HAVE_CONFIG_H
@@ -172,13 +171,6 @@ static void handle_signal(int sig)
}
}
-static inline void my_free(const void *ptr)
-{
- /* free(NULL) is ignored; the check below is just to be sure */
- if (ptr)
- free((void *)ptr);
-}
-
static int device_is_readonly(const char *device, int *ro)
{
int fd, res;
@@ -255,7 +247,7 @@ static struct mntentchn *find_rw_mount(const char *device)
break;
mc = getmntdevbackward(fsname, mc);
}
- my_free(fsname);
+ free(fsname);
return mc;
}
@@ -275,8 +267,8 @@ static int mounted(const char *spec, const char *node)
}
mc = getmntdirbackward(dir, mc);
}
- my_free(fsname);
- my_free(dir);
+ free(fsname);
+ free(dir);
return ret;
}
@@ -335,10 +327,10 @@ update_mtab_entry(const char *spec, const char *node,
const char *type,
my_endmntent(mfp);
unlock_mtab();
}
- my_free(mnt.mnt_fsname);
- my_free(mnt.mnt_dir);
- my_free(mnt.mnt_type);
- my_free(mnt.mnt_opts);
+ free(mnt.mnt_fsname);
+ free(mnt.mnt_dir);
+ free(mnt.mnt_type);
+ free(mnt.mnt_opts);
}
enum remount_type {
@@ -349,7 +341,7 @@ enum remount_type {
static int check_remount_dir(struct mntentchn *mc, const char *mntdir)
{
- const char *dir = canonicalize(mntdir);
+ char *dir = canonicalize(mntdir);
int res = 0;
if (strcmp(dir, mc-m.mnt_dir) != 0) {
@@ -357,7 +349,7 @@ static int check_remount_dir(struct mntentchn *mc, const
char *mntdir)
progname, mntdir);
res = -1;
}
- my_free(dir);
+ free(dir);
return res;
}
@@ -514,7 +506,7 @@ do_mount_one(struct nilfs_mount_info *mi, const struct
mount_options *mo)
} else
printf(_(%s not restarted\n), NILFS_CLEANERD_NAME);
out:
- my_free(exopts);
+ free(exopts);
return res;
}
@@ -542,14 +534,14 @@ static void update_mount_state(struct nilfs_mount_info
*mi,
if (!check_mtab())
return;
- my_free(mi-optstr);
+ free(mi-optstr);
exopts = fix_extra_opts_string(mo-extra_opts, pid, pp);
mi-optstr = fix_opts_string(((mo-flags ~MS_NOMTAB) | MS_NETDEV),
exopts, NULL);
update_mtab_entry(mi-device, mi-mntdir, fstype, mi-optstr, 0, 0,
!mi-mounted);
- my_free(exopts);
+ free(exopts);
}
static int mount_one(char *device, char *mntdir,
@@ -591,7 +583,7 @@ static int mount_one(char *device, char *mntdir,
err = 0;
failed:
- my_free(mi.optstr);
+ free(mi.optstr);
return err;
}
@@ -655,7 +647,7 @@ int main(int argc, char *argv[])
res = mount_one(device, mntdir, opts);
block_signals(SIG_UNBLOCK);
- my_free(opts-opts);
-
[PATCH v2 2/3] nilfs-utils: get rid of my_free()
Remove my_free wrapper functions used in fstab.c, mount.nilfs2.c and
umount.nilfs2.c. They are just doing an unnecessary null check before
calling free() and eliminable since free(NULL) is just ignored.
Signed-off-by: Ryusuke Konishi konishi.ryus...@lab.ntt.co.jp
---
sbin/mount/fstab.c | 20 +++-
sbin/mount/mount.nilfs2.c | 38 +++---
sbin/mount/mount_mntent.h | 8
sbin/mount/umount.nilfs2.c | 15 ---
4 files changed, 30 insertions(+), 51 deletions(-)
diff --git a/sbin/mount/fstab.c b/sbin/mount/fstab.c
index b0addbe..656a39b 100644
--- a/sbin/mount/fstab.c
+++ b/sbin/mount/fstab.c
@@ -124,19 +124,13 @@ fstab_head() {
return fstab;
}
-static void
-my_free(const void *s) {
- if (s)
- free((void *) s);
-}
-
-static void
-my_free_mc(struct mntentchn *mc) {
+static void 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);
- my_free(mc-m.mnt_opts);
+ free(mc-m.mnt_fsname);
+ free(mc-m.mnt_dir);
+ free(mc-m.mnt_type);
+ free(mc-m.mnt_opts);
free(mc);
}
}
@@ -574,7 +568,7 @@ void update_mtab(const char *dir, struct my_mntent *instead)
}
} else {
/* Replace option strings. (changed for nilfs2) */
- my_free(mc-m.mnt_opts);
+ free(mc-m.mnt_opts);
mc-m.mnt_opts = xstrdup(instead-mnt_opts);
}
} else if (instead) {
diff --git a/sbin/mount/mount.nilfs2.c b/sbin/mount/mount.nilfs2.c
index e9cb25e..e3fc727 100644
--- a/sbin/mount/mount.nilfs2.c
+++ b/sbin/mount/mount.nilfs2.c
@@ -24,7 +24,6 @@
* The following functions are extracted from util-linux-2.12r/mount.c:
* - print_one()
* - update_mtab_entry()
- * - my_free()
*/
#ifdef HAVE_CONFIG_H
@@ -172,13 +171,6 @@ static void handle_signal(int sig)
}
}
-static inline void my_free(const void *ptr)
-{
- /* free(NULL) is ignored; the check below is just to be sure */
- if (ptr)
- free((void *)ptr);
-}
-
static int device_is_readonly(const char *device, int *ro)
{
int fd, res;
@@ -255,7 +247,7 @@ static struct mntentchn *find_rw_mount(const char *device)
break;
mc = getmntdevbackward(fsname, mc);
}
- my_free(fsname);
+ free(fsname);
return mc;
}
@@ -275,8 +267,8 @@ static int mounted(const char *spec, const char *node)
}
mc = getmntdirbackward(dir, mc);
}
- my_free(fsname);
- my_free(dir);
+ free(fsname);
+ free(dir);
return ret;
}
@@ -335,10 +327,10 @@ update_mtab_entry(const char *spec, const char *node,
const char *type,
my_endmntent(mfp);
unlock_mtab();
}
- my_free(mnt.mnt_fsname);
- my_free(mnt.mnt_dir);
- my_free(mnt.mnt_type);
- my_free(mnt.mnt_opts);
+ free(mnt.mnt_fsname);
+ free(mnt.mnt_dir);
+ free(mnt.mnt_type);
+ free(mnt.mnt_opts);
}
enum remount_type {
@@ -349,7 +341,7 @@ enum remount_type {
static int check_remount_dir(struct mntentchn *mc, const char *mntdir)
{
- const char *dir = canonicalize(mntdir);
+ char *dir = canonicalize(mntdir);
int res = 0;
if (strcmp(dir, mc-m.mnt_dir) != 0) {
@@ -357,7 +349,7 @@ static int check_remount_dir(struct mntentchn *mc, const
char *mntdir)
progname, mntdir);
res = -1;
}
- my_free(dir);
+ free(dir);
return res;
}
@@ -514,7 +506,7 @@ do_mount_one(struct nilfs_mount_info *mi, const struct
mount_options *mo)
} else
printf(_(%s not restarted\n), NILFS_CLEANERD_NAME);
out:
- my_free(exopts);
+ free(exopts);
return res;
}
@@ -542,14 +534,14 @@ static void update_mount_state(struct nilfs_mount_info
*mi,
if (!check_mtab())
return;
- my_free(mi-optstr);
+ free(mi-optstr);
exopts = fix_extra_opts_string(mo-extra_opts, pid, pp);
mi-optstr = fix_opts_string(((mo-flags ~MS_NOMTAB) | MS_NETDEV),
exopts, NULL);
update_mtab_entry(mi-device, mi-mntdir, fstype, mi-optstr, 0, 0,
!mi-mounted);
- my_free(exopts);
+ free(exopts);
}
static int mount_one(char *device, char *mntdir,
@@ -591,7 +583,7 @@ static int mount_one(char *device, char *mntdir,
err = 0;
failed:
- my_free(mi.optstr);
+ free(mi.optstr);
return err;
}
@@ -655,7 +647,7 @@ int main(int argc, char *argv[])
res = mount_one(device, mntdir, opts);
[PATCH v2 3/3] nilfs-utils: get rid of null checks before calling free()
Remove unnecessary null checks before calling free() function.
Signed-off-by: Ryusuke Konishi konishi.ryus...@lab.ntt.co.jp
---
lib/cleaner_ctl.c | 6 ++
lib/gc.c | 3 +--
lib/nilfs.c | 19 +++
lib/realpath.c| 9 +++--
4 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/lib/cleaner_ctl.c b/lib/cleaner_ctl.c
index fa41ac1..9c458a4 100644
--- a/lib/cleaner_ctl.c
+++ b/lib/cleaner_ctl.c
@@ -226,8 +226,7 @@ static int nilfs_cleaner_find_fs(struct nilfs_cleaner
*cleaner,
sizeof(canonical))) {
mdev = canonical;
}
- if (last_match_dev)
- free(last_match_dev);
+ free(last_match_dev);
last_match_dev = strdup(mdev);
if (!last_match_dev)
goto error;
@@ -238,8 +237,7 @@ static int nilfs_cleaner_find_fs(struct nilfs_cleaner
*cleaner,
sizeof(canonical))) {
mdir = canonical;
}
- if (last_match_dir)
- free(last_match_dir);
+ free(last_match_dir);
last_match_dir = strdup(mdir);
if (!last_match_dir)
goto error;
diff --git a/lib/gc.c b/lib/gc.c
index 54d0b66..48c295a 100644
--- a/lib/gc.c
+++ b/lib/gc.c
@@ -508,8 +508,7 @@ static int nilfs_toss_vdescs(struct nilfs *nilfs,
}
ret = 0;
out:
- if (ss != NULL)
- free(ss);
+ free(ss);
return ret;
}
diff --git a/lib/nilfs.c b/lib/nilfs.c
index 52ddee9..30db654 100644
--- a/lib/nilfs.c
+++ b/lib/nilfs.c
@@ -435,13 +435,10 @@ out_fd:
close(nilfs-n_devfd);
if (nilfs-n_iocfd = 0)
close(nilfs-n_iocfd);
- if (nilfs-n_dev != NULL)
- free(nilfs-n_dev);
- if (nilfs-n_ioc != NULL)
- free(nilfs-n_ioc);
- if (nilfs-n_sb != NULL)
- free(nilfs-n_sb);
+ free(nilfs-n_dev);
+ free(nilfs-n_ioc);
+ free(nilfs-n_sb);
free(nilfs);
return NULL;
}
@@ -458,12 +455,10 @@ void nilfs_close(struct nilfs *nilfs)
close(nilfs-n_devfd);
if (nilfs-n_iocfd = 0)
close(nilfs-n_iocfd);
- if (nilfs-n_dev != NULL)
- free(nilfs-n_dev);
- if (nilfs-n_ioc != NULL)
- free(nilfs-n_ioc);
- if (nilfs-n_sb != NULL)
- free(nilfs-n_sb);
+
+ free(nilfs-n_dev);
+ free(nilfs-n_ioc);
+ free(nilfs-n_sb);
free(nilfs);
}
diff --git a/lib/realpath.c b/lib/realpath.c
index 3f01b87..691360b 100644
--- a/lib/realpath.c
+++ b/lib/realpath.c
@@ -133,8 +133,7 @@ myrealpath(const char *path, char *resolved_path, int
maxreslth) {
/* Insert symlink contents into path. */
m = strlen(path);
- if (buf)
- free(buf);
+ free(buf);
buf = malloc(m + n + 1);
if (!buf) {
errno = ENOMEM;
@@ -153,12 +152,10 @@ myrealpath(const char *path, char *resolved_path, int
maxreslth) {
/* Make sure it's null terminated. */
*npath = '\0';
- if (buf)
- free(buf);
+ free(buf);
return resolved_path;
err:
- if (buf)
- free(buf);
+ free(buf);
return NULL;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-nilfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
