xfs
[Top] [All Lists]

Re: [PATCH] XFS: Check for valid transaction headers in recovery

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] XFS: Check for valid transaction headers in recovery
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Tue, 23 Sep 2008 22:07:56 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1222218974-5161-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1222218974-5161-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.16 (Macintosh/20080707)
Dave Chinner wrote:
> When we are about to add a new item to a transaction in recovery,
> we need to check that it is valid first. Current we just assert
> that header magic number matches, but in production systems
> that is not done add a corrupted transaction to the list to be
> processed. This results in a kernel oops later when processing the
> corrupted transaction.
> 
> Instead, if we detect a corrupted transaction, abort recovery and
> leave the user to clean up the mess that has occurred.
> 
> Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>

Seems fine to me (I guess you tried the provided corrupt image?) but the
commit message could be made a bit more ... English ;)

-Eric

> ---
>  fs/xfs/xfs_log_recover.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 37c2bf9..1ccc80d 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1420,7 +1420,13 @@ xlog_recover_add_to_trans(
>               return 0;
>       item = trans->r_itemq;
>       if (item == NULL) {
> -             ASSERT(*(uint *)dp == XFS_TRANS_HEADER_MAGIC);
> +             /* we need to catch log corruptions here */
> +             if (*(uint *)dp != XFS_TRANS_HEADER_MAGIC) {
> +                     xlog_warn("XFS: xlog_recover_add_to_trans: "
> +                               "bad header magic number");
> +                     ASSERT(0);
> +                     return XFS_ERROR(EIO);
> +             }
>               if (len == sizeof(xfs_trans_header_t))
>                       xlog_recover_add_item(&trans->r_itemq);
>               memcpy(&trans->r_theader, dp, len); /* d, s, l */

<Prev in Thread] Current Thread [Next in Thread>