[PATCH AUTOSEL 7.0-5.10] affs: bound hash_pos before table lookup in affs_readdir

[Sasha Levin <sashal@kernel.org>]

From: Hyungjung Joo <jhj140711@gmail.com>

[ Upstream commit 6fa253b38b9b293a0de2a361de400557ca7666ca ]

affs_readdir() decodes ctx->pos into hash_pos and chain_pos and then
dereferences AFFS_HEAD(dir_bh)->table[hash_pos] before validating
that hash_pos is within the runtime table bound. Treat out-of-range
positions as end-of-directory before the first table lookup.

Signed-off-by: Hyungjung Joo <jhj140711@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a thorough analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: PARSE THE SUBJECT LINE
- **Subsystem**: `affs` (Amiga Fast File System)
- **Action verb**: "bound" — implies adding a missing bounds check
- **Summary**: Bound `hash_pos` before using it as an array index into
  `AFFS_HEAD(dir_bh)->table[]` in `affs_readdir()`

Record: [affs] [bound/validate] [Add missing bounds check on hash_pos
before table array lookup in readdir]

### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- **Signed-off-by**: Hyungjung Joo <jhj140711@gmail.com> — the author
- **Reviewed-by**: David Sterba <dsterba@suse.com> — the AFFS maintainer
- **Signed-off-by**: David Sterba <dsterba@suse.com> — the AFFS
  maintainer applied it
- No Fixes: tag, no Reported-by:, no Link:, no Cc: stable — all expected
  for autosel candidates.

Record: Patch was reviewed AND applied by the subsystem maintainer
(David Sterba is listed as AFFS maintainer in MAINTAINERS). Strong
quality signal.

### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The message explains:
1. `affs_readdir()` decodes `ctx->pos` into `hash_pos` and `chain_pos`.
2. It then dereferences `AFFS_HEAD(dir_bh)->table[hash_pos]` **before**
   validating that `hash_pos` is within the runtime bound
   (`s_hashsize`).
3. The fix treats out-of-range positions as end-of-directory before the
   first table lookup.

Record: Bug = out-of-bounds array access. Symptom = potential read
beyond buffer. The author clearly understands the bug mechanism.

### Step 1.4: DETECT HIDDEN BUG FIXES
This is NOT hidden — it's explicitly a missing bounds check (a real out-
of-bounds access fix).

Record: This is a direct bug fix adding a missing safety check.

---

## PHASE 2: DIFF ANALYSIS - LINE BY LINE

### Step 2.1: INVENTORY THE CHANGES
- **Files changed**: 1 (`fs/affs/dir.c`)
- **Lines added**: 2
- **Lines removed**: 0
- **Function modified**: `affs_readdir()`
- **Scope**: Single-file, single-function, 2-line surgical fix.

Record: Extremely minimal change — 2 lines added in one function in one
file.

### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE

The fix adds this check between lines 121 and 123 (before the first
array dereference):

```c
if (hash_pos >= AFFS_SB(sb)->s_hashsize)
    goto done;
```

**BEFORE**: `hash_pos` derived from `(ctx->pos - 2) >> 16` is used
directly as an index into `table[]` with no validation. The only bounds
check is in the later `for` loop at line 139.

**AFTER**: If `hash_pos >= s_hashsize`, we jump to `done` which cleanly
saves state and returns (end-of-directory).

### Step 2.3: IDENTIFY THE BUG MECHANISM

This is a **buffer overflow / out-of-bounds read** (category g -
logic/correctness + category d - memory safety):

- `struct affs_head` has a flexible array member `__be32 table[]` (from
  `amigaffs.h` line 84)
- `table` occupies space within the disk block buffer. Its valid size is
  `s_hashsize = blocksize / 4 - 56` entries (set in `super.c` line 401)
- `hash_pos` comes from `(ctx->pos - 2) >> 16`. Since `ctx->pos` is a
  `loff_t` and can be set via `lseek()` on the directory file
  descriptor, a user can set it to any value
- An out-of-range `hash_pos` reads past the allocated block buffer,
  which is a heap buffer overread

Contrast with other callers: `affs_hash_name()` (used in `namei.c` and
`amigaffs.c`) returns `hash % AFFS_SB(sb)->s_hashsize` — always bounded.
But `affs_readdir()` is the ONLY place where `hash_pos` comes from user-
controlled `ctx->pos` without bounds validation.

Record: Out-of-bounds array read. `hash_pos` from user-controlled
`ctx->pos` used as index into `table[]` without bounds check. Fix adds
the check before the first dereference.

### Step 2.4: ASSESS THE FIX QUALITY
- **Obviously correct**: Yes. The check `hash_pos >= s_hashsize` is the
  exact same condition used in the `for` loop at line 139. The `goto
  done` label already exists and is the correct cleanup path.
- **Minimal/surgical**: Yes. 2 lines, single function, no side effects.
- **Regression risk**: Essentially zero. For valid `hash_pos` values,
  behavior is unchanged. For invalid values that previously caused OOB
  access, we now cleanly return end-of-directory.

Record: Fix is trivially correct, minimal, and carries no regression
risk.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: BLAME THE CHANGED LINES
From git blame, line 123 (`ino =
be32_to_cpu(AFFS_HEAD(dir_bh)->table[hash_pos]);`) is attributed to
`^1da177e4c3f41` — Linus Torvalds, 2005-04-16 — the initial Linux
2.6.12-rc2 commit.

Record: The buggy code has been present since **Linux 2.6.12-rc2
(2005)**. This means the bug exists in **every stable tree ever**.

### Step 3.2: FOLLOW THE FIXES TAG
No Fixes: tag present. However, the bug effectively traces back to
`1da177e4c3f41` (Linux 2.6.12-rc2).

Record: Bug predates all current stable trees.

### Step 3.3: CHECK FILE HISTORY
The file `fs/affs/dir.c` has been remarkably stable. Between v5.15 and
v6.6, there were **zero changes**. The only change between v6.6 and v7.0
was `bad74142a04bf` (affs: store cookie in private data, 2024-08-30)
which refactored how the iversion cookie is stored. The core readdir
logic including the buggy lines hasn't changed since 2005.

Record: Very stable file. No prerequisites needed. The fix is
standalone.

### Step 3.4: CHECK THE AUTHOR
Hyungjung Joo doesn't appear to have other commits in this tree (not a
regular contributor). However, the patch was reviewed and applied by
David Sterba, who is the AFFS maintainer per MAINTAINERS.

Record: External contributor, but vetted by the subsystem maintainer.

### Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
The fix adds a simple check using `AFFS_SB(sb)->s_hashsize` and the
existing `done` label — both present in all kernel versions since 6.6+
(and much earlier). No dependencies.

Record: Completely standalone. No prerequisites.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Steps 4.1-4.5
Lore.kernel.org was not accessible due to bot protection. b4 dig could
not find the commit hash (the commit isn't in this tree). However:

- The patch was reviewed by the AFFS maintainer David Sterba (`Reviewed-
  by:`)
- David Sterba also applied it (`Signed-off-by:` as committer)
- The "Odd Fixes" maintenance status in MAINTAINERS means this subsystem
  only gets bug fixes, which is consistent with this patch being a fix.

Record: Could not fetch lore discussion due to bot protection. The
maintainer's review and sign-off provide sufficient confidence.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: IDENTIFY KEY FUNCTIONS
Modified function: `affs_readdir()`

### Step 5.2: TRACE CALLERS
`affs_readdir` is registered as `.iterate_shared` in
`affs_dir_operations`. It is called by the VFS `getdents`/`readdir`
syscall path when reading entries from an AFFS directory. This is
directly reachable from userspace by any user who can mount and read
AFFS filesystems.

### Step 5.3-5.4: CALL CHAIN
Userspace path: `getdents64()` syscall -> `iterate_dir()` ->
`affs_readdir()` -> `AFFS_HEAD(dir_bh)->table[hash_pos]` (OOB access)

The user controls `ctx->pos` via `lseek()` on the directory fd. Setting
it to a large value produces a large `hash_pos` that triggers the OOB
read.

Record: Directly reachable from userspace. Any user with access to an
AFFS mount can trigger this.

### Step 5.5: SEARCH FOR SIMILAR PATTERNS
Other AFFS table accesses (in `namei.c`, `amigaffs.c`) use
`affs_hash_name()` which returns `hash % s_hashsize` — always bounded.
The `readdir` path is the only one that computes `hash_pos` from user-
controlled input without bounds checking.

Record: This is the only vulnerable access pattern; other paths are
properly bounded.

---

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

### Step 6.1: DOES THE BUGGY CODE EXIST IN STABLE TREES?
Yes. The buggy line (line 123) is from the original 2005 commit. It
exists in **all** stable trees. Verified that `fs/affs/dir.c` had zero
changes between v5.15 and v6.6, and only one minor refactor
(`bad74142a04bf`) between v6.6 and v7.0.

Record: Bug exists in all active stable trees.

### Step 6.2: BACKPORT COMPLICATIONS
The patch context differs slightly between v7.0 (where `data->ino` and
`data->cookie` are used) and v6.6 and older (where `file->private_data`
and `file->f_version` are used). However, the fix inserts between the
iversion check and the table lookup, and the critical line `ino =
be32_to_cpu(AFFS_HEAD(dir_bh)->table[hash_pos])` is identical across all
versions. The patch may need minor context adjustment for trees before
v6.12, but the fix itself is trivially portable.

Record: Clean apply on v6.12+; may need minor context fixup for v6.6 and
older. Trivially adaptable.

### Step 6.3: RELATED FIXES ALREADY IN STABLE
No related fixes found. This specific OOB access has never been patched
before.

Record: No duplicate fix exists.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: IDENTIFY SUBSYSTEM AND CRITICALITY
- **Subsystem**: `fs/affs/` — Amiga Fast File System
- **Criticality**: PERIPHERAL — niche filesystem, but used for Amiga
  disk image access and retro-computing communities
- **Maintenance status**: "Odd Fixes" — only bug fixes accepted,
  consistent with this patch

Record: Peripheral subsystem. However, filesystem bugs can cause data
corruption or security issues, and the fix is trivially safe.

### Step 7.2: SUBSYSTEM ACTIVITY
Very low activity — a handful of commits over years. This is a mature,
stable codebase. The bug has been latent for 20 years.

Record: Mature subsystem. Bug has been present since the beginning.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: AFFECTED USERS
Users who mount AFFS filesystems (Amiga-format media). This is niche but
real — retro-computing, disk image forensics, embedded systems with
Amiga hardware.

Record: Niche filesystem users, but any system that processes AFFS
images is affected.

### Step 8.2: TRIGGER CONDITIONS
- Mount an AFFS filesystem
- Open a directory
- `lseek()` the directory fd to a position where `(pos - 2) >> 16 >=
  s_hashsize`
- Call `getdents()` (or any readdir)

This is **trivially triggerable by an unprivileged local user** with
access to the mount, or by a **crafted disk image** (e.g., automounted
removable media).

Record: Easily triggered. Unprivileged user can trigger via lseek +
getdents. Also triggerable via crafted disk images.

### Step 8.3: FAILURE MODE SEVERITY
The OOB read on `AFFS_HEAD(dir_bh)->table[hash_pos]` reads beyond the
block buffer (`dir_bh->b_data`). This can:
- **Read garbage data** from adjacent slab objects → potential
  **information leak**
- **Trigger KASAN** reports (slab-out-of-bounds)
- **Crash** if the read hits an unmapped page
- Use the garbage value as a block number for `affs_bread()`, leading to
  further **unpredictable behavior**

Record: Severity = **HIGH**. Out-of-bounds heap read with potential for
crash, information leak, or cascading corruption.

### Step 8.4: RISK-BENEFIT RATIO
- **BENEFIT**: Fixes an OOB read reachable from userspace, present in
  all stable trees for 20 years. Prevents potential crash/info-leak.
- **RISK**: 2 lines, uses existing check pattern and existing `done`
  label. Effectively zero regression risk.
- **Ratio**: Very high benefit, near-zero risk.

Record: Excellent risk/benefit ratio.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: COMPILE THE EVIDENCE

**Evidence FOR backporting:**
- Fixes a real out-of-bounds array read bug
- Directly reachable from unprivileged userspace (lseek + getdents)
- Also triggerable by crafted disk images
- Bug exists since Linux 2.6.12-rc2 (2005) — present in ALL stable trees
- Fix is 2 lines, obviously correct, and uses existing patterns
- Reviewed and applied by the AFFS maintainer (David Sterba)
- Zero regression risk
- No dependencies on other patches

**Evidence AGAINST backporting:**
- AFFS is a niche filesystem (low user population)
- Minor context adjustment may be needed for pre-6.12 stable trees
- No syzbot report or Reported-by (but the bug is clearly real from code
  inspection)

**Unresolved:**
- Could not access lore discussion due to bot protection (not impactful
  — maintainer review provides sufficient confidence)

### Step 9.2: STABLE RULES CHECKLIST
1. **Obviously correct and tested?** YES — trivial bounds check,
   reviewed by maintainer
2. **Fixes a real bug?** YES — out-of-bounds array read from user-
   controlled input
3. **Important issue?** YES — potential crash, info leak, or undefined
   behavior from userspace
4. **Small and contained?** YES — 2 lines in one function in one file
5. **No new features or APIs?** CORRECT — purely a safety check
6. **Can apply to stable trees?** YES — clean or near-clean apply across
   all active stable trees

### Step 9.3: EXCEPTION CATEGORIES
Not an exception category — this is a standard bug fix.

### Step 9.4: DECISION
This is a small, obviously correct bounds check that prevents an out-of-
bounds array access reachable from unprivileged userspace. It has been
reviewed and applied by the subsystem maintainer, carries no regression
risk, and applies to all stable trees. This is a textbook stable
backport candidate.

---

## Verification

- [Phase 1] Parsed tags: Reviewed-by David Sterba (AFFS maintainer), SOB
  from David Sterba as committer
- [Phase 2] Diff analysis: 2 lines added in `affs_readdir()`, adds
  bounds check `if (hash_pos >= s_hashsize) goto done;` before first
  `table[hash_pos]` access
- [Phase 2] Verified `struct affs_head` has `__be32 table[]` flexible
  array member (`amigaffs.h:77-85`)
- [Phase 2] Verified `s_hashsize = blocksize / 4 - 56` (`super.c:401`)
- [Phase 2] Verified `hash_pos = (ctx->pos - 2) >> 16` derived from
  user-controllable file position
- [Phase 3] git blame: buggy line 123 from commit `1da177e4c3f41` (Linux
  2.6.12-rc2, 2005), present in all stable trees
- [Phase 3] git log: zero changes to dir.c between v5.15 and v6.6; one
  unrelated refactor `bad74142a04bf` between v6.6 and v7.0
- [Phase 3] MAINTAINERS: David Sterba listed as AFFS maintainer, status
  "Odd Fixes"
- [Phase 5] Verified `affs_hash_name()` returns `hash % s_hashsize`
  (bounded), but `affs_readdir` computes hash_pos from unchecked user
  input
- [Phase 5] Verified `affs_readdir` is called via `.iterate_shared` in
  VFS readdir path — directly reachable from getdents syscall
- [Phase 6] Verified v6.6 `fs/affs/dir.c` has identical buggy code at
  the same location
- [Phase 6] No duplicate fix found in any stable tree
- UNVERIFIED: Could not access lore discussion due to bot protection
  (does not affect decision — maintainer review confirmed via tags)

**YES**

 fs/affs/dir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/affs/dir.c b/fs/affs/dir.c
index 5c8d83387a394..075c18c4ccde6 100644
--- a/fs/affs/dir.c
+++ b/fs/affs/dir.c
@@ -119,6 +119,8 @@ affs_readdir(struct file *file, struct dir_context *ctx)
 		pr_debug("readdir() left off=%d\n", ino);
 		goto inside;
 	}
+	if (hash_pos >= AFFS_SB(sb)->s_hashsize)
+		goto done;
 
 	ino = be32_to_cpu(AFFS_HEAD(dir_bh)->table[hash_pos]);
 	for (i = 0; ino && i < chain_pos; i++) {
-- 
2.53.0