xfs: avoid LR buffer overrun due to crafted h_len

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 <hsiangkao@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index a17d788..782ec3e 100644 (file)
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2878,7 +2878,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;
 
@@ -2894,10 +2895,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;
@@ -2958,9 +2963,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
@@ -2975,21 +2977,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;
@@ -3070,7 +3069,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;
 
@@ -3151,7 +3150,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;