Skip to content

Commit d791896

Browse files
Gao Xianggregkh
authored andcommitted
xfs: avoid LR buffer overrun due to crafted h_len
[ Upstream commit f692d09 ] Currently, crafted h_len has been blocked for the log header of the tail block in commit a70f9fe ("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> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 7bd6f89 commit d791896

1 file changed

Lines changed: 19 additions & 20 deletions

File tree

fs/xfs/xfs_log_recover.c

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2904,7 +2904,8 @@ STATIC int
29042904
xlog_valid_rec_header(
29052905
struct xlog *log,
29062906
struct xlog_rec_header *rhead,
2907-
xfs_daddr_t blkno)
2907+
xfs_daddr_t blkno,
2908+
int bufsize)
29082909
{
29092910
int hlen;
29102911

@@ -2920,10 +2921,14 @@ xlog_valid_rec_header(
29202921
return -EFSCORRUPTED;
29212922
}
29222923

2923-
/* LR body must have data or it wouldn't have been written */
2924+
/*
2925+
* LR body must have data (or it wouldn't have been written)
2926+
* and h_len must not be greater than LR buffer size.
2927+
*/
29242928
hlen = be32_to_cpu(rhead->h_len);
2925-
if (XFS_IS_CORRUPT(log->l_mp, hlen <= 0 || hlen > INT_MAX))
2929+
if (XFS_IS_CORRUPT(log->l_mp, hlen <= 0 || hlen > bufsize))
29262930
return -EFSCORRUPTED;
2931+
29272932
if (XFS_IS_CORRUPT(log->l_mp,
29282933
blkno > log->l_logBBsize || blkno > INT_MAX))
29292934
return -EFSCORRUPTED;
@@ -2984,9 +2989,6 @@ xlog_do_recovery_pass(
29842989
goto bread_err1;
29852990

29862991
rhead = (xlog_rec_header_t *)offset;
2987-
error = xlog_valid_rec_header(log, rhead, tail_blk);
2988-
if (error)
2989-
goto bread_err1;
29902992

29912993
/*
29922994
* xfsprogs has a bug where record length is based on lsunit but
@@ -3001,21 +3003,18 @@ xlog_do_recovery_pass(
30013003
*/
30023004
h_size = be32_to_cpu(rhead->h_size);
30033005
h_len = be32_to_cpu(rhead->h_len);
3004-
if (h_len > h_size) {
3005-
if (h_len <= log->l_mp->m_logbsize &&
3006-
be32_to_cpu(rhead->h_num_logops) == 1) {
3007-
xfs_warn(log->l_mp,
3006+
if (h_len > h_size && h_len <= log->l_mp->m_logbsize &&
3007+
rhead->h_num_logops == cpu_to_be32(1)) {
3008+
xfs_warn(log->l_mp,
30083009
"invalid iclog size (%d bytes), using lsunit (%d bytes)",
3009-
h_size, log->l_mp->m_logbsize);
3010-
h_size = log->l_mp->m_logbsize;
3011-
} else {
3012-
XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
3013-
log->l_mp);
3014-
error = -EFSCORRUPTED;
3015-
goto bread_err1;
3016-
}
3010+
h_size, log->l_mp->m_logbsize);
3011+
h_size = log->l_mp->m_logbsize;
30173012
}
30183013

3014+
error = xlog_valid_rec_header(log, rhead, tail_blk, h_size);
3015+
if (error)
3016+
goto bread_err1;
3017+
30193018
if ((be32_to_cpu(rhead->h_version) & XLOG_VERSION_2) &&
30203019
(h_size > XLOG_HEADER_CYCLE_SIZE)) {
30213020
hblks = h_size / XLOG_HEADER_CYCLE_SIZE;
@@ -3096,7 +3095,7 @@ xlog_do_recovery_pass(
30963095
}
30973096
rhead = (xlog_rec_header_t *)offset;
30983097
error = xlog_valid_rec_header(log, rhead,
3099-
split_hblks ? blk_no : 0);
3098+
split_hblks ? blk_no : 0, h_size);
31003099
if (error)
31013100
goto bread_err2;
31023101

@@ -3177,7 +3176,7 @@ xlog_do_recovery_pass(
31773176
goto bread_err2;
31783177

31793178
rhead = (xlog_rec_header_t *)offset;
3180-
error = xlog_valid_rec_header(log, rhead, blk_no);
3179+
error = xlog_valid_rec_header(log, rhead, blk_no, h_size);
31813180
if (error)
31823181
goto bread_err2;
31833182

0 commit comments

Comments
 (0)