[PATCH] f2fs: fix potential integer overflow in update_sit_entry

yao xiao posted 1 patch 5 days, 21 hours ago
fs/f2fs/segment.c | 23 ++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
[PATCH] f2fs: fix potential integer overflow in update_sit_entry
Posted by yao xiao 5 days, 21 hours ago
Hi Jaegeuk and Chao,

I found a potential integer overflow/underflow issue in the
update_sit_entry()
function in fs/f2fs/segment.c. The problem occurs when adding a signed int
(del)
to an unsigned int (se->valid_blocks), which can lead to undefined behavior
and potential data corruption.

Problem Analysis:
=================
1. se->valid_blocks is a 10-bit bitfield (max value: 1023) in struct
seg_entry
2. When del is negative and large, the unsigned arithmetic can cause
underflow
   due to signed-to-unsigned conversion in C, resulting in a very large
   positive number instead of a negative value
3. When del is positive and large, the result can exceed the bitfield
capacity
4. The original code performs the addition first, then checks the result,
which
   is too late - the overflow/underflow has already occurred

Example scenarios:
- If se->valid_blocks = 5 and del = -10, the unsigned arithmetic will wrap
  around because -10 is converted to UINT_MAX - 9, producing an incorrect
large
  positive value
- If se->valid_blocks = 1020 and del = 10, the result exceeds the 10-bit
  capacity and will be truncated when assigned back to se->valid_blocks

Root cause:
In C, when adding unsigned int and int, the int is converted to unsigned
int.
For negative values, this causes wrap-around, leading to incorrect results.

Solution:
=========
This patch adds pre-calculation overflow/underflow checks before performing
the arithmetic operation. The checks ensure:
- For positive del: first verify del doesn't exceed max_valid, then check
  that se->valid_blocks + del won't exceed max_valid
- For negative del: verify se->valid_blocks is large enough to subtract
|del|

The fix maintains the original semantics by still calling
f2fs_usable_blks_in_seg() in the final f2fs_bug_on() check, while using
the pre-calculated max_valid for the overflow prevention logic.

Related history:
================
I found a similar overflow fix in commit a9af3fdcc425 ("f2fs: fix potential
overflow") which addressed a related issue in build_sit_entries(). This
suggests the community is aware of and accepts such defensive programming
practices for preventing integer overflows in f2fs.

Testing:
========
I've verified that the fix compiles without errors and maintains the same
logic flow. The overflow checks are performed before the calculation,
ensuring
that invalid operations are caught early, preventing undefined behavior.

Please review and let me know if you have any questions or suggestions.

Best regards,
xiaoyao

---
 fs/f2fs/segment.c | 23 ++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b45eace879d7..05ab34600e32 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2569,13 +2569,31 @@ static void update_sit_entry(struct f2fs_sb_info
*sbi, block_t blkaddr, int del)
  struct seg_entry *se;
  unsigned int segno, offset;
  long int new_vblocks;
+ unsigned int max_valid;

  segno = GET_SEGNO(sbi, blkaddr);
  if (segno == NULL_SEGNO)
  return;

  se = get_seg_entry(sbi, segno);
- new_vblocks = se->valid_blocks + del;
+ max_valid = f2fs_usable_blks_in_seg(sbi, segno);
+
+ /* Check for overflow/underflow before calculation to avoid
+ * integer overflow when adding signed int to unsigned int.
+ * This prevents undefined behavior from unsigned arithmetic
+ * when del is negative.
+ */
+ if (del > 0) {
+ if ((unsigned int)del > max_valid ||
+    se->valid_blocks > max_valid - (unsigned int)del) {
+ f2fs_bug_on(sbi, 1);
+ return;
+ }
+ } else if (del < 0) {
+ if (se->valid_blocks < (unsigned int)(-del)) {
+ f2fs_bug_on(sbi, 1);
+ return;
+ }
+ }
+
+ new_vblocks = (long int)se->valid_blocks + del;
  offset = GET_BLKOFF_FROM_SEG0(sbi, blkaddr);

  f2fs_bug_on(sbi, (new_vblocks < 0 ||
From: xiaoyao <xiangyaof4free@gmail.com>
Date: Wed, 26 Nov 2025 08:54:08 +0800
Subject: [PATCH] f2fs: fix potential integer overflow in update_sit_entry

Add overflow/underflow checks before calculating new_vblocks to prevent
integer overflow when adding signed int (del) to unsigned int (valid_blocks).

The issue occurs because:
1. se->valid_blocks is a 10-bit bitfield (max 1023) in struct seg_entry
2. When del is negative and large, unsigned arithmetic can cause underflow
   due to signed-to-unsigned conversion, resulting in a very large positive
   number instead of a negative value
3. When del is positive and large, the result can exceed the bitfield capacity
4. The original code performs the addition first, then checks, which is too late

This fix adds pre-calculation checks to detect overflow/underflow conditions
before performing the arithmetic operation, preventing potential data corruption
and undefined behavior.

Signed-off-by: xiaoyao <xiangyaof4free@gmail.com>
---
 fs/f2fs/segment.c | 23 ++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b45eace879d7..05ab34600e32 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2569,13 +2569,31 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
 	struct seg_entry *se;
 	unsigned int segno, offset;
 	long int new_vblocks;
+	unsigned int max_valid;
 
 	segno = GET_SEGNO(sbi, blkaddr);
 	if (segno == NULL_SEGNO)
 		return;
 
 	se = get_seg_entry(sbi, segno);
-	new_vblocks = se->valid_blocks + del;
+	max_valid = f2fs_usable_blks_in_seg(sbi, segno);
+
+	/* Check for overflow/underflow before calculation to avoid
+	 * integer overflow when adding signed int to unsigned int.
+	 * This prevents undefined behavior from unsigned arithmetic
+	 * when del is negative.
+	 */
+	if (del > 0) {
+		if ((unsigned int)del > max_valid ||
+		    se->valid_blocks > max_valid - (unsigned int)del) {
+			f2fs_bug_on(sbi, 1);
+			return;
+		}
+	} else if (del < 0) {
+		if (se->valid_blocks < (unsigned int)(-del)) {
+			f2fs_bug_on(sbi, 1);
+			return;
+		}
+	}
+
+	new_vblocks = (long int)se->valid_blocks + del;
 	offset = GET_BLKOFF_FROM_SEG0(sbi, blkaddr);
 
 	f2fs_bug_on(sbi, (new_vblocks < 0 ||
-- 
2.25.1