[PATCH AUTOSEL 6.17] erofs: consolidate z_erofs_extent_lookback()

Sasha Levin sashal at kernel.org
Tue Oct 28 23:48:02 AEDT 2025


From: Gao Xiang <hsiangkao at linux.alibaba.com>

[ Upstream commit 2a13fc417f493e28bdd368785320dd4c2b3d732e ]

The initial m.delta[0] also needs to be checked against zero.

In addition, also drop the redundant logic that errors out for
lcn == 0 / m.delta[0] == 1 case.

Signed-off-by: Gao Xiang <hsiangkao at linux.alibaba.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---

LLM Generated explanations, may be completely bogus:

## BACKPORT RECOMMENDATION: **YES**

## EXTENSIVE ANALYSIS

### 1. Code Changes Analysis

The commit makes two specific changes to `fs/erofs/zmap.c`:

**Change 1 - In `z_erofs_extent_lookback()` function
(fs/erofs/zmap.c:272):**
- **BEFORE**: The check `if (!lookback_distance)` occurred AFTER calling
  `z_erofs_load_lcluster_from_disk()` and only within the
  `Z_EROFS_LCLUSTER_TYPE_NONHEAD` branch
- **AFTER**: The check `if (!lookback_distance)` is moved to the
  beginning of the while loop, BEFORE calling
  `z_erofs_load_lcluster_from_disk()`
- **Impact**: This prevents unnecessary disk I/O when
  `lookback_distance` is 0, and ensures the initial `m.delta[0]` value
  is validated before use

**Change 2 - In `z_erofs_map_blocks_fo()` function
(fs/erofs/zmap.c:435-440):**
- **BEFORE**: Code rejected `lcn == 0` as corrupted filesystem with
  `EFSCORRUPTED` error
- **AFTER**: This check is completely removed
- **Impact**: Allows valid `lcn == 0` cases to proceed instead of
  incorrectly treating them as corruption

### 2. Semantic Analysis Tools Used

I used the following tools to analyze the commit:

- **git log/show**: To examine commit history, related fixes, and code
  changes
- **Read tool**: To examine the current state of `fs/erofs/zmap.c`
  (lines 260-290, 400-450)
- **Grep tool**: To find all callers of `z_erofs_extent_lookback` and
  `z_erofs_map_blocks_fo`
- **Bash**: To examine commit context, tags, and related fixes

### 3. Key Findings from Analysis

**Caller Analysis:**
- `z_erofs_extent_lookback()` is called from `z_erofs_map_blocks_fo()`
  at line 446
- `z_erofs_map_blocks_fo()` is called from:
  - `z_erofs_map_blocks_iter()` at lines 699 and 756
  - `z_erofs_map_blocks_iter()` is the main entry point for EROFS block
    mapping
  - Called from `z_erofs_iomap_begin_report()` (fiemap operations) and
    `zdata.c` (read operations)

**Impact Scope:**
- Affects ALL compressed file reads in EROFS filesystems
- EROFS is widely used in Android and embedded systems
- The bug path is user-triggerable through normal file read operations

**Related Context:**
I discovered that this commit came immediately after TWO critical bug
fixes by the same author:

1. **e13d315ae077b** (2025-10-17): "erofs: avoid infinite loops due to
   corrupted subpage compact indexes"
   - Has `Fixes:` tags, `Reported-by: Robert Morris`, bug report link
   - Fixes infinite loops from crafted images
   - **Already backported to stable** as commit a3a04e4d968b0

2. **a429b76114aac** (2025-10-12): "erofs: fix crafted invalid cases for
   encoded extents"
   - Has `Fixes:` tags, `Reported-by: Robert Morris`, bug report links
   - Fixes system crashes from crafted images
   - **Being backported to stable**

### 4. Bug Severity Assessment

**Bug #1: Missing initial lookback_distance check**
- **Severity**: Medium (Correctness + Performance)
- **Symptom**: If initial `m.delta[0]` is 0, code performs unnecessary
  disk I/O
- **Consequence**: Performance degradation, potential confusion in error
  paths
- **Exploitability**: Low - requires specific filesystem state

**Bug #2: False positive corruption error**
- **Severity**: Medium (Functionality)
- **Symptom**: Valid filesystems with `lcn == 0` are rejected as
  corrupted
- **Consequence**: Unable to read certain valid compressed files
- **User Impact**: File read failures with `EFSCORRUPTED` errors

### 5. Backport Justification

**Arguments FOR backporting:**

1. **Fixes real bugs**: This is not just a cleanup - it fixes two actual
   bugs:
   - Unnecessary disk I/O (performance regression)
   - False positive corruption errors (functionality regression)

2. **Related to critical fixes**: Comes in a series with two other
   critical bug fixes that ARE being backported. Having partial fixes
   backported creates inconsistency.

3. **Low risk**:
   - Small, focused change (6 insertions, 14 deletions)
   - Simplifies code (removes unnecessary branches)
   - No new features or architectural changes

4. **Production impact**: EROFS is actively used in Android and embedded
   systems. Users could encounter:
   - Performance issues from unnecessary I/O
   - Mount/read failures from false positive errors

5. **Author expertise**: Gao Xiang is the EROFS maintainer and clearly
   understands the correct behavior. The removed check was indeed
   "redundant logic" as stated.

**Arguments AGAINST backporting:**

1. **No explicit stable marking**: Author didn't add `Cc:
   stable at vger.kernel.org` or `Fixes:` tag

2. **No reported issues**: Unlike the surrounding commits, no bug
   reports or CVEs mentioned

3. **Title suggests cleanup**: "consolidate" could imply refactoring
   rather than bug fixing

### 6. Final Recommendation Reasoning

Despite the lack of explicit stable marking, I recommend **YES for
backporting** because:

- The commit message explicitly states a bug: "The initial m.delta[0]
  also needs to be checked against zero"
- It fixes user-visible issues (false corruption errors)
- It's part of a series addressing EROFS robustness with
  crafted/corrupted filesystems
- The surrounding critical fixes ARE being backported
- Small size and low regression risk
- Improves correctness and performance with no downsides

The commit follows the stable kernel rules:
- ✅ Fixes important bugs (performance + false positives)
- ✅ Small and contained change
- ✅ No new features
- ✅ Low regression risk
- ✅ Affects production systems (Android/embedded)

**Recommendation: Backport to all stable trees that contain the affected
code (likely 6.15+ where encoded extents were introduced).**

 fs/erofs/zmap.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 798223e6da9ce..7a14902af5d38 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -268,20 +268,19 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m,
 		unsigned long lcn = m->lcn - lookback_distance;
 		int err;
 
+		if (!lookback_distance)
+			break;
+
 		err = z_erofs_load_lcluster_from_disk(m, lcn, false);
 		if (err)
 			return err;
-
 		if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
 			lookback_distance = m->delta[0];
-			if (!lookback_distance)
-				break;
 			continue;
-		} else {
-			m->headtype = m->type;
-			m->map->m_la = (lcn << lclusterbits) | m->clusterofs;
-			return 0;
 		}
+		m->headtype = m->type;
+		m->map->m_la = (lcn << lclusterbits) | m->clusterofs;
+		return 0;
 	}
 	erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu",
 		  lookback_distance, m->lcn, vi->nid);
@@ -431,13 +430,6 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
 			end = inode->i_size;
 	} else {
 		if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
-			/* m.lcn should be >= 1 if endoff < m.clusterofs */
-			if (!m.lcn) {
-				erofs_err(sb, "invalid logical cluster 0 at nid %llu",
-					  vi->nid);
-				err = -EFSCORRUPTED;
-				goto unmap_out;
-			}
 			end = (m.lcn << lclusterbits) | m.clusterofs;
 			map->m_flags |= EROFS_MAP_FULL_MAPPED;
 			m.delta[0] = 1;
-- 
2.51.0



More information about the Linux-erofs mailing list