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 correction and a related finding in the same function the
previous mail discussed.

## Correction to item 8

Item 8 said that xlog_recover_do_reg_buffer() validates the same
bitmap against the same ri_buf[] array. That holds for the
destination side only. On the source side this function is just as
unchecked as do_inode_buffer() - which is what the finding below is
about. Both functions need the same two guards, not one.

## 9. xlog_recover_do_reg_buffer() indexes ri_buf[] by bitmap runs
   without checking against ri_total

The loop advances the source index once per set-bit run and only
asserts about it (fs/xfs/xfs_buf_item_recover.c:464):

  i = 1;  /* 0 is the buf format structure */
  while (1) {
          bit = xfs_next_bit(buf_f->blf_data_map, buf_f->blf_map_size, bit);
          if (bit == -1)
                  break;
          nbits = xfs_contig_bits(...);
          ASSERT(nbits > 0);
          ASSERT(item->ri_buf[i].iov_base != NULL);      /* no-op */
          ASSERT(item->ri_buf[i].iov_len % XFS_BLF_CHUNK == 0);

          /* destination side: validated for real since CVE-2026-80536 */
          if (XFS_IS_CORRUPT(mp, BBTOB(bp->b_length) <
                          ((uint)bit << XFS_BLF_SHIFT) + (nbits << XFS_BLF_SHIFT)))
                  return -EFSCORRUPTED;

          if (item->ri_buf[i].iov_len < (nbits << XFS_BLF_SHIFT))
                  nbits = item->ri_buf[i].iov_len >> XFS_BLF_SHIFT;
          memcpy(..., item->ri_buf[i].iov_base, nbits << XFS_BLF_SHIFT);
          i++;
  }
  ASSERT(i == item->ri_total);          /* still a no-op in HEAD */

The only bound on i is that trailing ASSERT. The number of
iterations is driven entirely by the dirty bitmap, which has no
relationship to ri_total (blf_size). The commit that added the
destination check says so in its own message:

  "The destination offset (bit/nbits) and the buffer size (from
   blf_len) are both attacker-controlled ... yet the only thing
   bounding the copy is an ASSERT(), which compiles away on
   production kernels."

The same argument applies to the source index that was left in
place.

## Reproducer

Five field changes on a real dirty-log image, all values legal except
the ones being forged: blf_flags 0x9000 -> 0 (regular buffer), blf_len
1 -> 8 (4 KiB, so the destination check passes), blf_blkno -> the
zeroed area of the device, blf_data_map 0x7 -> 0x55555555 (16 runs),
blf_size left at 2 (ri_total = 2). With 16 runs the index reaches 16
and ri_buf[2..15] are read out of bounds:

  BUG: KASAN: slab-out-of-bounds in xlog_recover_do_reg_buffer+0x676/0x740
  Read of size 8 ... by task init/1
   xlog_recover_do_reg_buffer -> xlog_recover_buf_commit_pass2
   -> xlog_recover_process_data -> xlog_do_recovery_pass -> xfs_log_mount

3/3 at mount(2); the unmodified image mounts cleanly. The function is
byte-identical to mainline as of 2026-11-09.

## Suggested fix

The same guard as item 8, applied to both functions:

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

or better, check before the ri_buf[i] use at the top of each
iteration. With the destination check, the nbits clamp and this
index check in place, the ASSERTs can go away entirely.

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

Thanks,
Guikun Yang