[PATCH AUTOSEL 7.0-6.18] dm cache: prevent entering passthrough mode after unclean shutdown

[PATCH AUTOSEL 7.0-6.18] dm cache: prevent entering passthrough mode after unclean shutdown

[Sasha Levin]

From: Ming-Hung Tsai <mtsai@redhat.com>

[ Upstream commit a373b3d5289e50ab26d4cf776bf5891436ff3658 ]

dm-cache assumes all cache blocks are dirty when it recovers from an
unclean shutdown. Given that the passthrough mode doesn't handle dirty
blocks, we should not load a cache in passthrough mode if it was not
cleanly shut down; or we'll risk data loss while updating an actually
dirty block.

Also bump the target version to 2.4.0 to mark completion of passthrough
mode fixes.

Reproduce steps:

1. Create a writeback cache with zero migration_threshold to produce
   dirty blocks.

dmsetup create cmeta --table "0 8192 linear /dev/sdc 0"
dmsetup create cdata --table "0 131072 linear /dev/sdc 8192"
dmsetup create corig --table "0 262144 linear /dev/sdc 262144"
dd if=/dev/zero of=/dev/mapper/cmeta bs=4k count=1 oflag=direct
dmsetup create cache --table "0 262144 cache /dev/mapper/cmeta \
/dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 writeback smq \
2 migration_threshold 0"

2. Write the first cache block dirty

fio --filename=/dev/mapper/cache --name=populate --rw=write --bs=4k \
--direct=1 --size=64k

3. Ensure the number of dirty blocks is 1. This status query triggers
   metadata commit without flushing the dirty bitset, setting up the
   unclean shutdown state.

dmsetup status cache | awk '{print $14}'

4. Force reboot, leaving the cache uncleanly shutdown.

echo b > /proc/sysrq-trigger

5. Activate the above cache components, and verify the first data block
   remains dirty.

dmsetup create cmeta --table "0 8192 linear /dev/sdc 0"
dmsetup create cdata --table "0 131072 linear /dev/sdc 8192"
dmsetup create corig --table "0 262144 linear /dev/sdc 262144"
dd if=/dev/mapper/cdata of=/tmp/cb0.bin bs=64k count=1
dd if=/dev/mapper/corig of=/tmp/ob0.bin bs=64k count=1
md5sum /tmp/cb0.bin /tmp/ob0.bin # expected to be different

6. Try bringing up the cache in passthrough mode. It succeeds, while the
   first cache block was loaded dirty due to unclean shutdown, violates
   the passthrough mode's constraints.

dmsetup create cache --table "0 262144 cache /dev/mapper/cmeta \
/dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 passthrough smq 0"
dmsetup status cache | awk '{print $14}'

7. (Optional) Demonstrate the integrity issue: invalidating the dirty
   block in passthrough mode doesn't write back the dirty data, causing
   data loss.

fio --filename=/dev/mapper/cache --name=invalidate --rw=write --bs=4k \
--direct=1 --size=4k  # overwrite the first 4k to trigger invalidation
dmsetup remove cache
dd if=/dev/mapper/corig of=/tmp/ob0new.bin bs=64k count=1
cb0sum=$(dd if=/tmp/cb0.bin bs=4k count=15 skip=1 | md5sum | \
awk '{print $1}')
ob0newsum=$(dd if=/tmp/ob0new.bin bs=4k count=15 skip=1 | md5sum | \
awk '{print $1}')
echo "$cb0sum, $ob0newsum"  # remaining 60k should differ (data loss)

Signed-off-by: Ming-Hung Tsai <mtsai@redhat.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.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 complete analysis. Let me
compile my findings.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: dm cache (device-mapper cache target)
- **Action verb**: "prevent" — indicates fixing a dangerous condition
- **Summary**: Prevents entering passthrough mode after unclean
  shutdown, which would cause data loss

### Step 1.2: Tags
- **Signed-off-by: Ming-Hung Tsai <mtsai@redhat.com>** — author, active
  dm-cache contributor
- **Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>** — dm
  subsystem maintainer
- No Fixes: tag, no Reported-by:, no Cc: stable (as expected for
  candidates)

### Step 1.3: Commit Body Analysis
The bug is clearly described: dm-cache assumes all cache blocks are
dirty after unclean shutdown. Passthrough mode doesn't handle dirty
blocks (reads from origin, writes go to origin + invalidate cache hits).
If a cache with potentially dirty blocks is loaded in passthrough mode
after an unclean shutdown, invalidating a dirty cache block discards
data that was never written back to the origin — causing **data loss**.

Detailed 7-step reproduction steps are provided with concrete commands.

### Step 1.4: Hidden Bug Fix Detection
This is explicitly a data integrity protection fix. The word "prevent"
combined with the description of data loss makes the intent unambiguous.

**Record**: Bug fix preventing data loss in dm-cache passthrough mode
after unclean shutdown.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **dm-cache-metadata.c**: +9 lines (new
  `dm_cache_metadata_clean_when_opened()` function)
- **dm-cache-metadata.h**: +5 lines (function declaration + comment)
- **dm-cache-target.c**: +17 lines changed (passthrough check in
  `can_resume()` + version bump)
- **Total**: ~31 lines added, 1 line modified
- **Functions modified**: `can_resume()` (body extended), new function
  `dm_cache_metadata_clean_when_opened()`
- **Scope**: single-subsystem surgical fix

### Step 2.2: Code Flow Change
1. **dm-cache-metadata.c**: New accessor function reads
   `cmd->clean_when_opened` under READ_LOCK. Trivial, obviously correct.
2. **dm-cache-target.c `can_resume()`**: Before the change,
   `can_resume()` only checked for failed resume retries. After, it also
   checks if we're in passthrough mode with an unclean shutdown.
3. **Version bump**: 2.3.0 → 2.4.0 — cosmetic marker for the behavioral
   change.

### Step 2.3: Bug Mechanism
This is a **data corruption / data loss** bug:
- The constructor (`cache_ctr` at line 2470) checks
  `dm_cache_metadata_all_clean()` which reads the **on-disk dirty
  bitset** — stale after an unclean shutdown
- The runtime (`__load_mapping_v1`/`__load_mapping_v2`) checks
  `clean_when_opened` and treats all blocks as dirty when false
- The gap: constructor says "all clean" (stale bitset), but runtime
  later marks everything dirty — passthrough mode then incorrectly
  invalidates blocks without writeback

### Step 2.4: Fix Quality
- **Obviously correct**: The check is a simple boolean read of an
  existing, well-tested field
- **Minimal**: Only adds essential check code
- **Regression risk**: Very low — worst case, a cache that should be
  refused in passthrough mode is correctly refused (fail-safe)

**Record**: Small, surgical fix. ~31 lines total. Three files, one
subsystem. Fix is fail-safe (blocks dangerous mode, doesn't change data
paths).

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
The `can_resume()` function was introduced by `5da692e2262b8` (Ming-Hung
Tsai, 2025-03-06), first in v6.15-rc1.

### Step 3.2: Fixes Tag
No Fixes: tag present. The underlying bug has existed since
`2ee57d587357f` ("dm cache: add passthrough mode", 2013-10-24, v3.13),
which never validated `clean_when_opened` before allowing passthrough
mode activation after a crash.

### Step 3.3: File History
The author (Ming-Hung Tsai) has contributed 10+ dm-cache fixes,
including out-of-bounds access fixes, BUG_ON prevention, and other
critical corrections. All accepted by the dm maintainer tree.

### Step 3.4: Author Assessment
Ming-Hung Tsai is a Red Hat engineer who has been a prolific dm-cache
bug fixer. Their patches go through Mikulas Patocka (dm maintainer) who
co-signs them.

### Step 3.5: Dependencies
This commit depends on `5da692e2262b8` which introduced `can_resume()`.
That commit is in v6.15+. For the 7.0.y tree, this dependency is
satisfied.

**Record**: Bug has existed since v3.13 (2013). Fix depends on
`can_resume()` from v6.15+.

---

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1-4.5
Could not access lore.kernel.org directly (Anubis protection). However,
b4 dig confirms the related series (v3 of 2 patches by Ming-Hung Tsai,
submitted to dm-devel, CC'd dm maintainers Joe Thornber, Heinz
Mauelshagen, Mike Snitzer, and Mikulas Patocka). The series went through
3 revisions, indicating active review. The commit analyzed is a follow-
up fix likely from a later submission.

**Record**: Author is well-known to dm maintainers. Prior patches in the
same series were reviewed and merged. Could not verify specific lore
discussion for this exact commit.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1-5.4: Key Functions
- `can_resume()` is called from `cache_preresume()` which is the DM
  target's `.preresume` callback — called during device activation
- `passthrough_mode()` checks `cache->features.io_mode ==
  CM_IO_PASSTHROUGH`
- `dm_cache_metadata_clean_when_opened()` reads `cmd->clean_when_opened`
  which is set from the CLEAN_SHUTDOWN superblock flag during metadata
  open

The constructor check at line 2470 (`dm_cache_metadata_all_clean`) reads
the on-disk dirty bitset, which is **stale after an unclean shutdown** —
the dirty bitset is not flushed on every dirty block write, only on
clean shutdown. The CLEAN_SHUTDOWN flag is the authoritative indicator.

### Step 5.5: Similar Patterns
Commit `5b1fe7bec8a8d` ("dm cache metadata: set dirty on all cache
blocks after a crash", 2018) fixed the same root issue for the normal
(non-passthrough) code path — it was Cc'd to stable. This commit fixes
the passthrough-specific gap.

**Record**: Bug is reachable from userspace (dmsetup commands). The
constructor check is insufficient because it reads stale on-disk data.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Code Existence
- Passthrough mode exists since v3.13 (all stable trees)
- `can_resume()` exists since v6.15 (7.0.y, 6.15.y+)
- `clean_when_opened` field exists since the beginning of dm-cache

### Step 6.2: Backport Complications
For 7.0.y: should apply cleanly (all prerequisites present, version is
2.3.0).
For trees < 6.15: would need adaptation (no `can_resume()`, check would
go directly in `cache_preresume()`).

**Record**: Clean apply expected for 7.0.y. Older trees need adaptation.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Criticality
- **Subsystem**: dm-cache (device-mapper caching layer)
- **Criticality**: IMPORTANT — used by LVM's lvmcache, enterprise
  storage stacks, and production workloads
- Data loss in a caching layer is especially severe as users expect
  transparent, reliable behavior

### Step 7.2: Activity
dm-cache has received numerous bug fixes recently, with Ming-Hung Tsai
being the most active contributor.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Affected Users
Users of dm-cache in passthrough mode after an unclean shutdown (power
failure, crash, sysrq-b). This includes LVM cache users (lvmcache) on
enterprise systems.

### Step 8.2: Trigger Conditions
1. Have a dm-cache with dirty blocks in writeback mode
2. Experience an unclean shutdown (crash, power loss)
3. Resume the cache in passthrough mode
4. Write to the cached region (triggers invalidation of dirty blocks)
5. Data loss occurs silently

### Step 8.3: Failure Mode
**DATA LOSS** — dirty data in the cache is silently discarded without
writeback to the origin device. Severity: **CRITICAL**.

### Step 8.4: Risk-Benefit Ratio
- **Benefit**: Very high — prevents silent data loss in production
  storage
- **Risk**: Very low — the fix only adds a fail-safe check that blocks a
  dangerous operation
- **Ratio**: Strongly favorable for backporting

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backporting:**
- Fixes silent data loss (CRITICAL severity)
- Small, contained fix (~31 lines, 3 files, 1 subsystem)
- Obviously correct — reads an existing, well-tested flag
- Fail-safe behavior (refuses dangerous operation rather than modifying
  data paths)
- Author is an established dm-cache contributor
- Co-signed by dm maintainer Mikulas Patocka
- Detailed reproduction steps demonstrate the bug is real and
  triggerable
- Related fix (`5b1fe7bec8a8d` from 2018) for non-passthrough path was
  Cc: stable

**AGAINST backporting:**
- Version bump from 2.3.0 to 2.4.0 (cosmetic, could be dropped for
  stable)
- Adds a new accessor function (trivial, just reads existing field)
- Dependency on `can_resume()` from v6.15+ (present in 7.0.y)

### Step 9.2: Stable Rules Checklist
1. **Obviously correct and tested?** YES — trivial accessor + simple
   conditional, with detailed repro steps
2. **Fixes a real bug?** YES — silent data loss on unclean shutdown with
   passthrough mode
3. **Important issue?** YES — data corruption/loss, CRITICAL severity
4. **Small and contained?** YES — ~31 lines, single subsystem
5. **No new features or APIs?** Correct — the new function is a private
   internal accessor, not user-facing
6. **Can apply to stable?** YES for 7.0.y (all prerequisites present)

### Step 9.3: Exception Categories
Not applicable — this is a straightforward critical bug fix.

### Step 9.4: Decision
This is a clear YES. It prevents silent data loss in dm-cache, is small
and surgical, obviously correct, and meets all stable kernel criteria.

---

## Verification

- [Phase 1] Parsed tags: Signed-off-by from Ming-Hung Tsai (author) and
  Mikulas Patocka (dm maintainer). No Fixes/Cc:stable tags (expected).
- [Phase 2] Diff analysis: ~31 lines added across 3 files. New trivial
  accessor `dm_cache_metadata_clean_when_opened()`, passthrough check in
  `can_resume()`, version bump.
- [Phase 2] Verified constructor check at line 2470-2486 calls
  `dm_cache_metadata_all_clean()` which reads stale on-disk dirty bitset
  — insufficient after crash.
- [Phase 2] Verified `blocks_are_clean_separate_dirty()` reads on-disk
  bitset (dm_bitset_cursor), not the `clean_when_opened` flag.
- [Phase 2] Verified `__load_mapping_v1` (line 1343) and
  `__load_mapping_v2` (line 1385) check `cmd->clean_when_opened` and
  treat all blocks as dirty when false.
- [Phase 3] git blame: `can_resume()` introduced by `5da692e2262b8`
  (2025-03-06, v6.15-rc1).
- [Phase 3] Passthrough mode introduced by `2ee57d587357f` (2013-10-24,
  v3.13) — bug has existed since then.
- [Phase 3] `git tag --contains 5da692e2262b8`: first appears in
  v6.15-rc1, present in 7.0.
- [Phase 3] `git tag --contains 2ee57d587357f`: present since v3.13, in
  all stable trees.
- [Phase 3] Version was 2.2.0 → 2.3.0 by `c2662b1544cbd` (same
  author/series), now 2.3.0 → 2.4.0.
- [Phase 4] b4 dig: found the related series (v3, 2 patches) on dm-
  devel. CC'd Joe Thornber, Heinz Mauelshagen, Mike Snitzer, Mikulas
  Patocka.
- [Phase 4] lore.kernel.org blocked by Anubis. Could not fetch specific
  discussion for this commit.
- [Phase 5] `clean_when_opened` is set at line 508 from CLEAN_SHUTDOWN
  flag in superblock, well-established mechanism.
- [Phase 5] `5b1fe7bec8a8d` (2018) fixed same root issue for non-
  passthrough path, was Cc: stable.
- [Phase 6] All prerequisites present in 7.0.y tree. Clean apply
  expected.
- [Phase 7] dm-cache is IMPORTANT subsystem used by LVM, enterprise
  storage.
- [Phase 8] Failure mode: silent data loss (CRITICAL). Trigger: unclean
  shutdown + passthrough mode resume + write.
- UNVERIFIED: Could not verify specific mailing list discussion or
  reviewer feedback for this exact commit due to lore.kernel.org access
  restrictions.

**YES**

 drivers/md/dm-cache-metadata.c |  9 +++++++++
 drivers/md/dm-cache-metadata.h |  5 +++++
 drivers/md/dm-cache-target.c   | 19 ++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index 57158c02d096e..70e0c6c064082 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -1824,3 +1824,12 @@ int dm_cache_metadata_abort(struct dm_cache_metadata *cmd)
 
 	return r;
 }
+
+int dm_cache_metadata_clean_when_opened(struct dm_cache_metadata *cmd, bool *result)
+{
+	READ_LOCK(cmd);
+	*result = cmd->clean_when_opened;
+	READ_UNLOCK(cmd);
+
+	return 0;
+}
diff --git a/drivers/md/dm-cache-metadata.h b/drivers/md/dm-cache-metadata.h
index 5f77890207fed..dca423522da6c 100644
--- a/drivers/md/dm-cache-metadata.h
+++ b/drivers/md/dm-cache-metadata.h
@@ -146,6 +146,11 @@ void dm_cache_metadata_set_read_only(struct dm_cache_metadata *cmd);
 void dm_cache_metadata_set_read_write(struct dm_cache_metadata *cmd);
 int dm_cache_metadata_abort(struct dm_cache_metadata *cmd);
 
+/*
+ * Query method.  Was the metadata cleanly shut down when opened?
+ */
+int dm_cache_metadata_clean_when_opened(struct dm_cache_metadata *cmd, bool *result);
+
 /*----------------------------------------------------------------*/
 
 #endif /* DM_CACHE_METADATA_H */
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 935ab79b1d0cd..962ac5ee081c2 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2929,6 +2929,9 @@ static dm_cblock_t get_cache_dev_size(struct cache *cache)
 
 static bool can_resume(struct cache *cache)
 {
+	bool clean_when_opened;
+	int r;
+
 	/*
 	 * Disallow retrying the resume operation for devices that failed the
 	 * first resume attempt, as the failure leaves the policy object partially
@@ -2945,6 +2948,20 @@ static bool can_resume(struct cache *cache)
 		return false;
 	}
 
+	if (passthrough_mode(cache)) {
+		r = dm_cache_metadata_clean_when_opened(cache->cmd, &clean_when_opened);
+		if (r) {
+			DMERR("%s: failed to query metadata flags", cache_device_name(cache));
+			return false;
+		}
+
+		if (!clean_when_opened) {
+			DMERR("%s: unable to resume into passthrough mode after unclean shutdown",
+			      cache_device_name(cache));
+			return false;
+		}
+	}
+
 	return true;
 }
 
@@ -3510,7 +3527,7 @@ static void cache_io_hints(struct dm_target *ti, struct queue_limits *limits)
 
 static struct target_type cache_target = {
 	.name = "cache",
-	.version = {2, 3, 0},
+	.version = {2, 4, 0},
 	.module = THIS_MODULE,
 	.ctr = cache_ctr,
 	.dtr = cache_dtr,
-- 
2.53.0

[PATCH AUTOSEL 7.0-6.12] dm-integrity: fix mismatched queue limits

[Sasha Levin]

From: Keith Busch <kbusch@kernel.org>

[ Upstream commit 6ebf3b6c6f16fda0568aa4207c6cd398f983c354 ]

A user can integritysetup a device with a backing device using a 4k
logical block size, but request the dm device use 1k or 2k. This
mismatch creates an inconsistency such that the dm device would report
limits for IO that it can't actually execute. Fix this by using the
backing device's limits if they are larger.

Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.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. Let me
compile the findings.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: `dm-integrity` (device mapper integrity target)
- **Action verb**: "fix"
- **Summary**: Fixes mismatched queue limits between DM integrity device
  and backing device

### Step 1.2: Tags
- `Signed-off-by: Keith Busch <kbusch@kernel.org>` - the author
- `Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>` - the DM
  integrity maintainer committed it
- No Fixes: tag, no Reported-by:, no Cc: stable (expected for manual
  review candidates)

### Step 1.3: Commit Body
The commit clearly describes the bug: a user can create an integrity
device (via `integritysetup`) on a 4k logical block size backing device
but request 1k or 2k for the DM device. The DM device then reports queue
limits it can't actually satisfy because they're below the backing
device's capabilities. This is an IO correctness bug - the device
advertises capabilities it doesn't have.

### Step 1.4: Hidden Bug Fix
No - this is an explicit "fix" with clear description. The word "fix" is
in the subject.

Record: Real bug fix. Queue limit mismatch causes IO that the backing
device cannot execute.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **Files**: `drivers/md/dm-integrity.c` only (+9, -3 lines)
- **Function**: `dm_integrity_io_hints()` - the io_hints callback
- **Scope**: Single-file, single-function surgical fix

### Step 2.2: Code Flow Change
Before: Unconditionally sets `logical_block_size`,
`physical_block_size`, and `io_min` to `ic->sectors_per_block <<
SECTOR_SHIFT`.

After: Uses `max()` to ensure these values are never LOWER than the
existing limits (which come from the backing device).

### Step 2.3: Bug Mechanism
Category: **Logic/correctness fix**. The old code overwrites the backing
device's queue limits with potentially smaller values, creating an
inconsistency where the DM device reports it accepts smaller IO than the
backing device can handle.

### Step 2.4: Fix Quality
- Obviously correct: `max()` ensures the larger of the two values is
  used
- Minimal and surgical
- Uses the exact same pattern that `dm-crypt` already uses (verified:
  `max_t(unsigned int, limits->logical_block_size, cc->sector_size)`)
- No regression risk: the fix only ever keeps limits the same or makes
  them larger, never smaller

Record: High quality fix. Pattern already proven in dm-crypt.

---

## PHASE 3: GIT HISTORY

### Step 3.1: Blame
The buggy code was introduced in commit `9d609f85b7eb96` ("dm integrity:
support larger block sizes") by Mikulas Patocka, which first appeared at
`v4.12-rc1~120^2~28` - so approximately **kernel v4.12 (2017)**. The bug
has existed for ~9 years and affects all stable trees.

### Step 3.2: Fixes Tag
No Fixes: tag present (expected for manual review).

### Step 3.3: File History
The dm-integrity.c file is actively developed with many recent changes.
The io_hints function itself has had additions (`dma_alignment` in v6.1
era, `discard_granularity` and `io_min` changes in v6.11) but the core
bug (unconditional assignment) has been present since introduction.

### Step 3.4: Author
Keith Busch (`kbusch@kernel.org`) is a well-known kernel developer and
NVMe/block layer expert at Meta. Not the dm-integrity maintainer but
very knowledgeable about block layer queue limits. The patch was
accepted by Mikulas Patocka, the dm-integrity maintainer.

### Step 3.5: Dependencies
This is patch 1/3 of a series. Patch 2 ("dm-integrity: always set the io
hints") removes the `if (sectors_per_block > 1)` guard. Patch 3 ("dm:
provide helper to set stacked limits") creates a common helper. **This
patch (1/3) is fully self-contained** - it fixes the core bug
independently. Patches 2 and 3 are enhancements/refactoring.

Record: Self-contained fix. Bug since v4.12. Accepted by maintainer.

---

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1: Original Discussion
Found via `b4 dig`: [PATCH 1/3] at
`https://patch.msgid.link/20260325193608.2827042-1-kbusch@meta.com`

Key findings from the mbox:
- **Mikulas Patocka** (maintainer): "I accepted all three patches."
- **Benjamin Marzinski** (Red Hat DM developer): Provided concrete
  reproduction demonstrating the bug causes real IO failures:

```
INVALID:
# modprobe scsi_debug dev_size_mb=1024 lbpu=1 sector_size=4096
# integritysetup format -s 1024 /dev/sda
# integritysetup open --allow-discards /dev/sda integrity-test
# cat /sys/block/sda/queue/discard_granularity
2048
# cat /sys/block/dm-1/queue/discard_granularity
1024
# blkdiscard -o 1024 -l 16384 /dev/mapper/integrity-test
blkdiscard: BLKDISCARD: /dev/mapper/integrity-test ioctl failed:
Input/output error
```

### Step 4.2: Reviewers
Sent to dm-devel@lists.linux.dev, mpatocka@redhat.com (maintainer),
snitzer@kernel.org (DM maintainer). Appropriate subsystem maintainers
were included.

### Step 4.3-4.5: No explicit stable nomination in discussion, but no
objections either.

Record: Concrete reproduction of IO failures. Maintainer accepted
immediately.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Functions Modified
Only `dm_integrity_io_hints()` modified.

### Step 5.2: Callers
Called via the `.io_hints` callback in `struct target_type
integrity_target`. This is invoked by the DM core when setting up queue
limits for the DM device - affects every dm-integrity device setup.

### Step 5.3-5.5: Impact Surface
Every dm-integrity device created via `integritysetup` (or similar) with
a block size smaller than the backing device's logical block size is
affected. This is a common user-facing operation in LUKS/dm-integrity
setups.

Record: Affects every dm-integrity device with mismatched block sizes.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Code Existence
The buggy code exists in ALL stable trees (since v4.12). Verified:
- v5.15: Same bug, uses `blk_limits_io_min()` instead of direct
  assignment
- v6.1: Same bug, same code as v5.15
- v6.6: Same bug, same code plus `dma_alignment`
- v6.12+: Same bug, uses direct `limits->io_min =` (matches the fix
  exactly)

### Step 6.2: Backport Complications
- **v6.12+**: Applies cleanly (code matches exactly)
- **v6.6 and earlier**: Minor conflict - uses `blk_limits_io_min()`
  instead of direct `limits->io_min =`. The `logical_block_size` and
  `physical_block_size` lines are identical in all versions. Only the
  io_min line needs trivial adaptation.

### Step 6.3: No related fixes already in stable for this issue.

Record: Bug present in all stable trees. Clean apply for v6.12+, trivial
adaptation for older.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem
- `drivers/md/dm-integrity.c` - Device Mapper integrity target
- **Criticality**: IMPORTANT - dm-integrity is used in LUKS setups,
  enterprise storage, and data integrity verification. It's a core
  component of the device mapper framework.

### Step 7.2: Activity
Actively developed - 20+ commits since v6.6, including several bug
fixes.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Affected Users
Users of dm-integrity devices where the integrity block size is smaller
than the backing device's logical block size. Common scenario: 4k
native-sector drives with 1k/2k integrity block sizes.

### Step 8.2: Trigger Conditions
Any IO operation (including discard) on a dm-integrity device with
mismatched block sizes. The mismatch is user-creatable via
`integritysetup`. Not timing-dependent.

### Step 8.3: Failure Mode
**IO errors** - the device accepts IO that the backing device cannot
execute. Demonstrated: `blkdiscard` fails with `Input/output error`.
Severity: **HIGH** - IO failures can cause data loss, filesystem errors,
application failures.

### Step 8.4: Risk-Benefit
- **Benefit**: HIGH - prevents IO errors on dm-integrity devices with
  common hardware configurations
- **Risk**: VERY LOW - 9 lines changed, uses `max()` pattern already
  proven in dm-crypt, only ever makes limits larger (never smaller),
  obviously correct
- **Ratio**: Strongly favorable for backporting

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Compilation

**FOR backporting:**
- Fixes real IO errors (demonstrated with concrete reproduction)
- Small, surgical fix (9 added, 3 removed, single function)
- Uses proven `max()` pattern from dm-crypt
- Bug present since v4.12 (all stable trees affected)
- Accepted by subsystem maintainer (Mikulas Patocka)
- Self-contained (no dependencies on patches 2/3)
- Reviewed by Red Hat DM developer who provided testing

**AGAINST backporting:**
- Minor adaptation needed for v6.6 and earlier (blk_limits_io_min vs
  direct assignment)
- No explicit Cc: stable nomination (but this is expected)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** - max() is trivially correct,
   maintainer accepted, Ben Marzinski tested
2. Fixes a real bug? **YES** - IO errors with concrete reproduction
3. Important issue? **YES** - IO errors leading to data access failures
4. Small and contained? **YES** - 12 lines net, single function
5. No new features? **YES** - pure bug fix
6. Can apply to stable? **YES** - cleanly for v6.12+, minor adaptation
   for older

### Step 9.3: Exception Categories
Not applicable - this is a standard bug fix, not an exception category.

### Step 9.4: Decision
Clear YES. This is a textbook stable backport candidate: small, obvious,
fixes a real user-facing bug (IO errors), long-standing (since v4.12),
accepted by the subsystem maintainer, and uses an already-proven
pattern.

---

## Verification

- [Phase 1] Parsed subject: "dm-integrity: fix mismatched queue limits"
  - explicit fix
- [Phase 1] Parsed tags: SOB from Keith Busch (author) and Mikulas
  Patocka (maintainer/committer)
- [Phase 2] Diff analysis: 3 assignments changed to max() in
  dm_integrity_io_hints(), +9/-3 lines
- [Phase 2] Verified dm-crypt uses identical max_t() pattern
  (drivers/md/dm-crypt.c line 3721)
- [Phase 3] git blame: buggy code introduced in 9d609f85b7eb96 (v4.12,
  2017)
- [Phase 3] git describe --contains: confirmed v4.12-rc1~120^2~28
- [Phase 3] Confirmed patch 1/3 is self-contained; patches 2-3 are
  enhancement/refactoring
- [Phase 4] b4 dig found:
  https://patch.msgid.link/20260325193608.2827042-1-kbusch@meta.com
- [Phase 4] b4 dig -a: single version (v1), no revisions needed
- [Phase 4] b4 dig -w: dm-devel, mpatocka, snitzer all included
- [Phase 4] Mbox review: Mikulas accepted ("I accepted all three
  patches"), Ben Marzinski provided IO error reproduction
- [Phase 5] dm_integrity_io_hints called via .io_hints callback for
  every dm-integrity device setup
- [Phase 6] Verified code exists in v5.15, v6.1, v6.6, v6.12 - all
  stable trees affected
- [Phase 6] v6.6 and earlier use blk_limits_io_min() - minor backport
  adaptation needed
- [Phase 6] v6.12+ uses direct limits->io_min assignment - clean apply
- [Phase 8] Failure mode: IO errors (EIO), demonstrated with blkdiscard,
  severity HIGH

**YES**

 drivers/md/dm-integrity.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 06e805902151c..8dfd498ed1ffd 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -4047,9 +4047,15 @@ static void dm_integrity_io_hints(struct dm_target *ti, struct queue_limits *lim
 	struct dm_integrity_c *ic = ti->private;
 
 	if (ic->sectors_per_block > 1) {
-		limits->logical_block_size = ic->sectors_per_block << SECTOR_SHIFT;
-		limits->physical_block_size = ic->sectors_per_block << SECTOR_SHIFT;
-		limits->io_min = ic->sectors_per_block << SECTOR_SHIFT;
+		limits->logical_block_size =
+				max(limits->logical_block_size,
+				    ic->sectors_per_block << SECTOR_SHIFT);
+		limits->physical_block_size =
+				max(limits->physical_block_size,
+				    ic->sectors_per_block << SECTOR_SHIFT);
+		limits->io_min =
+				max(limits->io_min,
+				    ic->sectors_per_block << SECTOR_SHIFT);
 		limits->dma_alignment = limits->logical_block_size - 1;
 		limits->discard_granularity = ic->sectors_per_block << SECTOR_SHIFT;
 	}
-- 
2.53.0

[PATCH AUTOSEL 7.0-6.12] dm vdo indexer: validate saved zone count

[Sasha Levin]

From: Matthew Sakai <msakai@redhat.com>

[ Upstream commit 9e809bb1defe9be7fed2e21552c6b03b2694394d ]

Verify that the loaded zone count is in the valid range
before using it as a loop iterator.

Signed-off-by: Matthew Sakai <msakai@redhat.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.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. Let me
compile everything.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem:** `dm vdo indexer` (device mapper, VDO deduplication
  index)
- **Action verb:** "validate" - implies adding a missing safety check
- **Summary:** Validates the zone count loaded from disk before using it
  as a loop iterator

**Record:** [dm vdo indexer] [validate] [Add bounds check on zone count
read from disk before loop use]

### Step 1.2: Tags
- `Signed-off-by: Matthew Sakai <msakai@redhat.com>` - the dm-vdo
  subsystem author/maintainer
- `Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>` - the dm
  subsystem maintainer who committed it

No Fixes: tag, no Reported-by, no Cc: stable. The absence of these is
expected for commits under manual review.

**Record:** Author is the dm-vdo subsystem maintainer. Committed through
the dm maintainer. No explicit bug reporter.

### Step 1.3: Commit Body
The message says: "Verify that the loaded zone count is in the valid
range before using it as a loop iterator." This clearly states:
- The zone count comes from loaded (on-disk) data
- It's used as a loop iterator
- Without validation, an invalid value would be used in the loop

**Record:** Bug = missing input validation on disk-loaded data used as
loop bound. Failure = out-of-bounds array access. Root cause = no bounds
check after reading from persistent storage.

### Step 1.4: Hidden Bug Fix Detection
This IS a bug fix despite using "validate" rather than "fix". It adds a
missing bounds check on data read from disk, preventing an out-of-bounds
array access. This is a classic data corruption / corrupted metadata
handling fix.

**Record:** Yes, this is a real bug fix - adding a missing bounds check
on untrusted data from disk.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Changes Inventory
- **File:** `drivers/md/dm-vdo/indexer/index-layout.c`
- **Lines added:** 3 (the `if` check + error return)
- **Function modified:** `reconstruct_index_save()`
- **Scope:** Single-file, single-function, 3-line surgical fix

**Record:** 1 file, +3 lines, extremely small and contained.

### Step 2.2: Code Flow Change
**Before:** Line 1447 computes `isl->zone_count =
table->header.region_count - 3` from disk data, then immediately uses
`zone_count` as the loop bound at line 1476: `for (z = 0; z <
isl->zone_count; z++)`, indexing into `volume_index_zones[z]`.

**After:** After computing `zone_count`, the code checks `if
(isl->zone_count > MAX_ZONES)` and returns `UDS_CORRUPT_DATA` error if
invalid.

### Step 2.3: Bug Mechanism
This is a **buffer overflow / out-of-bounds write** fix:

- `region_count` is a `u16` (0-65535) read from disk via
  `decode_u16_le()` at line 1129
- `zone_count = region_count - 3` (line 1447) - stored in `unsigned int`
- If `region_count > MAX_ZONES + 3 = 19`, then `zone_count > 16`, and
  the loop writes past the end of `volume_index_zones[MAX_ZONES]` (a
  fixed-size array of 16 entries at line 162)
- If `region_count < 3`, the subtraction wraps to a very large unsigned
  value, causing massive OOB access
- There's NO other validation of `region_count` vs `MAX_ZONES` in the
  load path

**Record:** [Out-of-bounds array access] [zone_count from disk used
without bounds check as index into fixed-size MAX_ZONES=16 array]

### Step 2.4: Fix Quality
- The fix is **obviously correct**: it checks `zone_count > MAX_ZONES`
  before the array is accessed
- It's **minimal**: exactly 3 lines
- It returns a proper error code (`UDS_CORRUPT_DATA`) with a log message
- **Zero regression risk**: it only rejects previously-invalid data that
  would have caused corruption

**Record:** Fix is obviously correct, minimal, zero regression risk.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
The buggy code was introduced in commit `b46d79bdb82aa1` ("dm vdo: add
deduplication index storage interface"), authored by Matthew Sakai on
2023-11-16. This commit first appeared in v6.9-rc1. The buggy code has
been present since the initial introduction of dm-vdo.

**Record:** Bug introduced in b46d79bdb82aa1 (v6.9-rc1). Present in all
kernels since v6.9.

### Step 3.2: Fixes Tag
No Fixes: tag present. The implicit target would be b46d79bdb82aa1.

### Step 3.3: File History
Recent changes to this file are minimal:
- `f4e99b846c901` - string warning fix (cosmetic)
- `b0e6210e7e616` - removed unused function
- `41c58a36e2c04` - use-after-free fix (similar safety concern)

There's also `9ddf6d3fcbe0b` ("dm vdo: return error on corrupted
metadata in start_restoring_volume functions") - a very similar pattern:
adding proper error returns on corrupted metadata in the same subsystem,
with a Fixes: tag.

**Record:** Standalone fix, no prerequisites. Similar metadata
validation fixes have been applied to dm-vdo.

### Step 3.4: Author
Matthew Sakai is the original author and maintainer of dm-vdo. He
authored the initial dm-vdo code (40-patch series) and continues
maintaining it. This fix comes from the subsystem maintainer.

**Record:** Author is the subsystem maintainer - highest trust level.

### Step 3.5: Dependencies
None. This is a self-contained 3-line addition that doesn't depend on
any other commits.

**Record:** No dependencies. Fully standalone.

---

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1-4.2: Patch Discussion
I was unable to find the exact mailing list submission via b4 dig (the
commit isn't in the tree yet, so there's no SHA to search). Web searches
didn't return the specific patch thread. However, the commit was signed
off by both the subsystem maintainer (Sakai) and the dm maintainer
(Patocka), indicating it went through the standard dm review process.

**Record:** Could not locate specific lore thread. Verified through
standard dm maintainer chain.

### Step 4.3: Bug Report
No Reported-by tag. This appears to be a proactive fix found through
code review by the maintainer.

**Record:** Proactive fix by maintainer, not triggered by user report.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1-5.4: Call Chain
The full call chain from user-facing API to the vulnerable function:
1. `uds_make_index_layout()` - public API for creating/loading VDO index
2. `load_index_layout()` - loads existing index from disk
3. `load_sub_index_regions()` - loads saved index regions
4. `load_index_save()` - loads individual index save
5. `load_region_table()` - reads region table from disk (reads
   `region_count` as u16)
6. **`reconstruct_index_save()`** - uses `region_count` without
   validation -> OOB

This is called during VDO volume activation/load, which happens when a
dm-vdo target is activated (e.g., mounting a VDO-backed filesystem or
activating a VDO logical volume). The data comes from on-disk metadata.

**Record:** Reachable from VDO volume activation. Triggered by corrupted
on-disk metadata.

### Step 5.5: Similar Patterns
The similar fix `9ddf6d3fcbe0b` validates corrupted metadata in
`start_restoring_volume` functions, showing this is a known pattern in
dm-vdo where disk metadata isn't sufficiently validated.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Buggy Code in Stable
dm-vdo was introduced in v6.9-rc1. Active stable trees that contain this
code:
- **v6.12.y** (LTS) - YES, contains dm-vdo
- **v6.14.y** (stable) - YES
- **v6.19.y** (stable) - YES
- v6.6.y (LTS) - NO (pre-dates dm-vdo)
- v6.1.y (LTS) - NO

**Record:** Bug exists in v6.12.y, v6.14.y, v6.19.y stable trees.

### Step 6.2: Backport Complications
Changes to the file between v6.12 and HEAD are minimal (MAGIC_SIZE
cleanup and function removal) - none affect the
`reconstruct_index_save()` function area. The patch should apply cleanly
to all stable trees with dm-vdo.

**Record:** Clean apply expected on all relevant stable trees.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem
- **Subsystem:** `drivers/md/dm-vdo` - Device Mapper VDO (deduplication
  + compression)
- **Criticality:** IMPORTANT - VDO is used for storage deduplication in
  RHEL/enterprise environments. Data integrity is paramount for storage
  subsystems.

### Step 7.2: Activity
dm-vdo sees regular maintenance commits from its author. It's an
actively maintained storage driver.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
Users of dm-vdo (VDO deduplication). This includes RHEL and enterprise
Linux users who use VDO for storage optimization.

### Step 8.2: Trigger Conditions
- **Trigger:** Corrupted on-disk VDO metadata where `region_count` is
  out of expected range
- **How likely:** Corruption can occur from disk errors, power failures,
  or malicious manipulation
- **User triggering:** Any user activating a VDO volume with corrupted
  metadata

### Step 8.3: Failure Mode Severity
Without this fix, corrupted metadata causes an **out-of-bounds array
write** on a stack-based or structure-embedded array
(`volume_index_zones[MAX_ZONES]`). This results in:
- **Stack/heap corruption** - writing past the array bounds
- **Kernel crash/panic** - likely from corrupted data structures
- **Potential privilege escalation** - corrupted kernel data structures
  from controlled input

**Severity: CRITICAL** - out-of-bounds write from disk-loaded data,
potential kernel crash or memory corruption.

### Step 8.4: Risk-Benefit Ratio
- **Benefit:** HIGH - prevents kernel crash/corruption from malformed
  on-disk metadata
- **Risk:** VERY LOW - 3-line check that only rejects invalid data; zero
  chance of regression for valid data
- **Ratio:** Extremely favorable

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backporting:**
- Fixes an out-of-bounds array access from unvalidated disk-read data
  (security-relevant)
- Only 3 lines added - minimal surgical fix
- Obviously correct - simple bounds check against well-defined constant
- Zero regression risk - only rejects data that would have caused OOB
  access
- Written by subsystem maintainer, committed through dm maintainer
- Bug exists since dm-vdo introduction (v6.9), affects all stable trees
  with dm-vdo
- Clean apply expected
- Similar fix pattern already accepted for dm-vdo (`9ddf6d3fcbe0b`)

**AGAINST backporting:**
- No explicit user report or syzbot trigger (proactive fix)
- Requires corrupted on-disk metadata to trigger (not everyday scenario)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** - trivial bounds check,
   maintainer-authored
2. Fixes a real bug? **YES** - OOB array access from unvalidated disk
   data
3. Important issue? **YES** - kernel memory corruption/crash from
   corrupted storage metadata
4. Small and contained? **YES** - 3 lines in one function
5. No new features? **YES** - pure safety check
6. Can apply to stable? **YES** - clean apply expected

### Step 9.3: Exception Categories
Not needed - this qualifies as a straightforward bug fix.

---

## Verification

- [Phase 1] Parsed tags: Signed-off-by from subsystem maintainer (Sakai)
  and dm maintainer (Patocka)
- [Phase 2] Diff analysis: 3 lines added in `reconstruct_index_save()`,
  adds `zone_count > MAX_ZONES` bounds check
- [Phase 2] Confirmed `volume_index_zones[MAX_ZONES]` at line 162,
  MAX_ZONES=16 at config.h:23
- [Phase 2] Confirmed `region_count` is u16 read from disk at line 1129,
  used unvalidated at line 1447
- [Phase 3] git blame: all code in `reconstruct_index_save` from commit
  b46d79bdb82aa1 (v6.9-rc1)
- [Phase 3] git describe: b46d79bdb82aa1 first appears at
  v6.9-rc1~148^2~132
- [Phase 3] Similar fix 9ddf6d3fcbe0b validates corrupted metadata in
  same subsystem, has Fixes: tag
- [Phase 4] Could not locate specific lore thread (commit not yet
  merged, no SHA for b4 dig)
- [Phase 5] Call chain traced: uds_make_index_layout ->
  load_index_layout -> load_sub_index_regions -> load_index_save ->
  reconstruct_index_save
- [Phase 5] Trigger: VDO volume activation with corrupted on-disk
  metadata
- [Phase 6] File exists in v6.12.y, v6.14.y, v6.19.y; confirmed via git
  log
- [Phase 6] Changes since v6.12 don't affect the patched function area -
  clean apply expected
- [Phase 7] Subsystem: dm-vdo, actively maintained, IMPORTANT
  criticality (enterprise storage)
- [Phase 8] Failure mode: OOB array write -> kernel memory
  corruption/crash, severity CRITICAL

This is a textbook stable-worthy fix: a tiny, obviously correct bounds
check that prevents an out-of-bounds array access from corrupted on-disk
metadata in an enterprise storage driver. The fix has zero regression
risk and addresses a potentially security-relevant memory corruption
bug.

**YES**

 drivers/md/dm-vdo/indexer/index-layout.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/md/dm-vdo/indexer/index-layout.c b/drivers/md/dm-vdo/indexer/index-layout.c
index 61edf2b72427d..37144249f7ba6 100644
--- a/drivers/md/dm-vdo/indexer/index-layout.c
+++ b/drivers/md/dm-vdo/indexer/index-layout.c
@@ -1445,6 +1445,9 @@ static int __must_check reconstruct_index_save(struct index_save_layout *isl,
 	u64 last_block = next_block + isl->index_save.block_count;
 
 	isl->zone_count = table->header.region_count - 3;
+	if (isl->zone_count > MAX_ZONES)
+		return vdo_log_error_strerror(UDS_CORRUPT_DATA,
+					      "invalid zone count");
 
 	last_region = &table->regions[table->header.region_count - 1];
 	if (last_region->kind == RL_KIND_EMPTY) {
-- 
2.53.0