[PATCH] xfs_repair: Fix root inode's parent when it's bogus for sf directory

[PATCH] xfs_repair: Fix root inode's parent when it's bogus for sf directory

[Marco Benatto]

Currently when root inode is in short-form and its parent ino
has an invalid value, process_sf_dir2() ends up not fixing it,
because if verify_inum() fails we never get to the next case which
would fix the root inode's parent pointer.

This behavior triggers the following assert on process_dir2():

   ASSERT((ino != mp->m_sb.sb_rootino && ino != *parent) ||
        (ino == mp->m_sb.sb_rootino &&
        (ino == *parent || need_root_dotdot == 1)));

This patch fixes this behavior by making sure we always properly
handle rootino parent pointer in process_sf_dir2()

Signed-off-by: Marco Benatto <mbenatto@redhat.com>
---
 repair/dir2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/repair/dir2.c b/repair/dir2.c
index e162d2b..225f926 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -495,8 +495,10 @@ _("corrected entry offsets in directory %" PRIu64 "\n"),
 
 	/*
 	 * if parent entry is bogus, null it out.  we'll fix it later .
+	 * If the validation fails for the root inode we fix it in
+	 * the next else case.
 	 */
-	if (verify_inum(mp, *parent))  {
+	if (verify_inum(mp, *parent) && ino != mp->m_sb.sb_rootino)  {
 
 		do_warn(
 _("bogus .. inode number (%" PRIu64 ") in directory inode %" PRIu64 ", "),
-- 
2.9.5

Re: [PATCH] xfs_repair: Fix root inode's parent when it's bogus for sf directory

[Eric Sandeen]

On 6/14/18 5:16 PM, Marco Benatto wrote:
> Currently when root inode is in short-form and its parent ino
> has an invalid value, process_sf_dir2() ends up not fixing it,
> because if verify_inum() fails we never get to the next case which
> would fix the root inode's parent pointer.
> 
> This behavior triggers the following assert on process_dir2():
> 
>    ASSERT((ino != mp->m_sb.sb_rootino && ino != *parent) ||
>         (ino == mp->m_sb.sb_rootino &&
>         (ino == *parent || need_root_dotdot == 1)));
> 
> This patch fixes this behavior by making sure we always properly
> handle rootino parent pointer in process_sf_dir2()
> 
> Signed-off-by: Marco Benatto <mbenatto@redhat.com>

This looks correct to me, thanks.

FWIW the problem can be demonstrated by setting the root inode's
parent to 0 on a freshly made fs:

# mkfs.xfs -f -dfile,name=fsfile,size=1g
# xfs_db -x -c "sb 0" -c "addr rootino" -c "write u3.sfdir3.hdr.parent.i4 0" fsfile
# xfs_repair fsfile

Signed-off-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  repair/dir2.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index e162d2b..225f926 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -495,8 +495,10 @@ _("corrected entry offsets in directory %" PRIu64 "\n"),
>  
>  	/*
>  	 * if parent entry is bogus, null it out.  we'll fix it later .
> +	 * If the validation fails for the root inode we fix it in
> +	 * the next else case.
>  	 */
> -	if (verify_inum(mp, *parent))  {
> +	if (verify_inum(mp, *parent) && ino != mp->m_sb.sb_rootino)  {
>  
>  		do_warn(
>  _("bogus .. inode number (%" PRIu64 ") in directory inode %" PRIu64 ", "),
> 

Re: [PATCH] xfs_repair: Fix root inode's parent when it's bogus for sf directory

[Dave Chinner]

On Thu, Jun 14, 2018 at 05:20:56PM -0500, Eric Sandeen wrote:
> On 6/14/18 5:16 PM, Marco Benatto wrote:
> > Currently when root inode is in short-form and its parent ino
> > has an invalid value, process_sf_dir2() ends up not fixing it,
> > because if verify_inum() fails we never get to the next case which
> > would fix the root inode's parent pointer.
> > 
> > This behavior triggers the following assert on process_dir2():
> > 
> >    ASSERT((ino != mp->m_sb.sb_rootino && ino != *parent) ||
> >         (ino == mp->m_sb.sb_rootino &&
> >         (ino == *parent || need_root_dotdot == 1)));
> > 
> > This patch fixes this behavior by making sure we always properly
> > handle rootino parent pointer in process_sf_dir2()
> > 
> > Signed-off-by: Marco Benatto <mbenatto@redhat.com>
> 
> This looks correct to me, thanks.
> 
> FWIW the problem can be demonstrated by setting the root inode's
> parent to 0 on a freshly made fs:
> 
> # mkfs.xfs -f -dfile,name=fsfile,size=1g
> # xfs_db -x -c "sb 0" -c "addr rootino" -c "write u3.sfdir3.hdr.parent.i4 0" fsfile
> # xfs_repair fsfile

xfstest please (also to check that xfs_scrub detects it)?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs_repair: Fix root inode's parent when it's bogus for sf directory

[Marco Benatto]

On Thu, Jun 14, 2018 at 8:35 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Thu, Jun 14, 2018 at 05:20:56PM -0500, Eric Sandeen wrote:
>> On 6/14/18 5:16 PM, Marco Benatto wrote:
>> > Currently when root inode is in short-form and its parent ino
>> > has an invalid value, process_sf_dir2() ends up not fixing it,
>> > because if verify_inum() fails we never get to the next case which
>> > would fix the root inode's parent pointer.
>> >
>> > This behavior triggers the following assert on process_dir2():
>> >
>> >    ASSERT((ino != mp->m_sb.sb_rootino && ino != *parent) ||
>> >         (ino == mp->m_sb.sb_rootino &&
>> >         (ino == *parent || need_root_dotdot == 1)));
>> >
>> > This patch fixes this behavior by making sure we always properly
>> > handle rootino parent pointer in process_sf_dir2()
>> >
>> > Signed-off-by: Marco Benatto <mbenatto@redhat.com>
>>
>> This looks correct to me, thanks.
>>
>> FWIW the problem can be demonstrated by setting the root inode's
>> parent to 0 on a freshly made fs:
>>
>> # mkfs.xfs -f -dfile,name=fsfile,size=1g
>> # xfs_db -x -c "sb 0" -c "addr rootino" -c "write u3.sfdir3.hdr.parent.i4 0" fsfile
>> # xfs_repair fsfile
>
> xfstest please (also to check that xfs_scrub detects it)?

Sure, working on this. I'll send you once it's done.

>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com



Cheers,

-- 
Marco Benatto

Re: [PATCH] xfs_repair: Fix root inode's parent when it's bogus for sf directory

[Marco Benatto]

FYI,

just sent out "[PATCH] xfstests: Teste root inode parent pointer repairing"

On Fri, Jun 15, 2018 at 2:30 PM, Marco Benatto <mbenatto@redhat.com> wrote:
> On Thu, Jun 14, 2018 at 8:35 PM, Dave Chinner <david@fromorbit.com> wrote:
>> On Thu, Jun 14, 2018 at 05:20:56PM -0500, Eric Sandeen wrote:
>>> On 6/14/18 5:16 PM, Marco Benatto wrote:
>>> > Currently when root inode is in short-form and its parent ino
>>> > has an invalid value, process_sf_dir2() ends up not fixing it,
>>> > because if verify_inum() fails we never get to the next case which
>>> > would fix the root inode's parent pointer.
>>> >
>>> > This behavior triggers the following assert on process_dir2():
>>> >
>>> >    ASSERT((ino != mp->m_sb.sb_rootino && ino != *parent) ||
>>> >         (ino == mp->m_sb.sb_rootino &&
>>> >         (ino == *parent || need_root_dotdot == 1)));
>>> >
>>> > This patch fixes this behavior by making sure we always properly
>>> > handle rootino parent pointer in process_sf_dir2()
>>> >
>>> > Signed-off-by: Marco Benatto <mbenatto@redhat.com>
>>>
>>> This looks correct to me, thanks.
>>>
>>> FWIW the problem can be demonstrated by setting the root inode's
>>> parent to 0 on a freshly made fs:
>>>
>>> # mkfs.xfs -f -dfile,name=fsfile,size=1g
>>> # xfs_db -x -c "sb 0" -c "addr rootino" -c "write u3.sfdir3.hdr.parent.i4 0" fsfile
>>> # xfs_repair fsfile
>>
>> xfstest please (also to check that xfs_scrub detects it)?
>
> Sure, working on this. I'll send you once it's done.
>
>>
>> Cheers,
>>
>> Dave.
>> --
>> Dave Chinner
>> david@fromorbit.com
>
>
>
> Cheers,
>
> --
> Marco Benatto



-- 
Marco Benatto
Senior Software Maintenance Engineer | Red Hat Brasil
T: +55 11 35246161 | M: +55 41 9 88504051
Av. Brigadeiro Faria Lima 3900, 8° Andar. São Paulo, Brasil. RED HAT |
TRIED. TESTED. TRUSTED. Saiba porque em redhat.com