lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ