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 */
|