On Mon, Jun 03, 2024 at 05:46:08PM +0800, lei lu wrote:
> There is a lack of verification of the space occupied by fixed members
> of xlog_op_header in the xlog_recover_process_data.
>
> We can create a crafted image to trigger an out of bounds read by
> following these steps:
> 1) Mount an image of xfs, and do some file operations to leave records
> 2) Before umounting, copy the image for subsequent steps to simulate
> abnormal exit. Because umount will ensure that tail_blk and
> head_blk are the same, which will result in the inability to enter
> xlog_recover_process_data
> 3) Write a tool to parse and modify the copied image in step 2
> 4) Make the end of the xlog_op_header entries only 1 byte away from
> xlog_rec_header->h_size
> 5) xlog_rec_header->h_num_logops++
> 6) Modify xlog_rec_header->h_crc
>
> Fix:
> Add a check to make sure there is sufficient space to access fixed members
> of xlog_op_header.
>
> Signed-off-by: lei lu <llfamsec@xxxxxxxxx>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_log_recover.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 1251c81e55f9..14609ce212db 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2456,7 +2456,10 @@ xlog_recover_process_data(
>
> ohead = (struct xlog_op_header *)dp;
> dp += sizeof(*ohead);
> - ASSERT(dp <= end);
> + if (dp > end) {
> + xfs_warn(log->l_mp, "%s: op header overrun", __func__);
> + return -EFSCORRUPTED;
> + }
>
> /* errors will abort recovery */
> error = xlog_recover_process_ophdr(log, rhash, rhead, ohead,
> --
> 2.34.1
>
>
