From ac6500389b12da35c954e81d6317944338cce9dc Mon Sep 17 00:00:00 2001 From: c1ick <35128600+XDTG@users.noreply.github.com> Date: Thu, 1 Aug 2024 08:17:04 +0800 Subject: [PATCH] zfs: add bounds checking to zil_parse (#16308) Make sure log record don't stray beyond valid memory region. There is a lack of verification of the space occupied by fixed members of lr_t in the zil_parse. We can create a crafted image to trigger an out of bounds read by following these steps: 1) Do some file operations and reboot to simulate abnormal exit without umount 2) zil_chain.zc_nused: 0x1000 3) First lr_t lr_t.lrc_txtype: 0x0 lr_t.lrc_reclen: 0x1000-0xb8-0x1 lr_t.lrc_txg: 0x0 lr_t.lrc_seq: 0x1 4) Update checksum in zil_chain.zc_eck Fix: Add some checks to make sure the remaining bytes are large enough to hold an log record. Signed-off-by: XDTG Reviewed-by: Alexander Motin Reviewed-by: Tony Hutter Reviewed-by: Brian Behlendorf --- module/zfs/zil.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 9b5d866a8c..9293429e43 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -513,9 +513,26 @@ zil_parse(zilog_t *zilog, zil_parse_blk_func_t *parse_blk_func, for (; lrp < end; lrp += reclen) { lr_t *lr = (lr_t *)lrp; + + /* + * Are the remaining bytes large enough to hold an + * log record? + */ + if ((char *)(lr + 1) > end) { + cmn_err(CE_WARN, "zil_parse: lr_t overrun"); + error = SET_ERROR(ECKSUM); + arc_buf_destroy(abuf, &abuf); + goto done; + } reclen = lr->lrc_reclen; - ASSERT3U(reclen, >=, sizeof (lr_t)); - ASSERT3U(reclen, <=, end - lrp); + if (reclen < sizeof (lr_t) || reclen > end - lrp) { + cmn_err(CE_WARN, + "zil_parse: lr_t has an invalid reclen"); + error = SET_ERROR(ECKSUM); + arc_buf_destroy(abuf, &abuf); + goto done; + } + if (lr->lrc_seq > claim_lr_seq) { arc_buf_destroy(abuf, &abuf); goto done;