xfs
[Top] [All Lists]

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

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH] repair: fix wrong logic when validating node magic number
From: Eryu Guan <eguan@xxxxxxxxxx>
Date: Wed, 2 Sep 2015 10:19:20 +0800
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150901140442.GA16640@xxxxxxxxxxxxxxx>
References: <1439449276-1699-1-git-send-email-eguan@xxxxxxxxxx> <20150813071524.GI17933@xxxxxxxxxxxxxxxxxxxxxxxxxx> <20150901140442.GA16640@xxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Tue, Sep 01, 2015 at 10:04:43AM -0400, Brian Foster wrote:
> 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@xxxxxxxxxx>
> > 
> > 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?

Test passed after applying this patch, thanks! Tested by shared/002 and
the simplified reproducer above.

Thanks,
Eryu

> 
> 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);
>  
>               /*

<Prev in Thread] Current Thread [Next in Thread>