All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_repair: don't error out on dirs with a single leafn block
@ 2018-10-30 18:04 Darrick J. Wong
  2018-10-30 18:25 ` Eric Sandeen
  0 siblings, 1 reply; 2+ messages in thread
From: Darrick J. Wong @ 2018-10-30 18:04 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Dave Chinner, xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

In process_node_dir2, we need to distinguish between a directory with a
single leafn block (yes, they exist) having no interior da nodes, and a
directory with a da tree that incorrectly points to dablk 0.  If we
happened to fill out any part of the da cursor then we have a da btree
with garbage in it; otherwise, we have a single leafn block.

This was found by repair repeatedly rebuilding a directory containing a
single leafn block.

Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
Reported-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/dir2.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/repair/dir2.c b/repair/dir2.c
index 3c010f50..8dd021a0 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -1247,11 +1247,22 @@ process_node_dir2(
 		return 1;
 
 	/*
-	 * Directories with a root marked XFS_DIR2_LEAFN_MAGIC are corrupt
+	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC.
+	 *
+	 * Be careful here: If any level of the da cursor was filled out then
+	 * the directory has a da btree containing an invalid before pointer to
+	 * dblock 0, and we should move on to rebuilding the directory.  If no
+	 * levels in the da cursor got filled out, then we just have a single
+	 * leafn block and we're done.
 	 */
 	if (bno == 0) {
-		err_release_da_cursor(mp, &da_cursor, 0);
-		return 1;
+		if (da_cursor.active > 0) {
+			err_release_da_cursor(mp, &da_cursor, 0);
+			return 1;
+		} else {
+			release_da_cursor(mp, &da_cursor, 0);
+			return 0;
+		}
 	} else {
 		/*
 		 * Now pass cursor and bno into leaf-block processing routine.

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] xfs_repair: don't error out on dirs with a single leafn block
  2018-10-30 18:04 [PATCH] xfs_repair: don't error out on dirs with a single leafn block Darrick J. Wong
@ 2018-10-30 18:25 ` Eric Sandeen
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Sandeen @ 2018-10-30 18:25 UTC (permalink / raw)
  To: Darrick J. Wong, Eric Sandeen; +Cc: Dave Chinner, xfs

On 10/30/18 1:04 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In process_node_dir2, we need to distinguish between a directory with a
> single leafn block (yes, they exist) having no interior da nodes, and a
> directory with a da tree that incorrectly points to dablk 0.  If we
> happened to fill out any part of the da cursor then we have a da btree
> with garbage in it; otherwise, we have a single leafn block.
> 
> This was found by repair repeatedly rebuilding a directory containing a
> single leafn block.
> 
> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
> Reported-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I'm inclined to apply Dave's revert for 4.19, and after review,
add this fix for 4.20 for more soak.  Thoughts?

> ---
>  repair/dir2.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 3c010f50..8dd021a0 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -1247,11 +1247,22 @@ process_node_dir2(
>  		return 1;
>  
>  	/*
> -	 * Directories with a root marked XFS_DIR2_LEAFN_MAGIC are corrupt
> +	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC.
> +	 *
> +	 * Be careful here: If any level of the da cursor was filled out then
> +	 * the directory has a da btree containing an invalid before pointer to
> +	 * dblock 0, and we should move on to rebuilding the directory.  If no
> +	 * levels in the da cursor got filled out, then we just have a single
> +	 * leafn block and we're done.
>  	 */
>  	if (bno == 0) {
> -		err_release_da_cursor(mp, &da_cursor, 0);
> -		return 1;
> +		if (da_cursor.active > 0) {
> +			err_release_da_cursor(mp, &da_cursor, 0);
> +			return 1;
> +		} else {
> +			release_da_cursor(mp, &da_cursor, 0);
> +			return 0;
> +		}
>  	} else {
>  		/*
>  		 * Now pass cursor and bno into leaf-block processing routine.
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-10-31  3:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 18:04 [PATCH] xfs_repair: don't error out on dirs with a single leafn block Darrick J. Wong
2018-10-30 18:25 ` Eric Sandeen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.