[<prev] [next>] [day] [month] [year] [list]
Message-ID: <CACpam_aKbixkFp4e6W-81FiCu=Y3izJwPsmvfUu3_A0ZOYUNHQ@mail.gmail.com>
Date: Wed, 26 Nov 2025 09:39:57 +0800
From: yao xiao <xiangyaof4free@...il.com>
To: linux-f2fs-devel@...ts.sourceforge.net
Cc: jaegeuk@...nel.org, chao@...nel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] f2fs: fix potential integer overflow in update_sit_entry
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 ||
Content of type "text/html" skipped
Download attachment "f2fs-overflow-fix.patch" of type "application/octet-stream" (2355 bytes)
Powered by blists - more mailing lists