diff mbox

xfs_repair: detect invalid zero-sized symlink inodes

Message ID 1488380403-5826-1-git-send-email-bfoster@redhat.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Brian Foster March 1, 2017, 3 p.m. UTC
Mathias Troiden reproduced a filesystem corruption that resulted in
a zero-sized local format symlink inode. This is invalid state and
results in an inode that cannot be accessed or modified.

The kernel detects this problem on inode access, fails and warns the
user to umount and run xfs_repair. Unfortunately, xfs_repair doesn't
even detect the problem. Thus the user has no path to recovery.

Update xfs_repair to check for invalid zero-sized symlinks and flag
them as corrupted. This results in tossing the inode, but returns
the fs to a valid state.

Reported-by: Mathias Troiden <mathias.troiden@gmail.com>
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 repair/dinode.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Darrick J. Wong March 1, 2017, 4:32 p.m. UTC | #1
On Wed, Mar 01, 2017 at 10:00:03AM -0500, Brian Foster wrote:
> Mathias Troiden reproduced a filesystem corruption that resulted in
> a zero-sized local format symlink inode. This is invalid state and
> results in an inode that cannot be accessed or modified.
> 
> The kernel detects this problem on inode access, fails and warns the
> user to umount and run xfs_repair. Unfortunately, xfs_repair doesn't
> even detect the problem. Thus the user has no path to recovery.
> 
> Update xfs_repair to check for invalid zero-sized symlinks and flag
> them as corrupted. This results in tossing the inode, but returns
> the fs to a valid state.
> 
> Reported-by: Mathias Troiden <mathias.troiden@gmail.com>
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks good,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  repair/dinode.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/repair/dinode.c b/repair/dinode.c
> index 8d01409..d664f87 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -1385,6 +1385,11 @@ process_symlink(
>  		return(1);
>  	}
>  
> +	if (be64_to_cpu(dino->di_size) == 0) {
> +		do_warn(_("zero size symlink in inode %" PRIu64 "\n"), lino);
> +		return 1;
> +	}
> +
>  	/*
>  	 * have to check symlink component by component.
>  	 * get symlink contents into data area
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig March 2, 2017, 12:35 a.m. UTC | #2
> +	if (be64_to_cpu(dino->di_size) == 0) {

You can drop the be64_to_cpu if you're only comparing with 0.

Otherwise this looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/repair/dinode.c b/repair/dinode.c
index 8d01409..d664f87 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -1385,6 +1385,11 @@  process_symlink(
 		return(1);
 	}
 
+	if (be64_to_cpu(dino->di_size) == 0) {
+		do_warn(_("zero size symlink in inode %" PRIu64 "\n"), lino);
+		return 1;
+	}
+
 	/*
 	 * have to check symlink component by component.
 	 * get symlink contents into data area