Message-ID: <20251209001610.611575-4-sashal@kernel.org>
Date: Mon,  8 Dec 2025 19:14:56 -0500
From: Sasha Levin <sashal@...nel.org>
To: patches@...ts.linux.dev,
	stable@...r.kernel.org
Cc: Zhihao Cheng <chengzhihao1@...wei.com>,
	Baokun Li <libaokun1@...wei.com>,
	Zhang Yi <yi.zhang@...wei.com>,
	Jan Kara <jack@...e.cz>,
	Ojaswin Mujoo <ojaswin@...ux.ibm.com>,
	Theodore Ts'o <tytso@....edu>,
	Sasha Levin <sashal@...nel.org>,
	adilger.kernel@...ger.ca,
	linux-ext4@...r.kernel.org
Subject: [PATCH AUTOSEL 6.18-5.10] ext4: remove page offset calculation in ext4_block_zero_page_range()

From: Zhihao Cheng <chengzhihao1@...wei.com>

[ Upstream commit 5835b1339e33549d9e7342fae56243b4fcd758c9 ]

For bs <= ps scenarios, calculating the offset within the block is
sufficient. For bs > ps, an initial page offset calculation can lead to
incorrect behavior. Thus this redundant calculation has been removed.

Signed-off-by: Zhihao Cheng <chengzhihao1@...wei.com>
Signed-off-by: Baokun Li <libaokun1@...wei.com>
Reviewed-by: Zhang Yi <yi.zhang@...wei.com>
Reviewed-by: Jan Kara <jack@...e.cz>
Reviewed-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Message-ID: <20251121090654.631996-2-libaokun@...weicloud.com>
Signed-off-by: Theodore Ts'o <tytso@....edu>
Signed-off-by: Sasha Levin <sashal@...nel.org>
---

LLM Generated explanations, may be completely bogus:

## Commit Analysis: ext4_block_zero_page_range() page offset calculation
fix

### 1. COMMIT MESSAGE ANALYSIS

The commit removes a page offset calculation in
`ext4_block_zero_page_range()`. Key points from the message:
- For bs <= ps (block size <= page size): calculating block offset alone
  is sufficient
- For bs > ps (block size > page size): the page offset calculation
  leads to "incorrect behavior"
- The calculation is described as "redundant"

**Notable tags:**
- Multiple `Reviewed-by:` tags including **Jan Kara** (ext4 maintainer)
  and other ext4 experts
- **No `Cc: stable@...r.kernel.org`** tag
- **No `Fixes:` tag**

### 2. CODE CHANGE ANALYSIS

```c
- unsigned offset = from & (PAGE_SIZE-1);
        unsigned blocksize = inode->i_sb->s_blocksize;
- unsigned max = blocksize - (offset & (blocksize - 1));
+       unsigned int max = blocksize - (from & (blocksize - 1));
```

**The Bug:**
The old code first calculates `offset = from & (PAGE_SIZE-1)` (offset
within the page), then uses this to calculate the remaining bytes in the
block.

For **bs > ps** (e.g., 16K blocks on 4K pages):
- `offset` gets truncated to 0-4095 range (page offset)
- When calculating `offset & (blocksize - 1)`, the higher bits of the
  block offset are lost
- Example: `from = 5000` with 8K blocks → `offset = 904` (wrong), should
  use `5000 & 8191 = 5000`
- This results in calculating the wrong `max` value for how much data to
  zero

**Impact:** This function is called during truncate operations to zero
partial blocks. A wrong `max` calculation could:
- Zero the wrong range of data (data corruption)
- Not zero enough data (potential data leak of old file contents)

### 3. CLASSIFICATION

- **Type:** Bug fix (incorrect calculation for bs > ps configurations)
- **Subsystem:** ext4 filesystem (critical)
- **Not a feature addition** - purely corrective

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed:** 3 (removes 2 lines, modifies 1)
- **Files touched:** 1 (fs/ext4/inode.c)
- **Complexity:** Very low - simple arithmetic correction
- **Risk:** Very low
  - For bs > ps: Fixes the bug
  - For bs <= ps: Mathematically equivalent behavior (no change)

### 5. USER IMPACT

- **Who's affected:** Users with bs > ps ext4 configurations (large
  block filesystems)
- **Severity:** Potential data corruption during truncate operations -
  HIGH severity for affected users
- **Configuration rarity:** bs > ps is less common but becoming more
  relevant for large storage
- **For normal configurations (bs <= ps):** No behavior change, just
  cleaner code

### 6. STABILITY INDICATORS

- **Reviewed-by:** Jan Kara (ext4 maintainer), Zhang Yi, Ojaswin Mujoo -
  strong expert review
- **Sign-offs:** Multiple Huawei engineers plus Ted Ts'o (ext4
  maintainer)

### 7. DEPENDENCY CHECK

The fix is self-contained. It only changes how the `max` variable is
calculated using local variables. No dependencies on other commits.

### CONCERNS

1. **No explicit stable tag** - Maintainers didn't explicitly request
   stable backport
2. **No Fixes: tag** - Can't trace when the bug was introduced
3. **bs > ps support** is relatively recent, so older stable kernels may
   not benefit
4. **Vague description** - "incorrect behavior" isn't specific about
   user-visible symptoms

### RISK VS BENEFIT

**Benefits:**
- Fixes a real bug that could cause data corruption in ext4
- Trivially correct fix (just use `from` directly instead of page-
  truncated offset)
- Zero risk of regression for bs <= ps (same behavior)
- Expert reviewed by ext4 maintainers

**Risks:**
- Minimal - the fix is mathematically obvious
- Only changes behavior for bs > ps where the old behavior was wrong

### CONCLUSION

This is a correctness fix for ext4 filesystem that addresses a real bug
in bs > ps configurations. The change is:
- Small and surgical (3 lines)
- Obviously correct (direct calculation vs. lossy intermediate step)
- Low risk (no behavior change for common configurations)
- Expert reviewed by multiple ext4 maintainers

However, the lack of explicit `Cc: stable` or `Fixes:` tags suggests the
maintainers may not have considered this critical for stable. The bs >
ps feature is also relatively new, limiting which stable kernels would
benefit.

Given that it fixes a potential data corruption issue in a critical
filesystem with minimal risk, and the fix has been thoroughly reviewed,
it is a reasonable candidate for backporting to stable kernels that
support bs > ps ext4 configurations.

**YES**

 fs/ext4/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e99306a8f47ce..0742039c53a77 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4157,9 +4157,8 @@ static int ext4_block_zero_page_range(handle_t *handle,
 		struct address_space *mapping, loff_t from, loff_t length)
 {
 	struct inode *inode = mapping->host;
-	unsigned offset = from & (PAGE_SIZE-1);
 	unsigned blocksize = inode->i_sb->s_blocksize;
-	unsigned max = blocksize - (offset & (blocksize - 1));
+	unsigned int max = blocksize - (from & (blocksize - 1));
 
 	/*
 	 * correct length if it does not fall between
-- 
2.51.0

Powered by blists - more mailing lists