Skip to content

Commit

Permalink
zfs: add bounds checking to zil_parse
Browse files Browse the repository at this point in the history
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 <click1799@163.com>
  • Loading branch information
XDTG committed Jun 28, 2024
1 parent c98295e commit 7e7c5ab
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions module/zfs/zil.c
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,23 @@ 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) {
error = SET_ERROR(EUCLEAN);
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) {

Check failure on line 526 in module/zfs/zil.c

View workflow job for this annotation

GitHub Actions / checkstyle

missing space between keyword and paren
error = SET_ERROR(EUCLEAN);
arc_buf_destroy(abuf, &abuf);
goto done;
}

if (lr->lrc_seq > claim_lr_seq) {
arc_buf_destroy(abuf, &abuf);
goto done;
Expand Down

0 comments on commit 7e7c5ab

Please sign in to comment.