[PATCH] prevent panic during log recovery due to bogus operation header length

[PATCH] prevent panic during log recovery due to bogus operation header length

[Lachlan McIlroy]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

A problem was reported where a system panicked in log recovery due
to a corrupt log record.  The cause of the corruption is not known
but this change will at least prevent a crash for this specific
scenario.  Log recovery definitely needs some more work in this area.

Lachlan

[-- Attachment #2: xfs_log_recover.diff --]
[-- Type: text/x-patch, Size: 615 bytes --]

--- fs/xfs/xfs_log_recover.c_1.332	2007-12-12 17:14:57.000000000 +1100
+++ fs/xfs/xfs_log_recover.c	2007-12-12 17:15:42.000000000 +1100
@@ -2912,7 +2912,12 @@ xlog_recover_process_data(
 				xlog_recover_new_tid(&rhash[hash], tid,
 					be64_to_cpu(rhead->h_lsn));
 		} else {
-			ASSERT(dp + be32_to_cpu(ohead->oh_len) <= lp);
+			if (dp + be32_to_cpu(ohead->oh_len) > lp) {
+				xlog_warn(
+			"XFS: xlog_recover_process_data: bad length");
+				ASSERT(0);
+				return (XFS_ERROR(EIO));
+			}
 			flags = ohead->oh_flags & ~XLOG_END_TRANS;
 			if (flags & XLOG_WAS_CONT_TRANS)
 				flags &= ~XLOG_CONTINUE_TRANS;

Re: [PATCH] prevent panic during log recovery due to bogus operation header length

[Christoph Hellwig]

On Wed, Dec 12, 2007 at 06:07:47PM +1100, Lachlan McIlroy wrote:
> A problem was reported where a system panicked in log recovery due
> to a corrupt log record.  The cause of the corruption is not known
> but this change will at least prevent a crash for this specific
> scenario.  Log recovery definitely needs some more work in this area.
>
> Lachlan

> --- fs/xfs/xfs_log_recover.c_1.332	2007-12-12 17:14:57.000000000 +1100
> +++ fs/xfs/xfs_log_recover.c	2007-12-12 17:15:42.000000000 +1100
> @@ -2912,7 +2912,12 @@ xlog_recover_process_data(
>  				xlog_recover_new_tid(&rhash[hash], tid,
>  					be64_to_cpu(rhead->h_lsn));
>  		} else {
> -			ASSERT(dp + be32_to_cpu(ohead->oh_len) <= lp);
> +			if (dp + be32_to_cpu(ohead->oh_len) > lp) {
> +				xlog_warn(
> +			"XFS: xlog_recover_process_data: bad length");
> +				ASSERT(0);
> +				return (XFS_ERROR(EIO));
> +			}

this still gives a panic for debug builds..  Maybe this should become
a WARN_ON(1) instead?