From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Fri, 28 Feb 2014 09:35:34 +0000 Subject: [Cluster-devel] [PATCH 7/7] gfs2_edit: More static analysis fixes In-Reply-To: <1393580134-21574-1-git-send-email-anprice@redhat.com> References: <1393580134-21574-1-git-send-email-anprice@redhat.com> Message-ID: <1393580134-21574-8-git-send-email-anprice@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 --- 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