[PATCH] xfs: validate iclog size and log record length in log recovery

[Brian Foster <bfoster@xxxxxxxxxx>]

Each log record header contains an h_size value which represents the
size of the iclog buffers when the record was logged and an h_len value
which identifies the length of the particular log record. Log recovery
uses both fields to determine the size of the log buffers to use for
recovery and to correctly process each log record.

Neither field is completely validated during recovery, however. While
on-disk corruptions might be detected by CRC verification, we are still
susceptible to errors such as excessively sized buffer allocations and
overruns of the log record header and data buffers.

Update the xlog_valid_rec_header() function to validate the record
h_size and h_len fields against a new max_size parameter. The maximum
size value is passed as a parameter because the value differs depending
on whether we are trying to identify the iclog size or actually
processing a log record. In the former case, validate that neither field
exceeds the maximum supported iclog size of XFS. Once the iclog size is
identified and log record buffers allocated, validate all records to be
processed against the iclog size to ensure that buffer overrun cannot
occur.

Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
---

Here's a stab at addressing the log record field validation issues Dave
calls out here:

        http://oss.sgi.com/pipermail/xfs/2015-July/042557.html

This is still undergoing some testing, but I've not hit any problems so
far...

Brian

 fs/xfs/xfs_log_recover.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 01dd228..98c420a 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4078,13 +4078,22 @@ xlog_unpack_data(
        return 0;
 }
 
+/*
+ * Sanity check a log record header. The caller provides the maximum iclog size
+ * and record length since validity depends on the context. For example, the
+ * first record is used to allocate buffers and thus is validated against the
+ * maximum supported iclog size. Subsequent records must be validated against
+ * the identified iclog size to avoid overflow of the record buffers.
+ */
 STATIC int
 xlog_valid_rec_header(
        struct xlog             *log,
        struct xlog_rec_header  *rhead,
-       xfs_daddr_t             blkno)
+       xfs_daddr_t             blkno,
+       int                     max_size)
 {
        int                     hlen;
+       int                     hsize;
 
        if (unlikely(rhead->h_magicno != cpu_to_be32(XLOG_HEADER_MAGIC_NUM))) {
                XFS_ERROR_REPORT("xlog_valid_rec_header(1)",
@@ -4099,14 +4108,21 @@ xlog_valid_rec_header(
                return -EIO;
        }
 
+       hsize = be32_to_cpu(rhead->h_size);
+       if (unlikely(hsize <= 0 || hsize > max_size)) {
+               xfs_warn(log->l_mp, "%s: invalid iclog size (%d).", __func__,
+                        hsize);
+               return -EFSCORRUPTED;
+       }
+
        /* LR body must have data or it wouldn't have been written */
        hlen = be32_to_cpu(rhead->h_len);
-       if (unlikely( hlen <= 0 || hlen > INT_MAX )) {
+       if (unlikely(hlen <= 0 || hlen > max_size)) {
                XFS_ERROR_REPORT("xlog_valid_rec_header(2)",
                                XFS_ERRLEVEL_LOW, log->l_mp);
                return -EFSCORRUPTED;
        }
-       if (unlikely( blkno > log->l_logBBsize || blkno > INT_MAX )) {
+       if (unlikely(blkno > log->l_logBBsize || blkno > INT_MAX)) {
                XFS_ERROR_REPORT("xlog_valid_rec_header(3)",
                                XFS_ERRLEVEL_LOW, log->l_mp);
                return -EFSCORRUPTED;
@@ -4159,7 +4175,8 @@ xlog_do_recovery_pass(
                        goto bread_err1;
 
                rhead = (xlog_rec_header_t *)offset;
-               error = xlog_valid_rec_header(log, rhead, tail_blk);
+               error = xlog_valid_rec_header(log, rhead, tail_blk,
+                                             XLOG_MAX_RECORD_BSIZE);
                if (error)
                        goto bread_err1;
                h_size = be32_to_cpu(rhead->h_size);
@@ -4244,7 +4261,8 @@ 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;
 
@@ -4318,7 +4336,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.1.0