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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
