[Top] [All Lists]

[PATCH 2/2] xfs: validate transaction header length on log recovery

To: xfs@xxxxxxxxxxx
Subject: [PATCH 2/2] xfs: validate transaction header length on log recovery
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 18 Jun 2015 08:49:01 -0400
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1434631741-50856-1-git-send-email-bfoster@xxxxxxxxxx>
References: <1434631741-50856-1-git-send-email-bfoster@xxxxxxxxxx>
When log recovery hits a new transaction, it copies the transaction
header from the expected location in the log to the in-core structure
using the length from the op record header. This length is validated to
ensure it doesn't exceed the length of the record, but not against the
expected size of a transaction header (and thus the size of the in-core
structure). If the on-disk length is corrupted, the associated memcpy()
can overflow, write to unrelated memory and lead to crashes. This has
been reproduced via filesystem fuzzing.

The code already checks that the length matches the transaction header
in order to add a recovery item to the transaction. Convert this check
to an explicit validation of the length to prevent memcpy() overflow. In
the event of the latter, warn the user and fail the log recovery.

Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
 fs/xfs/xfs_log_recover.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 299fbaf..3c6ad4c 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3437,8 +3437,14 @@ xlog_recover_add_to_trans(
                        return -EIO;
-               if (len == sizeof(xfs_trans_header_t))
-                       xlog_recover_add_item(&trans->r_itemq);
+               if (len != sizeof(struct xfs_trans_header)) {
+                       xfs_warn(log->l_mp, "%s: bad header size (%d)",
+                                __func__, len);
+                       ASSERT(0);
+                       return -EIO;
+               }
+               xlog_recover_add_item(&trans->r_itemq);
                memcpy(&trans->r_theader, dp, len);
                return 0;

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