From: Chao Yu <[email protected]>

[ Upstream commit 8fc6056dcf79937c46c97fa4996cda65956437a9 ]

As reported, on-disk footer.ino and footer.nid is the same and
out-of-range, let's add sanity check on f2fs_alloc_nid() to detect
any potential corruption in free_nid_list.

Signed-off-by: Chao Yu <[email protected]>
Signed-off-by: Jaegeuk Kim <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---

LLM Generated explanations, may be completely bogus:

YES

Why this is a good stable backport
- Fixes a real corruption case: detects out-of-range node IDs (NIDs) in
  the allocator’s `free_nid_list`, preventing silent use of a corrupted
  NID that can cascade into further on-disk corruption and/or memory
  corruption.
- Minimal, surgical changes in f2fs: adds a guard and a stop-checkpoint
  reason; no architectural refactor.
- Low regression risk: the new branch runs only when a NID is out-of-
  range, which should never happen on a healthy filesystem.

What changed (by file/function)
- fs/f2fs/node.c: Adds a shared helper.
  - Adds `is_invalid_nid(sbi, nid)` to centralize the validity check
    (NID < root or NID >= max_nid).
- fs/f2fs/node.c: f2fs_check_nid_range
  - Replaces inline check with `is_invalid_nid` for consistency and adds
    error signaling via `f2fs_handle_error(..., ERROR_CORRUPTED_INODE)`
    when invalid. Reference point in older trees: fs/f2fs/node.c:33
    starts the function; it currently calls `set_sbi_flag(...,
    SBI_NEED_FSCK)` and warns, but lacks `f2fs_handle_error`.
- fs/f2fs/node.c: f2fs_alloc_nid
  - After taking the first entry from `free_nid_list`, adds an immediate
    range check and bails out on corruption:
    - Logs “Corrupted nid %u in free_nid_list”
    - Stops checkpoints for a safe error handling path.
  - Reference points in 5.4: function start at fs/f2fs/node.c:2424; the
    list head read at fs/f2fs/node.c:2444 is where the new check would
    insert.
- include/linux/f2fs_fs.h: Adds `STOP_CP_REASON_CORRUPTED_NID` to stop-
  checkpoint reasons so error path reports a specific cause.

Why it matters
- Without the check, a corrupted NID taken from `free_nid_list` will be
  used for preallocation and bitmap updates (see
  fs/f2fs/node.c:2448–2452), which can lead to out-of-bounds accesses or
  further filesystem metadata damage.
- The fix converts a silent corruption into a contained, explicitly
  reported error, prompting fsck and preventing further damage.

Backport considerations across stable series
- Newer stable (that already has stop-checkpoint “reasons” and
  `f2fs_handle_error`):
  - Apply as-is: add `is_invalid_nid`, call it in
    `f2fs_check_nid_range`, add the early check in `f2fs_alloc_nid`, and
    extend `stop_cp_reason` with `STOP_CP_REASON_CORRUPTED_NID`.
- Older stable (e.g., v5.4 as in this tree):
  - API differences to adapt:
    - `f2fs_stop_checkpoint` is 2-arg (fs/f2fs/checkpoint.c:26). Use
      `f2fs_stop_checkpoint(sbi, false)` and omit the reason.
    - There is no `f2fs_handle_error` or `enum f2fs_error`; keep
      existing `SBI_NEED_FSCK` + warning in `f2fs_check_nid_range` and
      do not add the handle_error call.
    - `include/linux/f2fs_fs.h` in 5.4 does not define stop reasons;
      skip the reason addition.
  - Core safety fix remains identical: insert `is_invalid_nid` and the
    early bail-out in `f2fs_alloc_nid` before using the NID or touching
    bitmaps.

Risk assessment
- Scope: confined to f2fs nid allocation and a shared helper; no cross-
  subsystem impact.
- Behavioral change only on corrupted states; normal systems unaffected.
- Performance overhead negligible (one branch + function inline).

Security/integrity impact
- Prevents potential out-of-bounds/invalid accesses when updating NID
  bitmaps and allocator state, improving robustness against on-disk
  corruption. Turning silent misuse into a hard, reported error reduces
  data-loss risk.

Additional notes from repository review
- In v5.4 (for-greg/5.4-203), `f2fs_stop_checkpoint` has no reason
  parameter (fs/f2fs/checkpoint.c:26) and there is no
  `f2fs_handle_error`. The backport should therefore be the reduced
  variant described above.
- The allocator path where the check is added corresponds to
  `fs/f2fs/node.c:2424` onward, taking the first entry from
  `free_nid_list` at `fs/f2fs/node.c:2444`, exactly the spot where the
  sanity check prevents misuse.

Conclusion
- This is a classic stable-eligible bugfix: small, contained, and
  prevents real corruption. Backport it, adapting the error/stop-CP API
  to each stable series as needed.

 fs/f2fs/node.c          | 17 ++++++++++++++++-
 include/linux/f2fs_fs.h |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 92054dcbe20d0..4254db453b2d3 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -27,12 +27,17 @@ static struct kmem_cache *free_nid_slab;
 static struct kmem_cache *nat_entry_set_slab;
 static struct kmem_cache *fsync_node_entry_slab;
 
+static inline bool is_invalid_nid(struct f2fs_sb_info *sbi, nid_t nid)
+{
+       return nid < F2FS_ROOT_INO(sbi) || nid >= NM_I(sbi)->max_nid;
+}
+
 /*
  * Check whether the given nid is within node id range.
  */
 int f2fs_check_nid_range(struct f2fs_sb_info *sbi, nid_t nid)
 {
-       if (unlikely(nid < F2FS_ROOT_INO(sbi) || nid >= NM_I(sbi)->max_nid)) {
+       if (unlikely(is_invalid_nid(sbi, nid))) {
                set_sbi_flag(sbi, SBI_NEED_FSCK);
                f2fs_warn(sbi, "%s: out-of-range nid=%x, run fsck to fix.",
                          __func__, nid);
@@ -2654,6 +2659,16 @@ bool f2fs_alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
                f2fs_bug_on(sbi, list_empty(&nm_i->free_nid_list));
                i = list_first_entry(&nm_i->free_nid_list,
                                        struct free_nid, list);
+
+               if (unlikely(is_invalid_nid(sbi, i->nid))) {
+                       spin_unlock(&nm_i->nid_list_lock);
+                       f2fs_err(sbi, "Corrupted nid %u in free_nid_list",
+                                                               i->nid);
+                       f2fs_stop_checkpoint(sbi, false,
+                                       STOP_CP_REASON_CORRUPTED_NID);
+                       return false;
+               }
+
                *nid = i->nid;
 
                __move_free_nid(sbi, i, FREE_NID, PREALLOC_NID);
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 2f8b8bfc0e731..6afb4a13b81d6 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -79,6 +79,7 @@ enum stop_cp_reason {
        STOP_CP_REASON_FLUSH_FAIL,
        STOP_CP_REASON_NO_SEGMENT,
        STOP_CP_REASON_CORRUPTED_FREE_BITMAP,
+       STOP_CP_REASON_CORRUPTED_NID,
        STOP_CP_REASON_MAX,
 };
 
-- 
2.51.0



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to