Re: [PATCH] f2fs: validate orphan inode entry count

[Wenjie Qi]

  Yes. I sent a separate f2fs-tools patch to recognize and print the new
  ERROR_INCONSISTENT_ORPHAN s_errors bit in fsck.f2fs:

https://lore.kernel.org/linux-f2fs-devel/20260526113505.1312431-1-qiwenjie@xxxxxxxxxx/T/#u

  Thanks,

On Tue, May 26, 2026 at 7:03 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 5/26/26 13:38, Wenjie Qi wrote:
> > Agreed, SBI_NEED_FSCK may not be persisted at this stage.
> >
> >   I sent v2 to add ERROR_INCONSISTENT_ORPHAN and call f2fs_handle_error() on
> >   invalid orphan entry_count, so the corruption reason can be recorded in
> >   s_errors[] as a persistent hint for fsck.
>
> So, it needs another patch to let fsck recognize the new flag?
>
> Thanks,
>
> >
> > https://lore.kernel.org/linux-f2fs-devel/20260526053557.1096229-1-qiwenjie@xxxxxxxxxx/T/#u
> >
> > On Tue, May 26, 2026 at 10:19 AM Chao Yu <chao@xxxxxxxxxx> wrote:
> >>
> >> On 5/25/26 19:46, Wenjie Qi wrote:
> >>> f2fs_recover_orphan_inodes() trusts the orphan block entry_count when
> >>> replaying orphan inodes from the checkpoint pack.  A corrupted
> >>> entry_count larger than F2FS_ORPHANS_PER_BLOCK makes the recovery loop
> >>> read past the ino[] array and interpret footer or following data as
> >>> inode numbers.
> >>>
> >>> On a crafted image, mounting an unpatched kernel can drive orphan
> >>> recovery into f2fs_bug_on() and panic the kernel.  Validate entry_count
> >>> before consuming entries so corrupted checkpoint data fails the mount
> >>> with -EFSCORRUPTED and requests fsck instead.
> >>>
> >>> Fixes: 127e670abfa7 ("f2fs: add checkpoint operations")
> >>> Cc: stable@xxxxxxxxxx
> >>> Signed-off-by: Wenjie Qi <qiwenjie@xxxxxxxxxx>
> >>> ---
> >>>  fs/f2fs/checkpoint.c | 13 ++++++++++++-
> >>>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >>> index c00a6b6ebcbd..fc72b69ff769 100644
> >>> --- a/fs/f2fs/checkpoint.c
> >>> +++ b/fs/f2fs/checkpoint.c
> >>> @@ -943,6 +943,7 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
> >>>       for (i = 0; i < orphan_blocks; i++) {
> >>>               struct folio *folio;
> >>>               struct f2fs_orphan_block *orphan_blk;
> >>> +             unsigned int entry_count;
> >>>
> >>>               folio = f2fs_get_meta_folio(sbi, start_blk + i);
> >>>               if (IS_ERR(folio)) {
> >>> @@ -951,7 +952,17 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
> >>>               }
> >>>
> >>>               orphan_blk = folio_address(folio);
> >>> -             for (j = 0; j < le32_to_cpu(orphan_blk->entry_count); j++) {
> >>> +             entry_count = le32_to_cpu(orphan_blk->entry_count);
> >>> +             if (entry_count > F2FS_ORPHANS_PER_BLOCK) {
> >>> +                     f2fs_err(sbi, "invalid orphan inode entry count %u",
> >>> +                              entry_count);
> >>> +                     set_sbi_flag(sbi, SBI_NEED_FSCK);
> >>
> >> Well, at this stage, I guess there is no chance to persist SBI_NEED_FSCK flag,
> >> what about introduce ERROR_INCONSISTENT_ORPHAN in enum f2fs_error, so that
> >> we can persist the new bit to provide hint to fsck?
> >>
> >> Thanks,
> >>
> >>> +                     err = -EFSCORRUPTED;
> >>> +                     f2fs_folio_put(folio, true);
> >>> +                     goto out;
> >>> +             }
> >>> +
> >>> +             for (j = 0; j < entry_count; j++) {
> >>>                       nid_t ino = le32_to_cpu(orphan_blk->ino[j]);
> >>>
> >>>                       err = recover_orphan_inode(sbi, ino);
> >>
>