From: Gao Xiang <hsiang...@redhat.com>

[ Upstream commit f692d09e9c8fd0f5557c2e87f796a16dd95222b8 ]

Currently, crafted h_len has been blocked for the log
header of the tail block in commit a70f9fe52daa ("xfs:
detect and handle invalid iclog size set by mkfs").

However, each log record could still have crafted h_len
and cause log record buffer overrun. So let's check
h_len vs buffer size for each log record as well.

Signed-off-by: Gao Xiang <hsiang...@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.w...@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.w...@oracle.com>
Reviewed-by: Brian Foster <bfos...@redhat.com>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
 fs/xfs/xfs_log_recover.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index e2ec91b2d0f46..9ceb67d0f2565 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2904,7 +2904,8 @@ STATIC int
 xlog_valid_rec_header(
        struct xlog             *log,
        struct xlog_rec_header  *rhead,
-       xfs_daddr_t             blkno)
+       xfs_daddr_t             blkno,
+       int                     bufsize)
 {
        int                     hlen;
 
@@ -2920,10 +2921,14 @@ xlog_valid_rec_header(
                return -EFSCORRUPTED;
        }
 
-       /* LR body must have data or it wouldn't have been written */
+       /*
+        * LR body must have data (or it wouldn't have been written)
+        * and h_len must not be greater than LR buffer size.
+        */
        hlen = be32_to_cpu(rhead->h_len);
-       if (XFS_IS_CORRUPT(log->l_mp, hlen <= 0 || hlen > INT_MAX))
+       if (XFS_IS_CORRUPT(log->l_mp, hlen <= 0 || hlen > bufsize))
                return -EFSCORRUPTED;
+
        if (XFS_IS_CORRUPT(log->l_mp,
                           blkno > log->l_logBBsize || blkno > INT_MAX))
                return -EFSCORRUPTED;
@@ -2984,9 +2989,6 @@ xlog_do_recovery_pass(
                        goto bread_err1;
 
                rhead = (xlog_rec_header_t *)offset;
-               error = xlog_valid_rec_header(log, rhead, tail_blk);
-               if (error)
-                       goto bread_err1;
 
                /*
                 * xfsprogs has a bug where record length is based on lsunit but
@@ -3001,21 +3003,18 @@ xlog_do_recovery_pass(
                 */
                h_size = be32_to_cpu(rhead->h_size);
                h_len = be32_to_cpu(rhead->h_len);
-               if (h_len > h_size) {
-                       if (h_len <= log->l_mp->m_logbsize &&
-                           be32_to_cpu(rhead->h_num_logops) == 1) {
-                               xfs_warn(log->l_mp,
+               if (h_len > h_size && h_len <= log->l_mp->m_logbsize &&
+                   rhead->h_num_logops == cpu_to_be32(1)) {
+                       xfs_warn(log->l_mp,
                "invalid iclog size (%d bytes), using lsunit (%d bytes)",
-                                        h_size, log->l_mp->m_logbsize);
-                               h_size = log->l_mp->m_logbsize;
-                       } else {
-                               XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
-                                               log->l_mp);
-                               error = -EFSCORRUPTED;
-                               goto bread_err1;
-                       }
+                                h_size, log->l_mp->m_logbsize);
+                       h_size = log->l_mp->m_logbsize;
                }
 
+               error = xlog_valid_rec_header(log, rhead, tail_blk, h_size);
+               if (error)
+                       goto bread_err1;
+
                if ((be32_to_cpu(rhead->h_version) & XLOG_VERSION_2) &&
                    (h_size > XLOG_HEADER_CYCLE_SIZE)) {
                        hblks = h_size / XLOG_HEADER_CYCLE_SIZE;
@@ -3096,7 +3095,7 @@ xlog_do_recovery_pass(
                        }
                        rhead = (xlog_rec_header_t *)offset;
                        error = xlog_valid_rec_header(log, rhead,
-                                               split_hblks ? blk_no : 0);
+                                       split_hblks ? blk_no : 0, h_size);
                        if (error)
                                goto bread_err2;
 
@@ -3177,7 +3176,7 @@ xlog_do_recovery_pass(
                        goto bread_err2;
 
                rhead = (xlog_rec_header_t *)offset;
-               error = xlog_valid_rec_header(log, rhead, blk_no);
+               error = xlog_valid_rec_header(log, rhead, blk_no, h_size);
                if (error)
                        goto bread_err2;
 
-- 
2.27.0



Reply via email to