f2fs: missing cross-check between NAT entry ino and recovered node footer ino in do_recover_data() - kernel BUG at mount with CONFIG_F2FS_CHECK_FS

[Guikun Yang]

Hi,

Mounting a crafted f2fs image ends in a kernel BUG during fsync data
recovery, before any syscall beyond mount():

  F2FS-fs (vda): f2fs_recover_fsync_data: recovery fsync data, check_only: 0
  F2FS-fs (vda): do_recover_data: start to recover dnode
  kernel BUG at fs/f2fs/recovery.c:696!
  RIP: 0010:f2fs_recover_fsync_data+0x7ce6/0x9730
  Kernel panic - not syncing: Fatal exception

The BUG is the f2fs_bug_on() at fs/f2fs/recovery.c:698:

  err = f2fs_get_node_info(sbi, dn.nid, &ni, false);
  if (err)
          goto err;

  f2fs_bug_on(sbi, ni.ino != ino_of_node(sbi, entry));

ni.ino comes from the NAT, ino_of_node() from the node page footer being
recovered. The two values are read from independent on-disk structures
and nothing on the way to the assertion cross-checks them:

  f2fs_get_node_info()    validates only the blk_addr range
  __get_node_page()       validates nid == footer.nid only
  do_read_inode()         no ino cross-check either

So a NAT entry whose ino disagrees with the footer ino of the node the
NAT points at reaches the assertion, which is BUG_ON() under
CONFIG_F2FS_CHECK_FS.

## The two configurations behave differently

Under CONFIG_F2FS_CHECK_FS this is a kernel BUG at mount time. Without
it, f2fs_bug_on() degrades to WARN_ON(1) plus SBI_NEED_FSCK, and
do_recover_data() then proceeds to

  fill_node_footer(sbi, dn.node_entry, dn.nid, ni.ino, ...);

at recovery.c:790 - the NAT's wrong ino is written into the recovered
node's footer. On a production configuration the same input does not
crash; it silently stamps inconsistent metadata into the node page.
The missing cross-check is identical in both configurations.

## The ofs_of_node case was already fixed; the ino case was not

In 2018, a crafted image triggering do_recover_data() through an
inconsistent ofs_of_node was reported (bugzilla 203227) and fixed by
22d61e286e2d ("f2fs: fix to avoid panic in do_recover_data()"): the
ofs mismatch is handled today by the f2fs_warn() + -EFSCORRUPTED +
ERROR_INCONSISTENT_FOOTER path directly below this assertion.

The ino mismatch at the line above it was left as f2fs_bug_on(). It is
the same class of inconsistency, read from the same kind of
independently crafted on-disk state, in the same function, and it still
takes the whole kernel down.

f2fs_sanity_check_node_footer() (node.c:1545) added later checks
nid and the inode/xattr type combination, but not ino either.

## Suggested fix

Treat the ino mismatch the same way the ofs mismatch next to it is
already treated:

  -       f2fs_bug_on(sbi, ni.ino != ino_of_node(sbi, entry));
  +       if (ni.ino != ino_of_node(sbi, entry)) {
  +               f2fs_warn(sbi, "Inconsistent ino_of_node, ino:%llu, nid:%lu, %u",
  +                         inode->i_ino, dn.nid, ino_of_node(sbi, entry));
  +               err = -EFSCORRUPTED;
  +               f2fs_handle_error(sbi, ERROR_INCONSISTENT_FOOTER);
  +               goto err;
  +       }

Notes on reproduction that matter: the crafted state needs the
checkpoint version raised and the checkpoint checksums recomputed on
both copies (is_recoverable_dnode() requires the footer cp_ver to match),
and the NAT journal entry has to be updated rather than the NAT page
copies alone, since the journal takes precedence during lookup.

The reproducer (image builder, forged checkpoint/NAT builder, ready-made
image, crash log) is available on request.

Tested on 7.3.0 with CONFIG_F2FS_CHECK_FS=y, deterministic 3/3; the
assertion and the surrounding recovery path are unchanged in current
mainline.

Thanks,
Guikun Yang