[PATCH 01/10] e2fsck: release allocated memory on error or abort in e2fsck_pass1()

[PATCH 01/10] e2fsck: release allocated memory on error or abort in e2fsck_pass1()

[Theodore Ts'o]

Addresses-Coverity-Id: #1148450

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/pass1.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 71b3000..38b221c 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -546,9 +546,9 @@ void e2fsck_pass1(e2fsck_t ctx)
 	__u64	max_sizes;
 	ext2_filsys fs = ctx->fs;
 	ext2_ino_t	ino = 0;
-	struct ext2_inode *inode;
-	ext2_inode_scan	scan;
-	char		*block_buf;
+	struct ext2_inode *inode = NULL;
+	ext2_inode_scan	scan = NULL;
+	char		*block_buf = NULL;
 #ifdef RESOURCE_TRACK
 	struct resource_track	rtrack;
 #endif
@@ -662,8 +662,7 @@ void e2fsck_pass1(e2fsck_t ctx)
 	if (pctx.errcode) {
 		fix_problem(ctx, PR_1_ALLOCATE_DBCOUNT, &pctx);
 		ctx->flags |= E2F_FLAG_ABORT;
-		ext2fs_free_mem(&inode);
-		return;
+		goto endit;
 	}
 
 	/*
@@ -686,8 +685,7 @@ void e2fsck_pass1(e2fsck_t ctx)
 	if (pctx.errcode) {
 		fix_problem(ctx, PR_1_CONVERT_SUBCLUSTER, &pctx);
 		ctx->flags |= E2F_FLAG_ABORT;
-		ext2fs_free_mem(&inode);
-		return;
+		goto endit;
 	}
 	block_buf = (char *) e2fsck_allocate_memory(ctx, fs->blocksize * 3,
 						    "block interate buffer");
@@ -699,18 +697,16 @@ void e2fsck_pass1(e2fsck_t ctx)
 	if (pctx.errcode) {
 		fix_problem(ctx, PR_1_ISCAN_ERROR, &pctx);
 		ctx->flags |= E2F_FLAG_ABORT;
-		ext2fs_free_mem(&block_buf);
-		ext2fs_free_mem(&inode);
-		return;
+		goto endit;
 	}
 	ext2fs_inode_scan_flags(scan, EXT2_SF_SKIP_MISSING_ITABLE, 0);
 	ctx->stashed_inode = inode;
 	scan_struct.ctx = ctx;
 	scan_struct.block_buf = block_buf;
 	ext2fs_set_inode_callback(scan, scan_callback, &scan_struct);
-	if (ctx->progress)
-		if ((ctx->progress)(ctx, 1, 0, ctx->fs->group_desc_count))
-			return;
+	if (ctx->progress && ((ctx->progress)(ctx, 1, 0,
+					      ctx->fs->group_desc_count)))
+		goto endit;
 	if ((fs->super->s_wtime < fs->super->s_inodes_count) ||
 	    (fs->super->s_mtime < fs->super->s_inodes_count))
 		busted_fs_time = 1;
@@ -742,7 +738,7 @@ void e2fsck_pass1(e2fsck_t ctx)
 		if (pctx.errcode) {
 			fix_problem(ctx, PR_1_ISCAN_ERROR, &pctx);
 			ctx->flags |= E2F_FLAG_ABORT;
-			return;
+			goto endit;
 		}
 		if (!ino)
 			break;
@@ -756,7 +752,7 @@ void e2fsck_pass1(e2fsck_t ctx)
 				pctx.num = inode->i_links_count;
 				fix_problem(ctx, PR_1_ICOUNT_STORE, &pctx);
 				ctx->flags |= E2F_FLAG_ABORT;
-				return;
+				goto endit;
 			}
 		}
 
@@ -846,7 +842,7 @@ void e2fsck_pass1(e2fsck_t ctx)
 				pctx.num = 4;
 				fix_problem(ctx, PR_1_ALLOCATE_BBITMAP_ERROR, &pctx);
 				ctx->flags |= E2F_FLAG_ABORT;
-				return;
+				goto endit;
 			}
 			pb.ino = EXT2_BAD_INO;
 			pb.num_blocks = pb.last_block = 0;
@@ -863,12 +859,12 @@ void e2fsck_pass1(e2fsck_t ctx)
 			if (pctx.errcode) {
 				fix_problem(ctx, PR_1_BLOCK_ITERATE, &pctx);
 				ctx->flags |= E2F_FLAG_ABORT;
-				return;
+				goto endit;
 			}
 			if (pb.bbcheck)
 				if (!fix_problem(ctx, PR_1_BBINODE_BAD_METABLOCK_PROMPT, &pctx)) {
 				ctx->flags |= E2F_FLAG_ABORT;
-				return;
+				goto endit;
 			}
 			ext2fs_mark_inode_bitmap2(ctx->inode_used_map, ino);
 			clear_problem_context(&pctx);
@@ -1146,17 +1142,18 @@ void e2fsck_pass1(e2fsck_t ctx)
 			check_blocks(ctx, &pctx, block_buf);
 
 		if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
-			return;
+			goto endit;
 
 		if (process_inode_count >= ctx->process_inode_size) {
 			process_inodes(ctx, block_buf);
 
 			if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
-				return;
+				goto endit;
 		}
 	}
 	process_inodes(ctx, block_buf);
 	ext2fs_close_inode_scan(scan);
+	scan = NULL;
 
 	/*
 	 * If any extended attribute blocks' reference counts need to
@@ -1195,7 +1192,7 @@ void e2fsck_pass1(e2fsck_t ctx)
 			if (!fix_problem(ctx, PR_1_RESIZE_INODE_CREATE,
 					 &pctx)) {
 				ctx->flags |= E2F_FLAG_ABORT;
-				return;
+				goto endit;
 			}
 			pctx.errcode = 0;
 		}
@@ -1233,10 +1230,15 @@ void e2fsck_pass1(e2fsck_t ctx)
 endit:
 	e2fsck_use_inode_shortcuts(ctx, 0);
 
-	ext2fs_free_mem(&block_buf);
-	ext2fs_free_mem(&inode);
+	if (scan)
+		ext2fs_close_inode_scan(scan);
+	if (block_buf)
+		ext2fs_free_mem(&block_buf);
+	if (inode)
+		ext2fs_free_mem(&inode);
 
-	print_resource_track(ctx, _("Pass 1"), &rtrack, ctx->fs->io);
+	if ((ctx->flags & E2F_FLAG_SIGNAL_MASK) == 0)
+		print_resource_track(ctx, _("Pass 1"), &rtrack, ctx->fs->io);
 }
 
 /*
-- 
1.8.5.rc3.362.gdf10213

[PATCH 02/10] libext2fs: make ext2fs_group_desc_csum return 0 if meta_csum not enabled

[Theodore Ts'o]

Addresses-Coverity-Id: #1147784

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/csum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ext2fs/csum.c b/lib/ext2fs/csum.c
index b681d34..6989880 100644
--- a/lib/ext2fs/csum.c
+++ b/lib/ext2fs/csum.c
@@ -36,7 +36,7 @@ __u16 ext2fs_group_desc_csum(ext2_filsys fs, dgrp_t group)
 							 group);
 	size_t size = EXT2_DESC_SIZE(fs->super);
 	size_t offset;
-	__u16 crc;
+	__u16 crc = 0;
 
 	if (fs->super->s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_GDT_CSUM) {
 		size_t offset = offsetof(struct ext2_group_desc, bg_checksum);
-- 
1.8.5.rc3.362.gdf10213

[PATCH 03/10] e2fsck: fix memory leak on error path in read_bad_blocks_files()

[Theodore Ts'o]

Addresses-Coverity-Id: #1049170

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/badblocks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/e2fsck/badblocks.c b/e2fsck/badblocks.c
index 8519df0..7f3641b 100644
--- a/e2fsck/badblocks.c
+++ b/e2fsck/badblocks.c
@@ -111,6 +111,8 @@ void read_bad_blocks_file(e2fsck_t ctx, const char *bad_blocks_file,
 
 fatal:
 	ctx->flags |= E2F_FLAG_ABORT;
+	if (bb_list)
+		ext2fs_badblocks_list_free(bb_list);
 	return;
 
 }
-- 
1.8.5.rc3.362.gdf10213

[PATCH 04/10] libext2fs: remove redundant code in rb_print_stats()

[Theodore Ts'o]

Addresses-Coverity-Id: #709550

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/blkmap64_rb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/ext2fs/blkmap64_rb.c b/lib/ext2fs/blkmap64_rb.c
index a22682e..a189590 100644
--- a/lib/ext2fs/blkmap64_rb.c
+++ b/lib/ext2fs/blkmap64_rb.c
@@ -819,7 +819,6 @@ static void rb_print_stats(ext2fs_generic_bitmap bitmap)
 
 	bp = (struct ext2fs_rb_private *) bitmap->private;
 
-	node = ext2fs_rb_first(&bp->root);
 	for (node = ext2fs_rb_first(&bp->root); node != NULL;
 	     node = ext2fs_rb_next(node)) {
 		ext = node_to_extent(node);
-- 
1.8.5.rc3.362.gdf10213

[PATCH 05/10] libquota: add error checking to quota_write_inode()

[Theodore Ts'o]

Addresses-Coverity-Id: #709476

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/quota/mkquota.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/quota/mkquota.c b/lib/quota/mkquota.c
index 3aa8100..b920f27 100644
--- a/lib/quota/mkquota.c
+++ b/lib/quota/mkquota.c
@@ -133,11 +133,16 @@ errcode_t quota_write_inode(quota_ctx_t qctx, int qtype)
 	fs = qctx->fs;
 	retval = ext2fs_get_mem(sizeof(struct quota_handle), &h);
 	if (retval) {
-		log_err("Unable to allocate quota handle");
+		log_err("Unable to allocate quota handle: %s",
+			error_message(retval));
 		goto out;
 	}
 
-	ext2fs_read_bitmaps(fs);
+	retval = ext2fs_read_bitmaps(fs);
+	if (retval) {
+		log_err("Couldn't read bitmaps: %s", error_message(retval));
+		goto out;
+	}
 
 	for (i = 0; i < MAXQUOTAS; i++) {
 		if ((qtype != -1) && (i != qtype))
@@ -171,7 +176,11 @@ errcode_t quota_write_inode(quota_ctx_t qctx, int qtype)
 		fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
 	}
 
-	ext2fs_write_bitmaps(fs);
+	retval = ext2fs_write_bitmaps(fs);
+	if (retval) {
+		log_err("Couldn't write bitmaps: %s", error_message(retval));
+		goto out;
+	}
 out:
 	if (h)
 		ext2fs_free_mem(&h);
-- 
1.8.5.rc3.362.gdf10213

[PATCH 06/10] libquota: add error checking to quota_remove_inode

[Theodore Ts'o]

Addresses-Coverity-Id: #709475

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/quota/mkquota.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/lib/quota/mkquota.c b/lib/quota/mkquota.c
index b920f27..9883216 100644
--- a/lib/quota/mkquota.c
+++ b/lib/quota/mkquota.c
@@ -89,8 +89,13 @@ void quota_set_sb_inum(ext2_filsys fs, ext2_ino_t ino, int qtype)
 errcode_t quota_remove_inode(ext2_filsys fs, int qtype)
 {
 	ext2_ino_t qf_ino;
+	errcode_t	retval;
 
-	ext2fs_read_bitmaps(fs);
+	retval = ext2fs_read_bitmaps(fs);
+	if (retval) {
+		log_err("Couldn't read bitmaps: %s", error_message(retval));
+		return retval;
+	}
 	qf_ino = (qtype == USRQUOTA) ? fs->super->s_usr_quota_inum :
 		fs->super->s_grp_quota_inum;
 	quota_set_sb_inum(fs, 0, qtype);
@@ -100,7 +105,11 @@ errcode_t quota_remove_inode(ext2_filsys fs, int qtype)
 
 	ext2fs_mark_super_dirty(fs);
 	fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
-	ext2fs_write_bitmaps(fs);
+	retval = ext2fs_write_bitmaps(fs);
+	if (retval) {
+		log_err("Couldn't write bitmaps: %s", error_message(retval));
+		return retval;
+	}
 	return 0;
 }
 
-- 
1.8.5.rc3.362.gdf10213

[PATCH 07/10] libext2fs: fix memory leaks on error paths in ext2fs_create_icount_tdb

[Theodore Ts'o]

Addresses-Coverity-Id: #1138575

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/icount.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/lib/ext2fs/icount.c b/lib/ext2fs/icount.c
index c5ebf74..80085e1 100644
--- a/lib/ext2fs/icount.c
+++ b/lib/ext2fs/icount.c
@@ -192,10 +192,12 @@ 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;
 	fd = mkstemp(fn);
-	if (fd < 0)
-		return fd;
-
+	if (fd < 0) {
+		retval = errno;
+		goto errout;
+	}
 	/*
 	 * This is an overestimate of the size that we will need; the
 	 * ideal value is the number of used inodes with a count
@@ -206,18 +208,15 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir,
 	 */
 	num_inodes = fs->super->s_inodes_count - fs->super->s_free_inodes_count;
 
-	icount->tdb_fn = fn;
 	icount->tdb = tdb_open(fn, num_inodes, TDB_NOLOCK | TDB_NOSYNC,
 			       O_RDWR | O_CREAT | O_TRUNC, 0600);
-	if (icount->tdb) {
-		close(fd);
-		*ret = icount;
-		return 0;
-	}
-
-	retval = errno;
 	close(fd);

[PATCH 08/10] debugfs: remove dead code

[Theodore Ts'o]

Addresses-Coverity-Id: #1138573

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 debugfs/debugfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 998af33..0e56ead 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -289,8 +289,6 @@ void do_init_filesys(int argc, char **argv)
 	if (err)
 		return;
 	ext2fs_blocks_count_set(&param, blocks);
-	if (err)
-		return;
 	retval = ext2fs_initialize(argv[1], 0, &param,
 				   unix_io_manager, &current_fs);
 	if (retval) {
-- 
1.8.5.rc3.362.gdf10213

Re: [PATCH 08/10] debugfs: remove dead code

[Eric Sandeen]

On 1/7/14, 8:53 AM, Theodore Ts'o wrote:
> Addresses-Coverity-Id: #1138573
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

FWIW, Darrick had already sent this as part of

"[PATCH 1/2] misc: fix resource leaks in e2fsprogs"

- but I suppose small targeted patches are better.

-Eric

> ---
>  debugfs/debugfs.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
> index 998af33..0e56ead 100644
> --- a/debugfs/debugfs.c
> +++ b/debugfs/debugfs.c
> @@ -289,8 +289,6 @@ void do_init_filesys(int argc, char **argv)
>  	if (err)
>  		return;
>  	ext2fs_blocks_count_set(&param, blocks);
> -	if (err)
> -		return;
>  	retval = ext2fs_initialize(argv[1], 0, &param,
>  				   unix_io_manager, &current_fs);
>  	if (retval) {
> 

Re: [PATCH 08/10] debugfs: remove dead code

[Theodore Ts'o]

On Tue, Jan 07, 2014 at 11:14:39AM -0600, Eric Sandeen wrote:
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> FWIW, Darrick had already sent this as part of
> 
> "[PATCH 1/2] misc: fix resource leaks in e2fsprogs"
> 
> - but I suppose small targeted patches are better.

Whoops, I somehow managed to lose this patch in my inbox.  I see it in
patchwork, though.  I'll have to double check which of the other fixes
in his patch are still needed.

						- Ted

Re: [PATCH 08/10] debugfs: remove dead code

[Darrick J. Wong]

On Tue, Jan 07, 2014 at 12:25:48PM -0500, Theodore Ts'o wrote:
> On Tue, Jan 07, 2014 at 11:14:39AM -0600, Eric Sandeen wrote:
> > 
> > Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> > 
> > FWIW, Darrick had already sent this as part of
> > 
> > "[PATCH 1/2] misc: fix resource leaks in e2fsprogs"
> > 
> > - but I suppose small targeted patches are better.
> 
> Whoops, I somehow managed to lose this patch in my inbox.  I see it in
> patchwork, though.  I'll have to double check which of the other fixes
> in his patch are still needed.

As far as coverity bugs go, I put my name on all the bugs that I fixed, and
left the rest alone.

--D
> 
> 						- Ted
> --
> 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

[PATCH 09/10] badblocks: print warning if set_o_direct() fails

[Theodore Ts'o]

Addresses-Coverity-Id: #1049148

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 misc/badblocks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/misc/badblocks.c b/misc/badblocks.c
index 912ef28..e5024f6 100644
--- a/misc/badblocks.c
+++ b/misc/badblocks.c
@@ -300,7 +300,8 @@ static void set_o_direct(int dev, unsigned char *buffer, size_t size,
 		flag = fcntl(dev, F_GETFL);
 		if (flag > 0) {
 			flag = (flag & ~O_DIRECT) | new_flag;
-			fcntl(dev, F_SETFL, flag);
+			if (fcntl(dev, F_SETFL, flag) < 0)
+				perror("set_o_direct");
 		}
 		current_O_DIRECT = new_flag;
 	}
-- 
1.8.5.rc3.362.gdf10213

[PATCH 10/10] libss: fix memory leak if realloc() fails in ss_parse()

[Theodore Ts'o]

Addresses-Coverity-Id: #709491

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ss/parse.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/lib/ss/parse.c b/lib/ss/parse.c
index e2928e2..45a2de0 100644
--- a/lib/ss/parse.c
+++ b/lib/ss/parse.c
@@ -45,7 +45,7 @@ enum parse_mode { WHITESPACE, TOKEN, QUOTED_STRING };
 
 char **ss_parse(int sci_idx, register char *line_ptr, int *argc_ptr)
 {
-    register char **argv, *cp;
+    register char **argv, **new_argv, *cp;
     register int argc;
     register enum parse_mode parse_mode;
 
@@ -78,7 +78,13 @@ char **ss_parse(int sci_idx, register char *line_ptr, int *argc_ptr)
 		/* go to quoted-string mode */
 		parse_mode = QUOTED_STRING;
 		cp = line_ptr++;
-		argv = NEW_ARGV (argv, argc);
+		new_argv = NEW_ARGV (argv, argc);
+		if (new_argv == NULL) {
+			free(argv);
+			*argc_ptr = 0;
+			return NULL;
+		}
+		argv = new_argv;
 		argv[argc++] = cp;
 		argv[argc] = NULL;
 	    }
@@ -86,11 +92,13 @@ char **ss_parse(int sci_idx, register char *line_ptr, int *argc_ptr)
 		/* random-token mode */
 		parse_mode = TOKEN;
 		cp = line_ptr;
-		argv = NEW_ARGV (argv, argc);
+		new_argv = NEW_ARGV (argv, argc);
 		if (argv == NULL) {
-			*argc_ptr = errno;
-			return argv;
+			free(argv);
+			*argc_ptr = 0;
+			return NULL;
 		}
+		argv = new_argv;
 		argv[argc++] = line_ptr;
 		argv[argc] = NULL;
 	    }
-- 
1.8.5.rc3.362.gdf10213