From: Bob Peterson <rpete...@redhat.com> This patch fixes several problems detected by the coverity tool whereby variables could be accessed without being initialized, and where return codes should be checked and were not.
rhbz#675723 --- gfs2/fsck/fs_recovery.c | 9 ++++ gfs2/fsck/initialize.c | 107 +++++++++++++++++++++++++++++++++++++--------- gfs2/fsck/lost_n_found.c | 18 ++++++- gfs2/fsck/pass1.c | 19 ++++++--- gfs2/fsck/pass2.c | 25 ++++++++--- gfs2/fsck/pass3.c | 11 +++- gfs2/fsck/pass5.c | 17 ++++++-- gfs2/libgfs2/fs_ops.c | 41 ++++++++++++------ gfs2/libgfs2/libgfs2.h | 2 +- 9 files changed, 190 insertions(+), 59 deletions(-) diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c index 52303dd..d07a515 100644 --- a/gfs2/fsck/fs_recovery.c +++ b/gfs2/fsck/fs_recovery.c @@ -125,6 +125,10 @@ static int buf_lo_scan_elements(struct gfs2_inode *ip, unsigned int start, (unsigned long long)blkno, (unsigned long long)blkno, start); bh_ip = bget(sdp, blkno); + if (!bh_ip) { + log_err(_("Out of memory when replaying journals.\n")); + return FSCK_ERROR; + } memcpy(bh_ip->b_data, bh_log->b_data, sdp->bsize); check_magic = ((struct gfs2_meta_header *) @@ -233,6 +237,10 @@ static int databuf_lo_scan_elements(struct gfs2_inode *ip, unsigned int start, (unsigned long long)blkno, (unsigned long long)blkno, start); bh_ip = bget(sdp, blkno); + if (!bh_ip) { + log_err(_("Out of memory when replaying journals.\n")); + return FSCK_ERROR; + } memcpy(bh_ip->b_data, bh_log->b_data, sdp->bsize); /* Unescape */ @@ -357,6 +365,7 @@ static int fix_journal_seq_no(struct gfs2_inode *ip) uint32_t extlen; struct gfs2_buffer_head *bh; + memset(&lh, 0, sizeof(lh)); for (blk = 0; blk < jd_blocks; blk++) { error = get_log_header(ip, blk, &lh); if (error == 1) /* if not a log header */ diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c index 6a92992..28fe03d 100644 --- a/gfs2/fsck/initialize.c +++ b/gfs2/fsck/initialize.c @@ -352,6 +352,7 @@ static int rebuild_master(struct gfs2_sbd *sdp) { struct gfs2_inum inum; struct gfs2_buffer_head *bh; + int err = 0; log_err(_("The system master directory seems to be destroyed.\n")); if (!query(_("Okay to rebuild it? (y/n)"))) { @@ -368,59 +369,109 @@ static int rebuild_master(struct gfs2_sbd *sdp) if (fix_md.jiinode) { inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = fix_md.jiinode->i_di.di_num.no_addr; - dir_add(sdp->master_dir, "jindex", 6, &inum, - IF2DT(S_IFDIR | 0700)); + err = dir_add(sdp->master_dir, "jindex", 6, &inum, + IF2DT(S_IFDIR | 0700)); + if (err) { + log_crit(_("Error %d adding jindex directory\n"), errno); + exit(FSCK_ERROR); + } sdp->master_dir->i_di.di_nlink++; } else { - build_jindex(sdp); + err = build_jindex(sdp); + if (err) { + log_crit(_("Error %d building jindex\n"), err); + exit(FSCK_ERROR); + } } if (fix_md.pinode) { inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = fix_md.pinode->i_di.di_num.no_addr; - dir_add(sdp->master_dir, "per_node", 8, &inum, + err = dir_add(sdp->master_dir, "per_node", 8, &inum, IF2DT(S_IFDIR | 0700)); + if (err) { + log_crit(_("Error %d adding per_node directory\n"), + errno); + exit(FSCK_ERROR); + } sdp->master_dir->i_di.di_nlink++; } else { - build_per_node(sdp); + err = build_per_node(sdp); + if (err) { + log_crit(_("Error %d building per_node directory\n"), + err); + exit(FSCK_ERROR); + } } if (fix_md.inum) { inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = fix_md.inum->i_di.di_num.no_addr; - dir_add(sdp->master_dir, "inum", 4, &inum, + err = dir_add(sdp->master_dir, "inum", 4, &inum, IF2DT(S_IFREG | 0600)); + if (err) { + log_crit(_("Error %d adding inum inode\n"), errno); + exit(FSCK_ERROR); + } } else { - build_inum(sdp); + err = build_inum(sdp); + if (err) { + log_crit(_("Error %d building inum inode\n"), err); + exit(FSCK_ERROR); + } gfs2_lookupi(sdp->master_dir, "inum", 4, &sdp->md.inum); } if (fix_md.statfs) { inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = fix_md.statfs->i_di.di_num.no_addr; - dir_add(sdp->master_dir, "statfs", 6, &inum, - IF2DT(S_IFREG | 0600)); + err = dir_add(sdp->master_dir, "statfs", 6, &inum, + IF2DT(S_IFREG | 0600)); + if (err) { + log_crit(_("Error %d adding statfs inode\n"), errno); + exit(FSCK_ERROR); + } } else { - build_statfs(sdp); + err = build_statfs(sdp); + if (err) { + log_crit(_("Error %d building statfs inode\n"), err); + exit(FSCK_ERROR); + } gfs2_lookupi(sdp->master_dir, "statfs", 6, &sdp->md.statfs); } if (fix_md.riinode) { inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = fix_md.riinode->i_di.di_num.no_addr; - dir_add(sdp->master_dir, "rindex", 6, &inum, + err = dir_add(sdp->master_dir, "rindex", 6, &inum, IF2DT(S_IFREG | 0600)); + if (err) { + log_crit(_("Error %d adding rindex inode\n"), errno); + exit(FSCK_ERROR); + } } else { - build_rindex(sdp); + err = build_rindex(sdp); + if (err) { + log_crit(_("Error %d building rindex inode\n"), err); + exit(FSCK_ERROR); + } } if (fix_md.qinode) { inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = fix_md.qinode->i_di.di_num.no_addr; - dir_add(sdp->master_dir, "quota", 5, &inum, + err = dir_add(sdp->master_dir, "quota", 5, &inum, IF2DT(S_IFREG | 0600)); + if (err) { + log_crit(_("Error %d adding quota inode\n"), errno); + exit(FSCK_ERROR); + } } else { - build_quota(sdp); + err = build_quota(sdp); + if (err) { + log_crit(_("Error %d building quota inode\n"), err); + exit(FSCK_ERROR); + } } log_err(_("Master directory rebuilt.\n")); @@ -590,8 +641,13 @@ static int init_system_inodes(struct gfs2_sbd *sdp) } } /* Read inum entry into buffer */ - gfs2_readi(sdp->md.inum, &inumbuf, 0, - sdp->md.inum->i_di.di_size); + err = gfs2_readi(sdp->md.inum, &inumbuf, 0, + sdp->md.inum->i_di.di_size); + if (err != sdp->md.inum->i_di.di_size) { + log_crit(_("Error %d reading system inum inode. " + "Aborting.\n"), err); + goto fail; + } /* call gfs2_inum_range_in() to retrieve range */ sdp->md.next_inum = be64_to_cpu(inumbuf); } @@ -624,8 +680,14 @@ static int init_system_inodes(struct gfs2_sbd *sdp) if (sdp->md.statfs->i_di.di_size) { buf = malloc(sdp->md.statfs->i_di.di_size); if (buf) { - gfs2_readi(sdp->md.statfs, buf, 0, - sdp->md.statfs->i_di.di_size); + err = gfs2_readi(sdp->md.statfs, buf, 0, + sdp->md.statfs->i_di.di_size); + if (err != sdp->md.statfs->i_di.di_size) { + log_crit(_("Error %d reading statfs file. " + "Aborting.\n"), err); + free(buf); + goto fail; + } /* call gfs2_inum_range_in() to retrieve range */ gfs2_statfs_change_in(&sc, buf); free(buf); @@ -1251,14 +1313,17 @@ static int reconstruct_single_journal(struct gfs2_sbd *sdp, int jnum, */ static int reconstruct_journals(struct gfs2_sbd *sdp) { - int i; + int i, count; struct gfs_jindex ji; char buf[sizeof(struct gfs_jindex)]; log_err("Clearing GFS journals (this may take a while)\n"); for (i = 0; i < sdp->md.journals; i++) { - gfs2_readi(sdp->md.jiinode, buf, i * sizeof(struct gfs_jindex), - sizeof(struct gfs_jindex)); + count = gfs2_readi(sdp->md.jiinode, buf, + i * sizeof(struct gfs_jindex), + sizeof(struct gfs_jindex)); + if (count != sizeof(struct gfs_jindex)) + return 0; gfs_jindex_in(&ji, buf); if ((i % 2) == 0) log_err("."); diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index 70a9181..6f09de1 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -19,6 +19,7 @@ static void add_dotdot(struct gfs2_inode *ip) { struct dir_info *di; struct gfs2_sbd *sdp = ip->i_sbd; + int err; log_info( _("Adding .. entry to directory %llu (0x%llx) pointing back " "to lost+found\n"), @@ -75,8 +76,13 @@ static void add_dotdot(struct gfs2_inode *ip) log_warn( _("add_inode_to_lf: Unable to remove " "\"..\" directory entry.\n")); - dir_add(ip, "..", 2, &(lf_dip->i_di.di_num), - (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR)); + err = dir_add(ip, "..", 2, &(lf_dip->i_di.di_num), + (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR)); + if (err) { + log_crit(_("Error adding .. directory: %s\n"), + strerror(errno)); + exit(FSCK_ERROR); + } } static uint64_t find_free_blk(struct gfs2_sbd *sdp) @@ -133,6 +139,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){ uint64_t lf_blocks; struct gfs2_sbd *sdp = ip->i_sbd; struct dir_info *di; + int err = 0; uint32_t mode; if (!lf_dip) { @@ -255,8 +262,13 @@ int add_inode_to_lf(struct gfs2_inode *ip){ break; } - dir_add(lf_dip, tmp_name, strlen(tmp_name), &(ip->i_di.di_num), + err = dir_add(lf_dip, tmp_name, strlen(tmp_name), &(ip->i_di.di_num), inode_type); + if (err) { + log_crit(_("Error adding directory %s: %s\n"), + tmp_name, strerror(errno)); + exit(FSCK_ERROR); + } /* If the lf directory had new blocks added we have to mark them properly in the bitmap so they're not freed. */ if (lf_dip->i_di.di_blocks != lf_blocks) diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index b3ef7d4..52d4082 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1326,7 +1326,7 @@ static int check_system_inode(struct gfs2_sbd *sdp, { uint64_t iblock = 0; struct dir_status ds = {0}; - int error; + int error, err = 0; log_info( _("Checking system inode '%s'\n"), filename); if (*sysinode) { @@ -1400,12 +1400,19 @@ static int check_system_inode(struct gfs2_sbd *sdp, sysdir_fxns.private = &bc; if ((*sysinode)->i_di.di_flags & GFS2_DIF_EXHASH) check_metatree(*sysinode, &sysdir_fxns); - else - check_linear_dir(*sysinode, (*sysinode)->i_bh, - &sysdir_fxns); + else { + err = check_linear_dir(*sysinode, (*sysinode)->i_bh, + &sysdir_fxns); + /* If we encountered an error in our directory check + we should still call handle_ip, but return the + error later. */ + if (err) + log_err(_("Error found in %s while checking " + "directory entries.\n"), filename); + } } error = handle_ip(sdp, *sysinode); - return error; + return error ? error : err; } static int build_a_journal(struct gfs2_sbd *sdp) @@ -1537,7 +1544,7 @@ int pass1(struct gfs2_sbd *sdp) { struct osi_node *n, *next = NULL; struct gfs2_buffer_head *bh; - uint64_t block; + uint64_t block = 0; struct rgrp_tree *rgd; int first; uint64_t i; diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index be0e5fe..c772be3 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -667,10 +667,15 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname, } memcpy(filename, tmp_name, filename_len); log_warn( _("Adding '.' entry\n")); - dir_add(sysinode, filename, filename_len, - &(sysinode->i_di.di_num), - (sysinode->i_sbd->gfs1 ? - GFS_FILE_DIR : DT_DIR)); + error = dir_add(sysinode, filename, filename_len, + &(sysinode->i_di.di_num), + (sysinode->i_sbd->gfs1 ? + GFS_FILE_DIR : DT_DIR)); + if (error) { + log_err(_("Error adding directory %s: %s\n"), + filename, strerror(errno)); + return -errno; + } if (cur_blks != sysinode->i_di.di_blocks) reprocess_inode(sysinode, dirname); /* This system inode is linked to itself via '.' */ @@ -881,9 +886,15 @@ int pass2(struct gfs2_sbd *sdp) memcpy(filename, tmp_name, filename_len); cur_blks = ip->i_di.di_blocks; - dir_add(ip, filename, filename_len, - &(ip->i_di.di_num), - (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR)); + error = dir_add(ip, filename, filename_len, + &(ip->i_di.di_num), + (sdp->gfs1 ? GFS_FILE_DIR : + DT_DIR)); + if (error) { + log_err(_("Error adding directory %s: %s\n"), + filename, strerror(errno)); + return -errno; + } if (cur_blks != ip->i_di.di_blocks) { char dirname[80]; diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c index e1accc9..daa1809 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -18,7 +18,7 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t newdotdot, uint64_t olddotdot, uint64_t block) { char *filename; - int filename_len; + int filename_len, err; struct gfs2_inode *ip, *pip; uint64_t cur_blks; @@ -52,8 +52,13 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t newdotdot, else decr_link_count(olddotdot, block, _("old \"..\"")); cur_blks = ip->i_di.di_blocks; - dir_add(ip, filename, filename_len, &pip->i_di.di_num, - (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR)); + err = dir_add(ip, filename, filename_len, &pip->i_di.di_num, + (sdp->gfs1 ? GFS_FILE_DIR : DT_DIR)); + if (err) { + log_err(_("Error adding directory %s: %s\n"), + filename, strerror(errno)); + exit(FSCK_ERROR); + } if (cur_blks != ip->i_di.di_blocks) { char dirname[80]; diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c index ea033a0..2540b17 100644 --- a/gfs2/fsck/pass5.c +++ b/gfs2/fsck/pass5.c @@ -104,7 +104,8 @@ static int check_block_status(struct gfs2_sbd *sdp, char *buffer, { unsigned char *byte, *end; unsigned int bit; - unsigned char rg_status, block_status; + unsigned char rg_status; + int block_status; uint8_t q; uint64_t block; @@ -119,6 +120,7 @@ static int check_block_status(struct gfs2_sbd *sdp, char *buffer, warm_fuzzy_stuff(block); if (skip_this_pass || fsck_abort) /* if asked to skip the rest */ return 0; + q = block_type(block); if (sdp->gfs1) @@ -126,6 +128,12 @@ static int check_block_status(struct gfs2_sbd *sdp, char *buffer, else block_status = gfs2_convert_mark(q, count); + if (block_status < 0) { + log_err( _("Invalid status for block %llu (0x%llx).\n"), + (unsigned long long)block, + (unsigned long long)block); + return block_status; + } /* If one node opens a file and another node deletes it, we may be left with a block that appears to be "unlinked" in the bitmap, but nothing links to it. This is a valid case @@ -204,9 +212,10 @@ static void update_rgrp(struct gfs2_sbd *sdp, struct rgrp_tree *rgp, bits = &rgp->bits[i]; /* update the bitmaps */ - check_block_status(sdp, rgp->bh[i]->b_data + bits->bi_offset, - bits->bi_len, &rg_block, rgp->ri.ri_data0, - count); + if (check_block_status(sdp, rgp->bh[i]->b_data + + bits->bi_offset, bits->bi_len, + &rg_block, rgp->ri.ri_data0, count)) + return; if (skip_this_pass || fsck_abort) /* if asked to skip the rest */ return; } diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c index 5053c90..4f1774c 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -723,6 +723,11 @@ int gfs2_dirent_next(struct gfs2_inode *dip, struct gfs2_buffer_head *bh, return 0; } +/** + * Allocate a gfs2 dirent + * Returns 0 on success, with *dent_out pointing to the new dirent, + * or -1 on failure, with errno set + */ static int dirent_alloc(struct gfs2_inode *dip, struct gfs2_buffer_head *bh, int name_len, struct gfs2_dirent **dent_out) { @@ -793,7 +798,8 @@ static int dirent_alloc(struct gfs2_inode *dip, struct gfs2_buffer_head *bh, } } while (gfs2_dirent_next(dip, bh, &dent) == 0); - return -ENOSPC; + errno = ENOSPC; + return -1; } void dirent2_del(struct gfs2_inode *dip, struct gfs2_buffer_head *bh, @@ -1032,8 +1038,6 @@ int gfs2_get_leaf(struct gfs2_inode *dip, uint64_t leaf_no, int error = 0; *bhp = bread(dip->i_sbd, leaf_no); - if (error) - return error; error = gfs2_check_meta(*bhp, GFS2_METATYPE_LF); if(error) brelse(*bhp); @@ -1081,7 +1085,7 @@ static int get_next_leaf(struct gfs2_inode *dip,struct gfs2_buffer_head *bh_in, return 0; } -static void dir_e_add(struct gfs2_inode *dip, const char *filename, int len, +static int dir_e_add(struct gfs2_inode *dip, const char *filename, int len, struct gfs2_inum *inum, unsigned int type) { struct gfs2_buffer_head *bh, *nbh; @@ -1090,6 +1094,7 @@ static void dir_e_add(struct gfs2_inode *dip, const char *filename, int len, uint32_t lindex; uint32_t hash; uint64_t leaf_no, bn; + int err = 0; hash = gfs2_disk_hash(filename, len); restart: @@ -1138,8 +1143,9 @@ restart: nleaf->lf_depth = leaf->lf_depth; nleaf->lf_dirent_format = cpu_to_be32(GFS2_FORMAT_DE); - if (dirent_alloc(dip, nbh, len, &dent)) - die("dir_split_leaf (3)\n"); + err = dirent_alloc(dip, nbh, len, &dent); + if (err) + return err; dip->i_di.di_blocks++; bmodified(dip->i_bh); bmodified(bh); @@ -1159,7 +1165,7 @@ restart: bmodified(bh); brelse(bh); - return; + return err; } } @@ -1227,15 +1233,16 @@ static void dir_make_exhash(struct gfs2_inode *dip) bwrite(dip->i_bh); } -static void dir_l_add(struct gfs2_inode *dip, const char *filename, int len, +static int dir_l_add(struct gfs2_inode *dip, const char *filename, int len, struct gfs2_inum *inum, unsigned int type) { struct gfs2_dirent *dent; + int err = 0; if (dirent_alloc(dip, dip->i_bh, len, &dent)) { dir_make_exhash(dip); - dir_e_add(dip, filename, len, inum, type); - return; + err = dir_e_add(dip, filename, len, inum, type); + return err; } gfs2_inum_out(inum, (char *)&dent->de_inum); @@ -1244,15 +1251,18 @@ static void dir_l_add(struct gfs2_inode *dip, const char *filename, int len, dent->de_type = cpu_to_be16(type); memcpy((char *)(dent + 1), filename, len); bmodified(dip->i_bh); + return err; } -void dir_add(struct gfs2_inode *dip, const char *filename, int len, +int dir_add(struct gfs2_inode *dip, const char *filename, int len, struct gfs2_inum *inum, unsigned int type) { + int err = 0; if (dip->i_di.di_flags & GFS2_DIF_EXHASH) - dir_e_add(dip, filename, len, inum, type); + err = dir_e_add(dip, filename, len, inum, type); else - dir_l_add(dip, filename, len, inum, type); + err = dir_l_add(dip, filename, len, inum, type); + return err; } static struct gfs2_buffer_head *__init_dinode(struct gfs2_sbd *sdp, @@ -1350,6 +1360,7 @@ static struct gfs2_inode *__createi(struct gfs2_inode *dip, struct gfs2_inum inum; struct gfs2_buffer_head *bh; struct gfs2_inode *ip; + int err = 0; int is_dir; gfs2_lookupi(dip, filename, strlen(filename), &ip); @@ -1362,7 +1373,9 @@ static struct gfs2_inode *__createi(struct gfs2_inode *dip, inum.no_formal_ino = sdp->md.next_inum++; inum.no_addr = bn; - dir_add(dip, filename, strlen(filename), &inum, IF2DT(mode)); + err = dir_add(dip, filename, strlen(filename), &inum, IF2DT(mode)); + if (err) + return NULL; if (if_gfs1) is_dir = (IF2DT(mode) == GFS_FILE_DIR); diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index bb47580..f60654c 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -385,7 +385,7 @@ extern int dir_search(struct gfs2_inode *dip, const char *filename, int len, unsigned int *type, struct gfs2_inum *inum); extern int gfs2_lookupi(struct gfs2_inode *dip, const char *filename, int len, struct gfs2_inode **ipp); -extern void dir_add(struct gfs2_inode *dip, const char *filename, int len, +extern int dir_add(struct gfs2_inode *dip, const char *filename, int len, struct gfs2_inum *inum, unsigned int type); extern int gfs2_dirent_del(struct gfs2_inode *dip, const char *filename, int filename_len); -- 1.7.7.5