Skip to contentSkip to conversation

prevent panic during log recovery due to bogus operation header length

PATCH In review Lachlan McIlroy 2 participants 2 messages Dec 12, 2007 – Dec 14, 2007

lore
Share & tools
lore

Lachlan McIlroy

author
#0 lore raw

PATCH 1/1 prevent panic during log recovery due to bogus operation header length

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

xfs/xfs_log_recover.c renamed from xfs/xfs_log_recover.c_1.332 +6 -1
@@ -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;
#1 lore raw
Quoted 10 lines · Lachlan McIlroy

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

xfs/xfs_log_recover.c xlog_recover_process_data( · line 2920

Quoted 10 lines
xfs/xfs_log_recover.c renamed from xfs/xfs_log_recover.c_1.332 +6 -1
@@ -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?