On Fri, May 04, 2018 at 05:33:35PM -0500, Eric Sandeen wrote:
>
>
> On 4/17/18 9:47 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > If we encounter a directory with an entry that points to inode zero,
> > we'll crash due to an ASSERT during process_inode_chunk. Instead, just
> > set the in-core parent to NULLFSINO so that phase 6 will reset it for
> > us.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> So .... this is .... probably ok? but the ASSERT makes me think that
> process_dinode is never supposed to return a 0 parent if isa_dir is set,
> and so I'm a little worried about breaking that assumption up and just
> catching it later, here. Usually the asserts mean "the code should never
> let this happen and if it does, some prior code is wrong"
>
> Looking at the calls below it, it seems like we generally call verify_inum
> which would set NULLFSINO if the inum is bad.
>
> But in process_dir2_data
>
> [17:25] <sandeen> } else if (verify_inum(mp, ent_ino)) {
> [17:25] <sandeen> clearreason = _("invalid");
> [17:26] <sandeen> so we probably saw that it's bad, but:
> [17:26] <sandeen> * We have a special dot & dotdot fixer-upper below which can
> [17:26] <sandeen> * sort out the proper inode number, so don't clear it.
> [17:26] <sandeen> clearreason = NULL;
> [17:26] <sandeen> and then:
> [17:26] <sandeen> * Special .. entry processing.
> [17:26] <sandeen> *parent = ent_ino;
> [17:26] <sandeen> \o/
>
> so ... I wonder if it wouldn't be more consistent to find the place under
> process_inode() where we willingly/wrongly set *parent to an invalid value,
> and handle the problem there? Do you know what path you were on to hit this?
/me doesn't remember, it was either the sf dir scrub or the dir block
offline repair fuzztest setting the parent pointer to zero.
--D
> -Eric
> > ---
> > repair/dino_chunks.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
> > index 17de95f..2d34079 100644
> > --- a/repair/dino_chunks.c
> > +++ b/repair/dino_chunks.c
> > @@ -874,7 +874,8 @@ process_inode_chunk(
> > * be solid then.
> > */
> > if (!ino_discovery) {
> > - ASSERT(parent != 0);
> > + if (parent == 0)
> > + parent = NULLFSINO;
> > set_inode_parent(ino_rec, irec_offset, parent);
> > ASSERT(parent ==
> > get_inode_parent(ino_rec, irec_offset));
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
