Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=b2d1e993d3688c... Commit: b2d1e993d3688ccae39a1680615c425953662861 Parent: f6e426b5aa22692e414192d5aa70e4ef15db1c18 Author: Bob Peterson rpeterso@redhat.com AuthorDate: Thu Jan 12 15:29:17 2012 -0600 Committer: Bob Peterson rpeterso@redhat.com CommitterDate: Fri Apr 5 06:25:01 2013 -0700
fsck.gfs2: Add return code checks and initializations
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#877150 --- gfs2/fsck/fs_recovery.c | 9 ++++ gfs2/fsck/initialize.c | 99 ++++++++++++++++++++++++++++++++++++++------- gfs2/fsck/lost_n_found.c | 16 ++++++- gfs2/fsck/pass1.c | 17 ++++++-- gfs2/fsck/pass2.c | 19 +++++++-- gfs2/fsck/pass3.c | 10 ++++- gfs2/fsck/pass5.c | 17 ++++++-- gfs2/libgfs2/fs_ops.c | 45 +++++++++++++------- gfs2/libgfs2/libgfs2.h | 2 +- 9 files changed, 184 insertions(+), 50 deletions(-)
diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c index 61c3b9d..214aa18 100644 --- a/gfs2/fsck/fs_recovery.c +++ b/gfs2/fsck/fs_recovery.c @@ -139,6 +139,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 *) @@ -245,6 +249,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 */ @@ -369,6 +377,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 df955eb..75f81f5 100644 --- a/gfs2/fsck/initialize.c +++ b/gfs2/fsck/initialize.c @@ -340,6 +340,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)"))) { @@ -356,59 +357,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,6 +641,16 @@ static int init_system_inodes(struct gfs2_sbd *sdp) goto fail; } } + /* Read inum entry into buffer */ + 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); } /* Read inum entry into buffer */ gfs2_readi(sdp->md.inum, &inumbuf, 0, @@ -622,8 +683,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); diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c index afdeefc..c0de5d5 100644 --- a/gfs2/fsck/lost_n_found.c +++ b/gfs2/fsck/lost_n_found.c @@ -33,6 +33,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"), @@ -89,7 +90,12 @@ 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), DT_DIR); + err = dir_add(ip, "..", 2, &(lf_dip->i_di.di_num), DT_DIR); + if (err) { + log_crit(_("Error adding .. directory: %s\n"), + strerror(errno)); + exit(FSCK_ERROR); + } }
/* add_inode_to_lf - Add dir entry to lost+found for the inode @@ -107,6 +113,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) { @@ -207,8 +214,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 c241960..8bda4b8 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1315,7 +1315,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) { @@ -1387,12 +1387,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) diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 33eb2d4..e89640b 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -18,6 +18,7 @@ #include <inttypes.h> #include <sys/stat.h> #include <libintl.h> +#include <errno.h> #define _(String) gettext(String)
#include "libgfs2.h" @@ -679,8 +680,13 @@ 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), DT_DIR); + error = dir_add(sysinode, filename, filename_len, + &(sysinode->i_di.di_num), 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 '.' */ @@ -883,8 +889,13 @@ 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), DT_DIR); + error = dir_add(ip, filename, filename_len, + &(ip->i_di.di_num), 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 db59191..ebf4800 100644 --- a/gfs2/fsck/pass3.c +++ b/gfs2/fsck/pass3.c @@ -17,6 +17,7 @@ #include <string.h> #include <dirent.h> #include <libintl.h> +#include <errno.h> #define _(String) gettext(String)
#include "libgfs2.h" @@ -31,7 +32,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;
@@ -65,7 +66,12 @@ 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, DT_DIR); + err = dir_add(ip, filename, filename_len, &pip->i_di.di_num, 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 fc2eb80..f2d23cd 100644 --- a/gfs2/fsck/pass5.c +++ b/gfs2/fsck/pass5.c @@ -65,7 +65,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;
@@ -80,10 +81,17 @@ 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);
block_status = 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 @@ -161,9 +169,10 @@ static void update_rgrp(struct gfs2_sbd *sdp, struct rgrp_list *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 0691956..4e23b4a 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -711,6 +711,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) { @@ -778,7 +783,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, @@ -997,8 +1003,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); @@ -1046,7 +1050,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; @@ -1055,6 +1059,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: @@ -1103,8 +1108,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); @@ -1124,7 +1130,7 @@ restart:
bmodified(bh); brelse(bh); - return; + return err; } }
@@ -1191,15 +1197,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, - struct gfs2_inum *inum, unsigned int type) +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); @@ -1208,15 +1215,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, - struct gfs2_inum *inum, unsigned int type) +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; }
struct gfs2_buffer_head * @@ -1290,6 +1300,7 @@ struct gfs2_inode *createi(struct gfs2_inode *dip, const char *filename, struct gfs2_inum inum; struct gfs2_buffer_head *bh; struct gfs2_inode *ip; + int err = 0;
gfs2_lookupi(dip, filename, strlen(filename), &ip); if (!ip) { @@ -1298,7 +1309,9 @@ struct gfs2_inode *createi(struct gfs2_inode *dip, const char *filename, 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(S_ISDIR(mode)) { bmodified(dip->i_bh); diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 2c2bce5..87c65c6 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -380,7 +380,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);