2014-01-07 14:53:22

by Theodore Ts'o

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

Addresses-Coverity-Id: #1148450

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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



2014-01-07 14:53:21

by Theodore Ts'o

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

Addresses-Coverity-Id: #1147784

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:21

by Theodore Ts'o

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

Addresses-Coverity-Id: #1049170

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:21

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 08/10] debugfs: remove dead code

Addresses-Coverity-Id: #1138573

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:22

by Theodore Ts'o

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

Addresses-Coverity-Id: #1138575

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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);

2014-01-07 14:53:21

by Theodore Ts'o

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

Addresses-Coverity-Id: #709475

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:21

by Theodore Ts'o

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

Addresses-Coverity-Id: #709476

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:22

by Theodore Ts'o

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

Addresses-Coverity-Id: #709491

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:22

by Theodore Ts'o

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

Addresses-Coverity-Id: #1049148

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 14:53:22

by Theodore Ts'o

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

Addresses-Coverity-Id: #709550

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
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


2014-01-07 17:14:43

by Eric Sandeen

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

On 1/7/14, 8:53 AM, Theodore Ts'o wrote:
> Addresses-Coverity-Id: #1138573
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>

Reviewed-by: Eric Sandeen <[email protected]>

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) {
>


2014-01-07 17:25:51

by Theodore Ts'o

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

On Tue, Jan 07, 2014 at 11:14:39AM -0600, Eric Sandeen wrote:
>
> Reviewed-by: Eric Sandeen <[email protected]>
>
> 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

2014-01-07 18:58:40

by Darrick J. Wong

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

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 <[email protected]>
> >
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html