Re: xfs: XLOG_WAS_CONT_TRANS on a transaction with no buffered regions reads ri_buf[-1] - null deref Oops during mount log recovery

Re: xfs: XLOG_WAS_CONT_TRANS on a transaction with no buffered regions reads ri_buf[-1] - null deref Oops during mount log recovery

[Guikun Yang]

Hi,

While continuing on the same recovery path I found a second issue in
the log-replay handling of the primary superblock buffer - this one is
an unvalidated install of logged geometry, and it is a different
mechanism from the flag-sequence gap reported above.

## 5. Recovery installs the logged superblock without validating its
   geometry fields

xlog_recover_do_primary_sb_buffer() (fs/xfs/xfs_buf_item_recover.c)
recovers the buffer, then installs the on-disk superblock into the
in-core one with no validation:

  error = xlog_recover_do_reg_buffer(mp, item, bp, buf_f, current_lsn);
  ...
  xfs_sb_from_disk(&mp->m_sb, dsb);

The only checks afterwards are that the AG count and rtgroup count did
not shrink. Everything else in the freshly installed superblock is
used as-is. The immediately following consumers trust the geometry:

  error = xfs_update_last_ag_size(mp, orig_agcount);

which reaches __xfs_agino_range() (fs/xfs/libxfs/xfs_ag.c:172) and
shifts by the freshly installed sb_inopblog:

  UBSAN: shift-out-of-bounds in fs/xfs/libxfs/xfs_ag.c:172:11
  shift exponent 40 is too large for 32-bit type 'unsigned int'
   __xfs_agino_range.cold
   xfs_update_last_ag_size
   xlog_recover_buf_commit_pass2
  Kernel panic - not syncing: UBSAN: panic_on_warn set ...

The same mount-time read path validates these fields through
xfs_sb_read_verify()/xfs_validate_sb(); the recovery path bypasses
that entirely - the same on-disk structure gets two different levels
of trust depending on which path reads it.

## Reproducer

The base image carries a real dirty log from a normal crash. The
logged primary-superblock buffer item (XFS_LI_BUF, blf_blkno == 0,
XFS_BLFT_SB_BUF) is located, one byte in its data region is changed -
sb_inopblog, data offset 123, from 3 to 40 - and the v2 log record CRC
is recomputed. The unmodified image recovers and mounts cleanly.

Reproduced 3/3 with panic_on_warn=1. The code in mainline as of
2026-11-07 is identical.

Other geometry fields are installed through the same unvalidated
xfs_sb_from_disk() call and feed the same consumers: sb_agblocks,
sb_dblocks and sb_agcount go into __xfs_ag_block_count(),
sb_inopblock into the inode geometry, sb_blocklog into every shift and
mask derived from it. I did not chase each variant; the mechanism is
the single unvalidated install.

## Suggested fix

Validate the recovered superblock before installing it - the existing
xfs_validate_sb() (or at least its geometry subset) covers exactly the
fields these consumers use:

  xfs_sb_from_disk(&mp->m_sb, dsb);
+ error = xfs_validate_sb(mp, &mp->m_sb, true);
+ if (error) { ... return -EFSCORRUPTED; }

The reproducer (log scanner that finds the SB buffer item, flips
sb_inopblog and recomputes the CRC, ready-made image, serial log) is
available on request.

Thanks,
Guikun Yang

Re: xfs: XLOG_WAS_CONT_TRANS on a transaction with no buffered regions reads ri_buf[-1] - null deref Oops during mount log recovery

[Guikun Yang]

Hi,

One more from the same recovery path - this time in the inode item
handling, a different asymmetry from the two reported above.

## 6. ilf_len is used as the buffer read length without requiring
   whole filesystem blocks

xlog_recover_inode_commit_pass2() passes the logged ilf_len (a basic
block count) straight to xfs_buf_read()
(fs/xfs/xfs_inode_item_recover.c:351):

  error = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len,
                 0, &bp, &xfs_inode_buf_ops);

Nothing requires ilf_len to cover whole filesystem blocks. The
verifier, however, computes the number of inodes from the buffer
length rounded up (fs/xfs/libxfs/xfs_inode_buf.c):

  ni = XFS_BB_TO_FSB(mp, bp->b_length) * mp->m_sb.sb_inopblock;
  for (i = 0; i < ni; i++) {
          dip = xfs_buf_offset(bp, (i << mp->m_sb.sb_inodelog));
          unlinked_ino = be32_to_cpu(dip->di_next_unlinked);
          ...

With a 512-byte block size on the log and a 4k filesystem block,
ilf_len = 1 reads a single 512-byte buffer (kmalloc-512), the rounding
produces 1 FSB * 8 inodes, and the loop walks 4096 bytes - reading
past the slab from the second inode on:

  BUG: KASAN: slab-out-of-bounds in xfs_inode_buf_verify+...
  Read of size 2 ...
   xfs_inode_buf_verify -> xfs_buf_read -> xlog_recover_inode_commit_pass2
  Allocated by: kmalloc-512 (the buffer from xfs_buf_read)

The readahead path passes ilf_len through unbounded as well
(fs/xfs/xfs_inode_item_recover.c:36,41).

9232301894 ("xfs: bound inode fork length against the fork size during
log recovery") added bounds for the logged fork regions in this same
function, but the buffer read length itself is still unchecked - the
same log record carries both fields, one is now bounded and the other
is not.

## Reproducer

The dirty-log image from a normal crash is modified in five bytes:
ilf_len in the inode log item from 32 to 1, plus the four CRC bytes of
the log record. ilf_blkno and ilf_boffset keep their original values.
The unmodified image recovers and mounts cleanly; the modified one
hits the KASAN report above at mount(2), 3/3.

## Suggested fix

The verifier is the defence layer for every caller, so bounding the
walk by what the buffer actually holds seems most robust:

  ni = XFS_BB_TO_FSB(mp, bp->b_length) * mp->m_sb.sb_inopblock;
  becomes something like
  ni = XFS_BB_TO_FSB(mp, min(bp->b_length, XFS_FSB_TO_BB(mp, 1))) * ...

or equivalently computing ni from the buffer length divided by the
inode size instead of rounding up to a filesystem block. Rejecting
non-block-aligned ilf_len before xfs_buf_read() would also work but
leaves the verifier as-is for other callers.

The reproducer (log scanner, image builder, ready-made image, serial
log) is available on request.

Thanks,
Guikun Yang

Re: [Bug report] xfs: unvalidated ilf_boffset in an inode log item reaches an out-of-bounds read during log recovery

[Guikun Yang]

Hi,

Following up on this report from October 31 with a re-check against
current mainline (2026-11-08) and a smaller reproducer.

## Current state

xfs_buf_offset() is still a raw pointer add:

  return bp->b_addr + offset;

and ilf_boffset is still used through it exactly once in the tree -
the bare call in xlog_recover_inode_commit_pass2()
(fs/xfs/xfs_inode_item_recover.c):

  dip = xfs_buf_offset(bp, in_f->ilf_boffset);
  if (XFS_IS_CORRUPT(mp, !xfs_verify_magic16(bp, dip->di_magic)))

Nothing bounds ilf_boffset against the buffer that ilf_len bought.
The xfs_verify_magic16() comparison cannot help - it inspects a value
that has already been read out of bounds.

For what it is worth, the inode-item fork regions in this same
function gained bounds in 9232301894 ("xfs: bound inode fork length
against the fork size during log recovery", 2026-09-23), but the
record carries more fields than that fix covered: the buffer length
(ilf_len) and the buffer offset (ilf_boffset) are both still trusted
as-is.

## Smaller reproducer

The original report used a three-step image construction. The same
crash now reproduces with five bytes changed on a real dirty-log
image: ilf_boffset from 0 to 16384 (= BBTOB(ilf_len)) plus the four
CRC bytes of the log record. The unmodified image recovers and mounts
cleanly; the modified one hits

  BUG: KASAN: use-after-free in xlog_recover_inode_commit_pass2+...
  ...
   xlog_recover_inode_commit_pass2 -> ... -> xfs_log_recover -> mount

3/3 at mount(2), with panic_on_warn=1.

The reproducer (log scanner, image builder, ready-made image, serial
log) is available on request. Happy to test a patch - including one
revived from the October RFC series if that is the preferred route.

Thanks,
Guikun Yang

Re: xfs: XLOG_WAS_CONT_TRANS on a transaction with no buffered regions reads ri_buf[-1] - null deref Oops during mount log recovery

[Guikun Yang]

Hi,

Another one from the region-collection stage that runs before any of
the item handlers - this one does not involve any item type at all.

## 7. ilf_size is read from a region smaller than the format header

xlog_recover_add_to_trans() allocates the region buffer with the
attacker-controlled oh_len and then reads the inode log format from
it (fs/xfs/xfs_log_recover.c):

  ptr = xlog_kvmalloc(len);
  memcpy(ptr, dp, len);
  in_f = (struct xfs_inode_log_format *)ptr;
  ...
  if (item->ri_total == 0) {
          if (in_f->ilf_size == 0 ||
              in_f->ilf_size > XLOG_MAX_REGIONS_IN_ITEM) {

in_f->ilf_size is at offset 2 of struct xfs_inode_log_format, so the
read needs len >= 4 - nothing checks that. len is the raw oh_len from
the operation header.

Reachability is clean: this branch only requires the item list to be
non-empty with no regions collected yet, which is exactly the state
left by a complete 16-byte transaction header (len ==
sizeof(struct xfs_trans_header) makes xlog_recover_add_item() hang an
empty item). A normal transaction header followed by a region
operation with oh_len = 1 reaches the read with a 1-byte allocation:

  BUG: KASAN: slab-out-of-bounds in xlog_recover_add_to_trans+0x799/0x830
  Read of size 2 at addr ...
   xlog_recover_add_to_trans
   xlog_recovery_process_trans
   xlog_recover_process_ophdr
   xlog_recover_process_data
   ...
   xfs_fs_fill_super

The read is of size 2 - in_f->ilf_size itself.

## Reproducer

Same shape as the previous reports: a real dirty-log image, the region
operation header's oh_len changed from 0x14 to 0x01 plus the four CRC
bytes of the log record. The unmodified image recovers and mounts
cleanly; the modified one hits the KASAN report at mount(2), 3/3.
Unchanged in mainline as of 2026-11-08.

## Suggested fix

Check the region length before interpreting it as a log format - the
header magic is the only field this stage may assume:

  if (len < sizeof(struct xfs_inode_log_format_32) /* or 4 */) {
          xfs_warn(...);
          return -EFSCORRUPTED;
  }

before the ilf_size read; the later xfs_inode_item_format_convert()
already handles short legacy formats properly, so the check belongs at
this stage.

The reproducer (image builder, ready-made image, serial log) is
available on request.

Thanks,
Guikun Yang

Re: xfs: XLOG_WAS_CONT_TRANS on a transaction with no buffered regions reads ri_buf[-1] - null deref Oops during mount log recovery

[Guikun Yang]

Hi,

One more from the buffer item handling, again a different mechanism:
the number of set-bit runs in the buffer data map drives an index into
ri_buf[] that is never checked against the number of regions the log
item actually carries.

## 8. xlog_recover_do_inode_buffer() indexes ri_buf[] by bitmap runs,
   unchecked against ri_total

The inode-buffer recovery walks di_next_unlinked fields and skips
forward through the buffer data map run by run
(fs/xfs/xfs_buf_item_recover.c:625):

  while (next_unlinked_offset >= (reg_buf_offset + reg_buf_bytes)) {
          ...
          bit = xfs_next_bit(buf_f->blf_data_map, buf_f->blf_map_size, bit);
          if (bit == -1)
                  return 0;
          nbits = xfs_contig_bits(buf_f->blf_data_map, buf_f->blf_map_size, bit);
          ...
          item_index++;
  }
  ...
  ASSERT(item->ri_buf[item_index].iov_base != NULL);
  ...
  logged_nextp = item->ri_buf[item_index].iov_base + ...;

item_index is advanced once per set-bit run in blf_data_map, but
ri_buf[] has ri_total entries, where ri_total comes from blf_size -
the number of regions actually present in the log item. Nothing ties
the bitmap to blf_size: a data map with more runs than blf_size claims
regions walks item_index past the end of ri_buf[] and the ASSERTs are
the only guards, which compile away on non-DEBUG kernels.

The sibling path xlog_recover_do_reg_buffer() validates the same
bitmap against the same ri_buf[] array for the non-inode case and
clamps nbits to the region length - the asymmetry between the two
sisters in the same file is the whole bug.

## Reproducer

All field values are legal; nothing is forged out of range. A buffer
log item is turned into an inode-buffer type with blf_flags = 1,
blf_len = 8, blf_size = 2 (two regions), and a data map of 0x55555555
(alternating bits). The alternating map makes every inode skip a run:
item_index reaches 1 after i=0 and 3 after i=1, past ri_total = 2.
The blf_blkno is aimed at the zeroed area of the device so that
xlog_recover_get_buf_lsn() returns -1 and the item is replayed
immediately.

  BUG: KASAN: slab-out-of-bounds in xlog_recover_do_inode_buffer+0x5a4/0x620
  Read of size 8 at addr ...
   xlog_recover_do_inode_buffer -> xlog_recover_buf_commit_pass2
   -> ... -> xfs_log_mount -> xfs_mountfs
  Kernel panic - not syncing: KASAN: panic_on_warn set ...

The read of size 8 is ri_buf[item_index].iov_base itself. 3/3 at
mount(2); the unmodified image mounts cleanly.

## Suggested fix

Two guards, mirroring do_reg_buffer():

  item_index++;
+ if (XFS_IS_CORRUPT(mp, item_index >= item->ri_total))
+         return -EFSCORRUPTED;

plus clamping nbits against item->ri_buf[item_index].iov_len the way
do_reg_buffer() does, so the region copy cannot run past the region.

Unchanged in mainline as of 2026-11-09 (the function is byte-identical
to the tree this was tested on).

The reproducer (image builder, ready-made image, serial log) is
available on request.

Thanks,
Guikun Yang

xfs: XLOG_WAS_CONT_TRANS on a transaction with no buffered regions reads ri_buf[-1] - null deref Oops during mount log recovery

[Guikun Yang]

Hi,

Mounting an XFS volume whose log record after the transaction header
carries an operation header with the XLOG_WAS_CONT_TRANS flag panics
the kernel during log recovery - on a stock kernel, no sanitizer
configuration needed:

  BUG: unable to handle page fault for address: fffffffffffffff0
  #PF: supervisor read access in kernel mode
  Oops: Oops: 0000 [#1] SMP KASAN PTI
  RIP: xlog_recover_add_to_cont_trans+0xf9/0x460
   xlog_recovery_process_trans
   xlog_recover_process_ophdr
   xlog_recover_process_data
   xlog_recover_process
   xlog_do_recovery_pass
  Kernel panic - not syncing: Fatal exception

## The gap

xlog_recover_add_to_trans() creates a recover item for the transaction
header via xlog_recover_add_item():

  item = kzalloc_obj(struct xlog_recover_item, ...);
  INIT_LIST_HEAD(&item->ri_list);
  list_add_tail(&item->ri_list, head);

The item has ri_cnt == 0 and ri_buf == NULL - the transaction header
itself is copied into trans->r_theader, not into the item.

If the very next operation header carries XLOG_WAS_CONT_TRANS,
xlog_recovery_process_trans() dispatches to
xlog_recover_add_to_cont_trans(), which takes the tail item and indexes
it without checking that it has any regions buffered
(fs/xfs/xfs_log_recover.c:2144):

  item = list_entry(trans->r_itemq.prev, struct xlog_recover_item, ri_list);
  old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;

ri_buf is NULL and ri_cnt is 0, so the expression reads
NULL[-1] - the faulting address fffffffffffffff0.

Nothing on the path validates the operation-flag sequence against the
item state. The only ri_cnt-related check in the recovery code is an
ASSERT() in xlog_recover_reorder_trans(), which is a no-op on a kernel
without XFS debugging enabled.

Note that the flag's name suggests it belongs to a continuation, but
reaching the crash does not require constructing one: a single
XLOG_WAS_CONT_TRANS operation right after the transaction header is
enough, because the dispatcher never checks that the previous region
had a buffer attached.

## Reproducer shape

The volume is made with a real dirty log (mkfs.xfs, files, sync, crash),
then the first region operation header after the transaction header has
its oh_flags changed from 0x00 to 0x08 (XLOG_WAS_CONT_TRANS), and the
v2 log record CRC is recomputed (328-byte header + h_len payload). The
unmodified dirty-log image recovers and mounts cleanly as a control.

Trigger is mount(2) alone. Reproduced 3/3 with panic_on_oops; the code
in mainline as of 2026-11-06 is identical here.

## Suggested fix

Either bound the item before use in add_to_cont_trans(), or validate
the flag sequence at dispatch time - XLOG_WAS_CONT_TRANS on a
transaction whose tail item has no regions is corruption and should
fail recovery the same way the other -EFSCORRUPTED paths do:

  item = list_entry(trans->r_itemq.prev, struct xlog_recover_item, ri_list);
+ if (!item->ri_cnt) {
+         xfs_warn(log->l_mp, "%s: continuation with no previous region", __func__);
+         return -EFSCORRUPTED;
+ }

The reproducer (image builder that locates the record/op, flips the
flag and recomputes the CRC, plus a ready-made image and serial log) is
available on request.

Thanks,
Guikun Yang