| Message ID | 20140314135350.GN8282@thunk.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Fri, Mar 14, 2014 at 09:53:50AM -0400, Theodore Ts'o wrote: > On Mon, Mar 10, 2014 at 11:55:53PM -0700, Darrick J. Wong wrote: > > Fix a few minor bugs that cppcheck complained about. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > Applied with the following changes. It looks like cppcheck complained > with another false positive in ext2fs_create_icount_tdb(). The > filename is copied in icount->tdb_fn, and so adding a call to > ext2fs_free_mem() will actually result in a double-free bug, since > ext2fs_free_icount() will take care of releasing the memory. Also, > perhaps just as importantly, it will take care of deleting the > temporary file created by mkstemp() first. > > I did keep the first ext2fs_free_mem() and moved setting > icount->tdb_fn down by a bit just to avoid a potential bug if > mkstemp() fails, and there is a valid file of the form *-icount-XXXXXX > that the user would be unhappy with us deleting. Pedantic, perhaps, > since it would probably never happen, but it's good to be 100% > correct. :-) Ok, thanks for fixing the mistakes. I was ignorant of the putenv thing. :/ --D > > - Ted > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c > index 11c2693..b39383d 100644 > --- a/e2fsck/unix.c > +++ b/e2fsck/unix.c > @@ -1016,7 +1016,6 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) > strcat(newpath, oldpath); > } > putenv(newpath); > - free(newpath); > } > #ifdef CONFIG_JBD_DEBUG > jbd_debug = getenv("E2FSCK_JBD_DEBUG"); > diff --git a/lib/ext2fs/icount.c b/lib/ext2fs/icount.c > index 7d1b3d5..5e1f5c6 100644 > --- a/lib/ext2fs/icount.c > +++ b/lib/ext2fs/icount.c > @@ -193,7 +193,6 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir, > goto errout; > uuid_unparse(fs->super->s_uuid, uuid); > sprintf(fn, "%s/%s-icount-XXXXXX", tdb_dir, uuid); > - icount->tdb_fn = fn; > save_umask = umask(077); > fd = mkstemp(fn); > if (fd < 0) { > @@ -201,6 +200,7 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir, > ext2fs_free_mem(&fn); > goto errout; > } > + icount->tdb_fn = fn; > umask(save_umask); > /* > * This is an overestimate of the size that we will need; the > @@ -217,7 +217,6 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir, > close(fd); > if (icount->tdb == NULL) { > retval = errno; > - ext2fs_free_mem(&fn); > goto errout; > } > *ret = icount; > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/e2fsck/unix.c b/e2fsck/unix.c index 11c2693..b39383d 100644 --- a/e2fsck/unix.c +++ b/e2fsck/unix.c @@ -1016,7 +1016,6 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) strcat(newpath, oldpath); } putenv(newpath); - free(newpath); } #ifdef CONFIG_JBD_DEBUG jbd_debug = getenv("E2FSCK_JBD_DEBUG"); diff --git a/lib/ext2fs/icount.c b/lib/ext2fs/icount.c index 7d1b3d5..5e1f5c6 100644 --- a/lib/ext2fs/icount.c +++ b/lib/ext2fs/icount.c @@ -193,7 +193,6 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir, goto errout; uuid_unparse(fs->super->s_uuid, uuid); sprintf(fn, "%s/%s-icount-XXXXXX", tdb_dir, uuid); - icount->tdb_fn = fn; save_umask = umask(077); fd = mkstemp(fn); if (fd < 0) { @@ -201,6 +200,7 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir, ext2fs_free_mem(&fn); goto errout; } + icount->tdb_fn = fn; umask(save_umask); /* * This is an overestimate of the size that we will need; the @@ -217,7 +217,6 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir, close(fd); if (icount->tdb == NULL) { retval = errno; - ext2fs_free_mem(&fn); goto errout; } *ret = icount;