[PATCH] repair: fix wrong logic when validating node magic number

[Brian Foster]

On Thu, Aug 13, 2015 at 03:15:24PM +0800, Eryu Guan wrote:
> On Thu, Aug 13, 2015 at 03:01:16PM +0800, Eryu Guan wrote:
> > Magic number is wrong only when != XFS_DA_NODE_MAGIC and
> > != XFS_DA3_NODE_MAGIC.
> > 
> > This is triggered by shared/002 when testing 512 block size XFS.
> > 
> >   Phase 1 - find and verify superblock...
> >   Phase 2 - using internal log
> >           - scan filesystem freespace and inode maps...
> >           - found root inode chunk
> >   Phase 3 - for each AG...
> >           - scan (but don't clear) agi unlinked lists...
> >           - process known inodes and perform inode discovery...
> >           - agno = 0
> >   bad magic number febe in block 64 (108) for directory inode 35
> >   ......
> > 
> > Fix it by changing "||" to "&&".
> > 
> > Signed-off-by: Eryu Guan <eguan at redhat.com>
> 
> With this patch applied, shared/002 still fails on 512 block size XFS,
> full xfs_repair -n output is                                                                                                                  
>                                                                                                                                               
> *** xfs_repair -n output ***                                                                                                                  
> Phase 1 - find and verify superblock...                                                                                                       
> Phase 2 - using internal log                                                                                                                  
>         - scan filesystem freespace and inode maps...                                                                                         
>         - found root inode chunk                                                                                                              
> Phase 3 - for each AG...                                                                                                                      
>         - scan (but don't clear) agi unlinked lists...                                                                                        
>         - process known inodes and perform inode discovery...                                                                                 
>         - agno = 0                                                                                                                            
> problem with attribute contents in inode 35                                                                                                   
> would clear attr fork                                                                                                                         
> bad nblocks 67 for inode 35, would reset to 0                                                                                                 
> bad anextents 5 for inode 35, would reset to 0                                                                                                
>         - agno = 1                                                                                                                            
>         - agno = 2                                                                                                                            
>         - agno = 3                                                                                                                            
>         - process newly discovered inodes...                                                                                                  
> Phase 4 - check for duplicate blocks...                                                                                                       
>         - setting up duplicate extent list...                                                                                                 
>         - check for inodes claiming duplicate blocks...                                                                                       
>         - agno = 0                                                                                                                            
>         - agno = 1                                                                                                                            
>         - agno = 2                                                                                                                            
>         - agno = 3                                                                                                                            
> No modify flag set, skipping phase 5                                                                                                          
> Phase 6 - check inode connectivity...                                                                                                         
>         - traversing filesystem ...                                                                                                           
>         - traversal finished ...                                                                                                              
>         - moving disconnected inodes to lost+found ...                                                                                        
> Phase 7 - verify link counts...                                                                                                               
> No modify flag set, skipping filesystem flush and exiting.                                                                                    
> *** end xfs_repair output
> 
> And a simplified reproducer is just adding >= 577 xattrs to file foo on
> 512 block size XFS, no dmflaky is needed.
> 
> num_xattrs=577
> for ((i = 1; i <= $num_xattrs; i++)); do
>         name="user.attr_$(printf "%04d" $i)"
>         $SETFATTR_PROG -n $name -v "val_$(printf "%04d" $i)" $SCRATCH_MNT/foo
> done
> 
> And it's easily reproduced.
> 

Thanks for the reproducer. This looks like a bug in xfs_repair. Care to
test the appended hunk?

Brian

---8<---

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 83a07a8..b76618a 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -562,7 +562,7 @@ verify_da_path(xfs_mount_t	*mp,
 		}
 
 		newnode = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
-		btree = M_DIROPS(mp)->node_tree_p(node);
+		btree = M_DIROPS(mp)->node_tree_p(newnode);
 		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, newnode);
 
 		/*