commit 304bce8e5d34fae0aea3d2438b65ba163001a806 Author: Bob Peterson <rpeterso@redhat.com> Date: Thu Aug 18 15:58:13 2011 -0500 fsck.gfs2: Bad extended attributes not deleted properly This patch fixes a couple of problems fsck.gfs2 when it discovered corrupt extended attributes and tried to delete them. First, in check_eattr_entries it wasn't reporting the corruption. Second, when the EA block was freed, it wasn't returning a proper return code which caused fsck to not update the inode accordingly. Third, it wasn't zeroing out the EA block address properly. Fourth, I did a small amount of reformatting. rhbz#877150 diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index b7fff1c..4dd021f 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -735,6 +735,13 @@ static int check_eattr_entries(struct gfs2_inode *ip, bh, ea_hdr, ea_hdr_prev, pass->private)) { + log_err(_("Bad extended attribute " + "found at block %lld " + "(0x%llx)"), + (unsigned long long) + be64_to_cpu(*ea_data_ptr), + (unsigned long long) + be64_to_cpu(*ea_data_ptr)); if (query( _("Repair the bad Extended " "Attribute? (y/n) "))) { ea_hdr->ea_num_ptrs = i; @@ -787,12 +794,14 @@ static int check_leaf_eattr(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct metawalk_fxns *pass) { struct gfs2_buffer_head *bh = NULL; - int error = 0; - - log_debug( _("Checking EA leaf block #%"PRIu64" (0x%" PRIx64 ").\n"), - block, block); if (pass->check_eattr_leaf) { + int error = 0; + + log_debug( _("Checking EA leaf block #%llu (0x%llx).\n"), + (unsigned long long)block, + (unsigned long long)block); + error = pass->check_eattr_leaf(ip, block, parent, &bh, pass->private); if (error < 0) { @@ -866,13 +875,18 @@ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype) /** * free_block_if_notdup - free blocks associated with an inode, but if it's a * duplicate, just remove that designation instead. - * Returns: 0 if the block was freed, 1 if a duplicate reference was removed + * Returns: 1 if the block was freed, 0 if a duplicate reference was removed + * Note: The return code is handled this way because there are places in + * metawalk.c that assume "1" means "change was made" and "0" means + * change was not made. */ int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block, const char *btype) { - if (!find_remove_dup(ip, block, btype)) + if (!find_remove_dup(ip, block, btype)) { /* not a dup */ fsck_blockmap_set(ip, block, btype, gfs2_block_free); + return 1; + } return 0; } @@ -1420,16 +1434,36 @@ int delete_data(struct gfs2_inode *ip, uint64_t block, void *private) int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - return delete_block_if_notdup(ip, block, NULL, - _("indirect extended attribute"), - private); + int ret; + + ret = delete_block_if_notdup(ip, block, NULL, + _("indirect extended attribute"), + private); + /* Even if it's a duplicate reference, we want to eliminate the + reference itself, and adjust di_blocks accordingly. */ + if (ip->i_di.di_eattr) { + ip->i_di.di_blocks--; + if (block == ip->i_di.di_eattr) + ip->i_di.di_eattr = 0; + bmodified(ip->i_bh); + } + return ret; } int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent, struct gfs2_buffer_head **bh, void *private) { - return delete_block_if_notdup(ip, block, NULL, _("extended attribute"), - private); + int ret; + + ret = delete_block_if_notdup(ip, block, NULL, _("extended attribute"), + private); + if (ip->i_di.di_eattr) { + ip->i_di.di_blocks--; + if (block == ip->i_di.di_eattr) + ip->i_di.di_eattr = 0; + bmodified(ip->i_bh); + } + return ret; } static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c index 47891f6..62cc96e 100644 --- a/gfs2/fsck/pass1.c +++ b/gfs2/fsck/pass1.c @@ -1169,9 +1169,7 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip) (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)ip->i_di.di_num.no_addr); check_metatree(ip, &invalidate_fxns); - if (fsck_blockmap_set(ip, block, _("invalid mode"), - gfs2_inode_invalid)) - goto bad_dinode; + check_inode_eattr(ip, &invalidate_fxns); return 0; } else if (error) goto bad_dinode; diff --git a/gfs2/fsck/pass1c.c b/gfs2/fsck/pass1c.c index f362e35..48ea430 100644 --- a/gfs2/fsck/pass1c.c +++ b/gfs2/fsck/pass1c.c @@ -75,9 +75,10 @@ static int ask_remove_eattr(struct gfs2_inode *ip) ip->i_di.di_eattr = 0; bmodified(ip->i_bh); log_err( _("Bad Extended Attribute removed.\n")); - } else - log_err( _("Bad Extended Attribute not removed.\n")); - return 1; + return 1; + } + log_err( _("Bad Extended Attribute not removed.\n")); + return 0; } static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block, diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 3c5bda1..71f305f 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -215,9 +215,9 @@ static int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr, struct gfs2_ea_header *ea_hdr_prev, void *private) { - uint64_t block = be64_to_cpu(*ea_data_ptr); + uint64_t block = be64_to_cpu(*ea_data_ptr); - return delete_metadata(ip, block, NULL, 0, private); + return delete_metadata(ip, block, NULL, 0, private); } struct metawalk_fxns pass2_fxns_delete = { @@ -355,7 +355,10 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, entry_ip = ip; else entry_ip = fsck_load_inode(sdp, entryblock); - check_inode_eattr(entry_ip, &pass2_fxns_delete); + if (ip->i_di.di_eattr) { + check_inode_eattr(entry_ip, + &pass2_fxns_delete); + } check_metatree(entry_ip, &pass2_fxns_delete); if (entry_ip != ip) fsck_inode_put(&entry_ip);