[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Cluster-devel] [PATCH 7/7] gfs2_edit: More static analysis fixes



Fix some more minor issues found by Coverity and Clang, including
unchecked lseek()s, unbound strcpy()s, branching based on an
uninitialised variable, potential NULL pointer derefs, some dead
assignments and a dangling reference to a local variable.

Signed-off-by: Andrew Price <anprice redhat com>
---
 gfs2/edit/extended.c |  5 +----
 gfs2/edit/gfs2hex.c  |  7 ++++---
 gfs2/edit/hexedit.c  | 13 +++++++------
 gfs2/edit/journal.c  | 36 +++++++++++++++---------------------
 gfs2/edit/savemeta.c | 18 ++++++------------
 5 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
index 793ef6b..e5cb12c 100644
--- a/gfs2/edit/extended.c
+++ b/gfs2/edit/extended.c
@@ -391,9 +391,8 @@ static void print_block_details(struct iinfo *ind, int level, int cur_height,
 		return;
 	}
 	while (thisblk) {
-		lseek(sbd.device_fd, thisblk * sbd.bsize, SEEK_SET);
 		/* read in the desired block */
-		if (read(sbd.device_fd, tmpbuf, sbd.bsize) != sbd.bsize) {
+		if (pread(sbd.device_fd, tmpbuf, sbd.bsize, thisblk * sbd.bsize) != sbd.bsize) {
 			fprintf(stderr, "bad read: %s from %s:%d: block %lld "
 				"(0x%llx)\n", strerror(errno), __FUNCTION__,
 				__LINE__,
@@ -446,7 +445,6 @@ static int print_gfs_jindex(struct gfs2_inode *dij)
 	char jbuf[sizeof(struct gfs_jindex)];
 
 	start_line = line;
-	error = 0;
 	print_gfs2("Journal index entries found: %d.",
 		   dij->i_di.di_size / sizeof(struct gfs_jindex));
 	eol(0);
@@ -516,7 +514,6 @@ static int parse_rindex(struct gfs2_inode *dip, int print_rindex)
 	char highlighted_addr[32];
 
 	start_line = line;
-	error = 0;
 	print_gfs2("RG index entries found: %d.", dip->i_di.di_size /
 		   sizeof(struct gfs2_rindex));
 	eol(0);
diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
index 979aee0..b6075f4 100644
--- a/gfs2/edit/gfs2hex.c
+++ b/gfs2/edit/gfs2hex.c
@@ -38,7 +38,6 @@ char efield[64];
 int edit_mode = 0;
 int edit_row[DMODES], edit_col[DMODES];
 int edit_size[DMODES], last_entry_onscreen[DMODES];
-char edit_fmt[80];
 enum dsp_mode dmode = HEX_MODE; /* display mode */
 uint64_t block = 0;
 int blockhist = 0;
@@ -201,9 +200,11 @@ void print_it(const char *label, const char *fmt, const char *fmt2, ...)
 		if (termlines) {
 			refresh();
 			if (line == (edit_row[dmode] * lines_per_row[dmode]) + 4) {
-				strcpy(efield, label + 2); /* it's indented */
+				strncpy(efield, label + 2, 63); /* it's indented */
+				efield[63] = '\0';
 				strcpy(estring, tmp_string);
-				strcpy(edit_fmt, fmt);
+				strncpy(edit_fmt, fmt, 79);
+				edit_fmt[79] = '\0';
 				edit_size[dmode] = strlen(estring);
 				COLORS_NORMAL;
 			}
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 5c5ac89..bc3ca35 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1583,7 +1583,7 @@ static uint64_t find_metablockoftype_rg(uint64_t startblk, int metatype, int pri
 	struct osi_node *next = NULL;
 	uint64_t blk, errblk;
 	int first = 1, found = 0;
-	struct rgrp_tree *rgd;
+	struct rgrp_tree *rgd = NULL;
 	struct gfs2_rindex *ri;
 
 	blk = 0;
@@ -1726,7 +1726,7 @@ uint64_t check_keywords(const char *kword)
 		blk = get_rg_addr(rgnum);
 	} else if (!strncmp(kword, "journals", 8)) {
 		blk = JOURNALS_DUMMY_BLOCK;
-	} else if (!strncmp(kword, "journal", 7) && isdigit(kword[7])) {
+	} else if (strlen(kword) > 7 && !strncmp(kword, "journal", 7) && isdigit(kword[7])) {
 		uint64_t j_size;
 
 		blk = find_journal_block(kword, &j_size);
@@ -1837,8 +1837,7 @@ static void hex_edit(int *exitch)
 					ch += (estring[i+1] - 'A' + 0x0a);
 				bh->b_data[offset + hexoffset] = ch;
 			}
-			lseek(sbd.device_fd, dev_offset, SEEK_SET);
-			if (write(sbd.device_fd, bh->b_data, sbd.bsize) !=
+			if (pwrite(sbd.device_fd, bh->b_data, sbd.bsize, dev_offset) !=
 			    sbd.bsize) {
 				fprintf(stderr, "write error: %s from %s:%d: "
 					"offset %lld (0x%llx)\n",
@@ -2676,8 +2675,10 @@ static void parameterpass1(int argc, char *argv[], int i)
 		termlines = 0;
 	else if (!strcasecmp(argv[i], "-x"))
 		dmode = HEX_MODE;
-	else if (!device[0] && strchr(argv[i],'/'))
-		strcpy(device, argv[i]);
+	else if (!device[0] && strchr(argv[i],'/')) {
+		strncpy(device, argv[i], NAME_MAX-1);
+		device[NAME_MAX-1] = '\0';
+	}
 }
 
 /* ------------------------------------------------------------------------ */
diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
index 3c2f5c7..5000c2e 100644
--- a/gfs2/edit/journal.c
+++ b/gfs2/edit/journal.c
@@ -98,16 +98,6 @@ static void check_journal_wrap(uint64_t seq, uint64_t *highest_seq)
 	*highest_seq = seq;
 }
 
-static int is_meta(struct gfs2_buffer_head *lbh)
-{
-	uint32_t check_magic = ((struct gfs2_meta_header *)(lbh->b_data))->mh_magic;
-
-	check_magic = be32_to_cpu(check_magic);
-	if (check_magic == GFS2_MAGIC)
-		return 1;
-	return 0;
-}
-
 /**
  * fsck_readi - same as libgfs2's gfs2_readi, but sets absolute block #
  *              of the first bit of data read.
@@ -115,16 +105,20 @@ static int is_meta(struct gfs2_buffer_head *lbh)
 static int fsck_readi(struct gfs2_inode *ip, void *rbuf, uint64_t roffset,
 	       unsigned int size, uint64_t *abs_block)
 {
-	struct gfs2_sbd *sdp = ip->i_sbd;
+	struct gfs2_sbd *sdp;
 	struct gfs2_buffer_head *lbh;
 	uint64_t lblock, dblock;
 	unsigned int o;
 	uint32_t extlen = 0;
 	unsigned int amount;
 	int not_new = 0;
-	int isdir = !!(S_ISDIR(ip->i_di.di_mode));
+	int isdir;
 	int copied = 0;
 
+	if (ip == NULL)
+		return 0;
+	sdp = ip->i_sbd;
+	isdir = !!(S_ISDIR(ip->i_di.di_mode));
 	*abs_block = 0;
 	if (roffset >= ip->i_di.di_size)
 		return 0;
@@ -287,9 +281,9 @@ static int print_ld_blks(const uint64_t *b, const char *end, int start_line,
 	}
 	if (prnt)
 		eol(0);
-	if (!found_tblk || !is_meta_ld)
+	if (tblk_off && (!found_tblk || !is_meta_ld))
 		*tblk_off = 0;
-	if (!found_bblk || !is_meta_ld)
+	if (bblk_off && (!found_bblk || !is_meta_ld))
 		*bblk_off = 0;
 	return bcount;
 }
@@ -443,7 +437,7 @@ void dump_journal(const char *journal, int tblk)
 {
 	struct gfs2_buffer_head *j_bh = NULL, dummy_bh;
 	uint64_t jblock, j_size, jb, abs_block, saveblk, wrappt = 0;
-	int error, start_line, journal_num;
+	int start_line, journal_num;
 	struct gfs2_inode *j_inode = NULL;
 	int ld_blocks = 0, offset_from_ld = 0;
 	uint64_t tblk_off = 0, bblk_off = 0, bitblk = 0;
@@ -454,7 +448,6 @@ void dump_journal(const char *journal, int tblk)
 
 	start_line = line;
 	lines_per_row[dmode] = 1;
-	error = 0;
 	journal_num = atoi(journal + 7);
 	print_gfs2("Dumping journal #%d.", journal_num);
 	if (tblk) {
@@ -518,6 +511,7 @@ void dump_journal(const char *journal, int tblk)
 
 	for (jb = 0; jb < j_size; jb += (sbd.gfs1 ? 1 : sbd.bsize)) {
 		int is_pertinent = 1;
+		uint32_t block_type = 0;
 
 		if (sbd.gfs1) {
 			if (j_bh)
@@ -526,7 +520,7 @@ void dump_journal(const char *journal, int tblk)
 			j_bh = bread(&sbd, abs_block);
 			dummy_bh.b_data = j_bh->b_data;
 		} else {
-			error = fsck_readi(j_inode, (void *)jbuf,
+			int error = fsck_readi(j_inode, (void *)jbuf,
 					   ((jb + wrappt) % j_size),
 					   sbd.bsize, &abs_block);
 			if (!error) /* end of file */
@@ -534,14 +528,14 @@ void dump_journal(const char *journal, int tblk)
 			dummy_bh.b_data = jbuf;
 		}
 		offset_from_ld++;
-		if (get_block_type(&dummy_bh, NULL) == GFS2_METATYPE_LD) {
+		block_type = get_block_type(&dummy_bh, NULL);
+		if (block_type == GFS2_METATYPE_LD) {
 			ld_blocks = process_ld(abs_block, wrappt, j_size, jb,
 					       &dummy_bh, tblk, &tblk_off,
 					       bitblk, rgd, &is_pertinent,
 					       &bblk_off, ld_blk_refs);
 			offset_from_ld = 0;
-		} else if (!tblk &&
-			   get_block_type(&dummy_bh, NULL) == GFS2_METATYPE_LH) {
+		} else if (!tblk && block_type == GFS2_METATYPE_LH) {
 			struct gfs2_log_header lh;
 			struct gfs_log_header lh1;
 
@@ -577,7 +571,7 @@ void dump_journal(const char *journal, int tblk)
 						    sbd.bsize), start_line,
 						   tblk, &tblk_off, 0, rgd,
 						   0, 1, NULL, 1, NULL);
-		} else if (!is_meta(&dummy_bh)) {
+		} else if (block_type == 0) {
 			continue;
 		}
 		/* Check if this metadata block references the block we're
diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 5bf7963..c9f2d0a 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -662,7 +662,7 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 		 * If we don't, we may run into metadata allocation issues. */
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_UNLINKED);
 		for (j = 0; j < m; j++) {
-			blktype = save_block(sbd.device_fd, mfd, block);
+			save_block(sbd.device_fd, mfd, block);
 		}
 	}
 	free(ibuf);
@@ -933,6 +933,7 @@ static int restore_data(int fd, gzFile gzin_fd, int printblocksonly,
 			if (printblocksonly > 1 && printblocksonly == block) {
 				block_in_mem = block;
 				display(0, 0, 0, 0);
+				bh = NULL;
 				return 0;
 			} else if (printblocksonly == 1) {
 				print_gfs2("%d (l=0x%x): ", blks_saved,
@@ -944,19 +945,10 @@ static int restore_data(int fd, gzFile gzin_fd, int printblocksonly,
 			if (savedata->blk >= sbd.fssize) {
 				printf("\nOut of space on the destination "
 				       "device; quitting.\n");
+				bh = NULL;
 				break;
 			}
-			if (lseek(fd, savedata->blk * sbd.bsize, SEEK_SET) !=
-			    savedata->blk * sbd.bsize) {
-				fprintf(stderr, "bad seek: %s from %s:"
-					"%d: block %lld (0x%llx)\n",
-					strerror(errno), __FUNCTION__,
-					__LINE__,
-					(unsigned long long)savedata->blk,
-					(unsigned long long)savedata->blk);
-				exit(-1);
-			}
-			if (write(fd, savedata->buf, sbd.bsize) != sbd.bsize) {
+			if (pwrite(fd, savedata->buf, sbd.bsize, savedata->blk * sbd.bsize) != sbd.bsize) {
 				fprintf(stderr, "write error: %s from "
 					"%s:%d: block %lld (0x%llx)\n",
 					strerror(errno), __FUNCTION__,
@@ -970,6 +962,8 @@ static int restore_data(int fd, gzFile gzin_fd, int printblocksonly,
 				fsync(fd);
 		}
 		blks_saved++;
+		/* Don't leave a dangling reference to our local dummy_bh */
+		bh = NULL;
 	}
 	if (!printblocksonly && !find_highblk)
 		warm_fuzzy_stuff(sbd.fssize, TRUE);
-- 
1.8.5.3


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]