From nobody Mon Dec 1 23:35:00 2025 Received: from mail-qk1-f178.google.com (mail-qk1-f178.google.com [209.85.222.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 26E3D307AD3 for ; Wed, 26 Nov 2025 01:40:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764121219; cv=none; b=T1tXbwh7zQWIcJbKI1aPEUsx9vPbzApSoi8/ZCqMV+3yV6rUJ2rYd7b8jSDhTHmH3cXQPTpcT2jkCjt5dEZZqm070QYSJElSgpJ7mx3JZFEb8PEj6ZZPxZcFg0yqx2/SjBUtqrg3c9/SabBZLayWWHZtAhZ/6ZxAeg5pzcT1kFw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764121219; c=relaxed/simple; bh=LgE+1syrJiCS1Bb21rRu7bOJggc5MBuwgY6+UkKEkbU=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=PRaPe8DS63M4MxnyMLuPPyp04nkUaHJ1E9dSRKCS0CN885c83oqTdpUzZf1mrKoBH6IWXOk6PdfLtr0H9+e3XRbq+q6bavQVZ35Qf0dbI4J45sC14UCsE8i/FBKmehx8VDMc53mid1OHmnvkPjtpbHp+e1SeAtZ1PQkfWnrgJak= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=GCWHszlr; arc=none smtp.client-ip=209.85.222.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="GCWHszlr" Received: by mail-qk1-f178.google.com with SMTP id af79cd13be357-8b29ff9d18cso586267385a.3 for ; Tue, 25 Nov 2025 17:40:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764121209; x=1764726009; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=y0eIM0UZAaXO1kr/7olikazwHtsW8YoK8TUriXMHOt4=; b=GCWHszlrxL58zx8TlUtivoLoJkvsgmwuKKE+CDfSxzvi5oauzedkN2wT2s56irh261 C97gvInzmZ5aM6LjrSao2LQdrFuNjgFbLS318wlwddmFrSitaEqPgIbTNa1QmTDsUd8D o56k5x7s+rdEMT+AqTiGekn5ZOmLuIxjNnLI73wPS0uVgCP/wdsJ+wmC4AQdUv+BcTIw vZ5AVm+4iC9TVXPedyZIxBoaaUrD7oqpNrST+asUj7WCTmrv0t8FNI316gnTCw0ozHht R/BPGvxa6Ke1FvFg4rM5dOTeQL/X0SuQdrwqoSAPin4FgaPg53Rr5voqe6v8ONl/kzpr 80zA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764121209; x=1764726009; h=cc:to:subject:message-id:date:from:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=y0eIM0UZAaXO1kr/7olikazwHtsW8YoK8TUriXMHOt4=; b=YSr3Z61gYwQmOCl1v5lJimje0XsOq+kck7oinzIvoRGlLcuJRSjZ5/z5wDhM4YG6qD tx+VUOG6MJqF7vSWk5k59D7OPAmX1Xj5fPl1jWU7mVW638M7tj2moj7u/6w8bIOiPkkD nsfulsuVgcmfL5tiQt+2j8AXKMgkE/2bUZSrrnBXxJ2inrNMsS7dYYj3tgvGC7ooE9r0 iIjYUzBpolreznE8Cx47wvKcNsctvtXgh4dXBy8Z6FSSJ+RBQOT/wna2XBZ5K7PFRNPG pSrgLbMUMBDrrxDsZyhI3Kkid0/+hbaDugmpb+V77xw0ZzqTmxMtw4QwLHpSVdVXjw3Y BLqw== X-Forwarded-Encrypted: i=1; AJvYcCWBOxx/3RTLAGZN4uChEaOUxJ1BKb4jO/YSW1nnmAUXzCpw7GRI9Iq4DJdbEX7QlOoDa/WEGuAPaoDhDmc=@vger.kernel.org X-Gm-Message-State: AOJu0YxEkkLNmuGR7fXJ5O0apwaoxmgxsSR5hB+d67W/X82+Kp4mO46T BvQBHkNzhGZzr1aFLr5961S/ep4QlOhlU+4GvXtfw8JPAsoBryddqrbmwAJ/1+LVFx00FMmNMCq 9DqV4zWo9PVonwSr95lA/7qA/7nYHfMGTcDC6JZrOUw== X-Gm-Gg: ASbGncuuoe5a0Zg4/zmK53bhOqxIn6VZ548Nl1w3H1stGFL4BWnup9TiR0VKikcQX+p 6+dvc5EwRg6Vioh92xdeJh22KFtxjJ1OXyy1fjdz0IApN9ZsUSjtPg1rAOHJywteB02U97y1gqQ ZPPIMWbPUUEML2aQRUJIWz1wJd8/3wepyDArENwFYDC2EqGLHYX2/0ArdojdwvsOe8zfmxy5aXa fOWSlwF9nOD1g6jR4Ahtr2cPEQIOhhcJspM98AjokWTs6KDTQa/1VORW+tK7tjCvIAYUXw= X-Google-Smtp-Source: AGHT+IHRV0dGVEwxd4In44N65j1TD27dmSO6BjyX3tCF+iwcECSP0hU9jTSMb5EIhR2FjJGXia1RqJrqurjtMVbdJ5c= X-Received: by 2002:a05:620a:400c:b0:883:9b33:f6b6 with SMTP id af79cd13be357-8b33d48b7e1mr2454953885a.84.1764121209355; Tue, 25 Nov 2025 17:40:09 -0800 (PST) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: yao xiao Date: Wed, 26 Nov 2025 09:39:57 +0800 X-Gm-Features: AWmQ_blF1KKaukZovy99CRXg2hxHfHfYUhNSKCPMUBrME7CTsOuqlWBk0w5DRVs Message-ID: Subject: [PATCH] f2fs: fix potential integer overflow in update_sit_entry To: linux-f2fs-devel@lists.sourceforge.net Cc: jaegeuk@kernel.org, chao@kernel.org, linux-kernel@vger.kernel.org Content-Type: multipart/mixed; boundary="0000000000000670500644757a03" --0000000000000670500644757a03 Content-Type: multipart/alternative; boundary="00000000000006704f0644757a01" --00000000000006704f0644757a01 Content-Type: text/plain; charset="UTF-8" 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 || --00000000000006704f0644757a01 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Jaegeuk and Chao,

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

Problem Analysis:
=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
1. se->valid_blocks is a 10-bit bitfie= ld (max value: 1023) in struct seg_entry
2. When del is negative and lar= ge, the unsigned arithmetic can cause underflow
=C2=A0 =C2=A0due to sign= ed-to-unsigned conversion in C, resulting in a very large
=C2=A0 =C2=A0p= ositive number instead of a negative value
3. When del is positive and l= arge, the result can exceed the bitfield capacity
4. The original code p= erforms the addition first, then checks the result, which
=C2=A0 =C2=A0i= s too late - the overflow/underflow has already occurred

Example sce= narios:
- If se->valid_blocks =3D 5 and del =3D -10, the unsigned ari= thmetic will wrap
=C2=A0 around because -10 is converted to UINT_MAX - 9= , producing an incorrect large
=C2=A0 positive value
- If se->vali= d_blocks =3D 1020 and del =3D 10, the result exceeds the 10-bit
=C2=A0 c= apacity and will be truncated when assigned back to se->valid_blocks
=
Root cause:
In C, when adding unsigned int and int, the int is conve= rted to unsigned int.
For negative values, this causes wrap-around, lead= ing to incorrect results.

Solution:
=3D=3D=3D=3D=3D=3D=3D=3D=3DThis patch adds pre-calculation overflow/underflow checks before performi= ng
the arithmetic operation. The checks ensure:
- For positive del: f= irst verify del doesn't exceed max_valid, then check
=C2=A0 that se-= >valid_blocks + del won't exceed max_valid
- For negative del: ve= rify se->valid_blocks is large enough to subtract |del|

The fix m= aintains 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:
=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
I found a similar overflow fi= x in commit a9af3fdcc425 ("f2fs: fix potential
overflow") whic= h addressed a related issue in build_sit_entries(). This
suggests the co= mmunity is aware of and accepts such defensive programming
practices for= preventing integer overflows in f2fs.

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

Please review and let me know if you have an= y questions or suggestions.

Best regards,
xiaoyao

---
= =C2=A0fs/f2fs/segment.c | 23 ++++++++++++++++++++--
=C2=A01 file changed= , 21 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/segment.c b/= fs/f2fs/segment.c
index b45eace879d7..05ab34600e32 100644
--- a/fs/f2= fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2569,13 +2569,31 @@ static v= oid update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)=C2=A0 struct seg_entry *se;
=C2=A0 unsigned int segno, offset;
=C2= =A0 long int new_vblocks;
+ unsigned int max_valid;
=C2=A0
=C2=A0 = segno =3D GET_SEGNO(sbi, blkaddr);
=C2=A0 if (segno =3D=3D NULL_SEGNO)=C2=A0 return;
=C2=A0
=C2=A0 se =3D get_seg_entry(sbi, segno);
= - new_vblocks =3D se->valid_blocks + del;
+ max_valid =3D f2fs_usable= _blks_in_seg(sbi, segno);
+
+ /* Check for overflow/underflow before = calculation to avoid
+ * integer overflow when adding signed int to uns= igned int.
+ * This prevents undefined behavior from unsigned arithmeti= c
+ * when del is negative.
+ */
+ if (del > 0) {
+ if ((= unsigned int)del > max_valid ||
+ =C2=A0 =C2=A0se->valid_blocks = > max_valid - (unsigned int)del) {
+ f2fs_bug_on(sbi, 1);
+ re= turn;
+ }
+ } else if (del < 0) {
+ if (se->valid_blocks &= lt; (unsigned int)(-del)) {
+ f2fs_bug_on(sbi, 1);
+ return;
+= }
+ }
+
+ new_vblocks =3D (long int)se->valid_blocks + del;=C2=A0 offset =3D GET_BLKOFF_FROM_SEG0(sbi, blkaddr);
=C2=A0
=C2=A0= f2fs_bug_on(sbi, (new_vblocks < 0 ||

--00000000000006704f0644757a01-- --0000000000000670500644757a03 Content-Type: application/octet-stream; name="f2fs-overflow-fix.patch" Content-Disposition: attachment; filename="f2fs-overflow-fix.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_mifc4yyi0 RnJvbTogeGlhb3lhbyA8eGlhbmd5YW9mNGZyZWVAZ21haWwuY29tPg0KRGF0ZTogV2VkLCAyNiBO b3YgMjAyNSAwODo1NDowOCArMDgwMA0KU3ViamVjdDogW1BBVENIXSBmMmZzOiBmaXggcG90ZW50 aWFsIGludGVnZXIgb3ZlcmZsb3cgaW4gdXBkYXRlX3NpdF9lbnRyeQ0KDQpBZGQgb3ZlcmZsb3cv dW5kZXJmbG93IGNoZWNrcyBiZWZvcmUgY2FsY3VsYXRpbmcgbmV3X3ZibG9ja3MgdG8gcHJldmVu dA0KaW50ZWdlciBvdmVyZmxvdyB3aGVuIGFkZGluZyBzaWduZWQgaW50IChkZWwpIHRvIHVuc2ln bmVkIGludCAodmFsaWRfYmxvY2tzKS4NCg0KVGhlIGlzc3VlIG9jY3VycyBiZWNhdXNlOg0KMS4g c2UtPnZhbGlkX2Jsb2NrcyBpcyBhIDEwLWJpdCBiaXRmaWVsZCAobWF4IDEwMjMpIGluIHN0cnVj dCBzZWdfZW50cnkNCjIuIFdoZW4gZGVsIGlzIG5lZ2F0aXZlIGFuZCBsYXJnZSwgdW5zaWduZWQg YXJpdGhtZXRpYyBjYW4gY2F1c2UgdW5kZXJmbG93DQogICBkdWUgdG8gc2lnbmVkLXRvLXVuc2ln bmVkIGNvbnZlcnNpb24sIHJlc3VsdGluZyBpbiBhIHZlcnkgbGFyZ2UgcG9zaXRpdmUNCiAgIG51 bWJlciBpbnN0ZWFkIG9mIGEgbmVnYXRpdmUgdmFsdWUNCjMuIFdoZW4gZGVsIGlzIHBvc2l0aXZl IGFuZCBsYXJnZSwgdGhlIHJlc3VsdCBjYW4gZXhjZWVkIHRoZSBiaXRmaWVsZCBjYXBhY2l0eQ0K NC4gVGhlIG9yaWdpbmFsIGNvZGUgcGVyZm9ybXMgdGhlIGFkZGl0aW9uIGZpcnN0LCB0aGVuIGNo ZWNrcywgd2hpY2ggaXMgdG9vIGxhdGUNCg0KVGhpcyBmaXggYWRkcyBwcmUtY2FsY3VsYXRpb24g Y2hlY2tzIHRvIGRldGVjdCBvdmVyZmxvdy91bmRlcmZsb3cgY29uZGl0aW9ucw0KYmVmb3JlIHBl cmZvcm1pbmcgdGhlIGFyaXRobWV0aWMgb3BlcmF0aW9uLCBwcmV2ZW50aW5nIHBvdGVudGlhbCBk YXRhIGNvcnJ1cHRpb24NCmFuZCB1bmRlZmluZWQgYmVoYXZpb3IuDQoNClNpZ25lZC1vZmYtYnk6 IHhpYW95YW8gPHhpYW5neWFvZjRmcmVlQGdtYWlsLmNvbT4NCi0tLQ0KIGZzL2YyZnMvc2VnbWVu dC5jIHwgMjMgKysrKysrKysrKysrKysrKysrKystLQ0KIDEgZmlsZSBjaGFuZ2VkLCAyMSBpbnNl cnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvZnMvZjJmcy9zZWdtZW50 LmMgYi9mcy9mMmZzL3NlZ21lbnQuYw0KaW5kZXggYjQ1ZWFjZTg3OWQ3Li4wNWFiMzQ2MDBlMzIg MTAwNjQ0DQotLS0gYS9mcy9mMmZzL3NlZ21lbnQuYw0KKysrIGIvZnMvZjJmcy9zZWdtZW50LmMN CkBAIC0yNTY5LDEzICsyNTY5LDMxIEBAIHN0YXRpYyB2b2lkIHVwZGF0ZV9zaXRfZW50cnkoc3Ry dWN0IGYyZnNfc2JfaW5mbyAqc2JpLCBibG9ja190IGJsa2FkZHIsIGludCBkZWwpDQogCXN0cnVj dCBzZWdfZW50cnkgKnNlOw0KIAl1bnNpZ25lZCBpbnQgc2Vnbm8sIG9mZnNldDsNCiAJbG9uZyBp bnQgbmV3X3ZibG9ja3M7DQorCXVuc2lnbmVkIGludCBtYXhfdmFsaWQ7DQogDQogCXNlZ25vID0g R0VUX1NFR05PKHNiaSwgYmxrYWRkcik7DQogCWlmIChzZWdubyA9PSBOVUxMX1NFR05PKQ0KIAkJ cmV0dXJuOw0KIA0KIAlzZSA9IGdldF9zZWdfZW50cnkoc2JpLCBzZWdubyk7DQotCW5ld192Ymxv Y2tzID0gc2UtPnZhbGlkX2Jsb2NrcyArIGRlbDsNCisJbWF4X3ZhbGlkID0gZjJmc191c2FibGVf Ymxrc19pbl9zZWcoc2JpLCBzZWdubyk7DQorDQorCS8qIENoZWNrIGZvciBvdmVyZmxvdy91bmRl cmZsb3cgYmVmb3JlIGNhbGN1bGF0aW9uIHRvIGF2b2lkDQorCSAqIGludGVnZXIgb3ZlcmZsb3cg d2hlbiBhZGRpbmcgc2lnbmVkIGludCB0byB1bnNpZ25lZCBpbnQuDQorCSAqIFRoaXMgcHJldmVu dHMgdW5kZWZpbmVkIGJlaGF2aW9yIGZyb20gdW5zaWduZWQgYXJpdGhtZXRpYw0KKwkgKiB3aGVu IGRlbCBpcyBuZWdhdGl2ZS4NCisJICovDQorCWlmIChkZWwgPiAwKSB7DQorCQlpZiAoKHVuc2ln bmVkIGludClkZWwgPiBtYXhfdmFsaWQgfHwNCisJCSAgICBzZS0+dmFsaWRfYmxvY2tzID4gbWF4 X3ZhbGlkIC0gKHVuc2lnbmVkIGludClkZWwpIHsNCisJCQlmMmZzX2J1Z19vbihzYmksIDEpOw0K KwkJCXJldHVybjsNCisJCX0NCisJfSBlbHNlIGlmIChkZWwgPCAwKSB7DQorCQlpZiAoc2UtPnZh bGlkX2Jsb2NrcyA8ICh1bnNpZ25lZCBpbnQpKC1kZWwpKSB7DQorCQkJZjJmc19idWdfb24oc2Jp LCAxKTsNCisJCQlyZXR1cm47DQorCQl9DQorCX0NCisNCisJbmV3X3ZibG9ja3MgPSAobG9uZyBp bnQpc2UtPnZhbGlkX2Jsb2NrcyArIGRlbDsNCiAJb2Zmc2V0ID0gR0VUX0JMS09GRl9GUk9NX1NF RzAoc2JpLCBibGthZGRyKTsNCiANCiAJZjJmc19idWdfb24oc2JpLCAobmV3X3ZibG9ja3MgPCAw IHx8DQotLSANCjIuMjUuMQ0K --0000000000000670500644757a03--