ddt: fix refcount bypass and gang member leak for dedup gang blocks

[Zaczero]

Motivation and Context

This PR fixes two related defects in how deduplicated gang blocks are
freed and referenced.

Defect 1: traditional-DDT frees select the phys slot by BP DVA count,
bypassing refcounts.

The write path stores a traditional DDT entry in the phys slot chosen by
the logical zp_copies value:

zfs/module/zfs/zio.c

Lines 3843 to 3844 in 9bf75b4

	int p = DDT_PHYS_FOR_COPIES(ddt, zp->zp_copies);
	ddt_phys_variant_t v = DDT_PHYS_VARIANT(ddt, p);

but the free-path lookup validation selects the slot to compare by the
BP's physical DVA count:

zfs/module/zfs/ddt.c

Lines 1200 to 1228 in 9bf75b4

	static boolean_t
	ddt_entry_lookup_is_valid(ddt_t *ddt, const blkptr_t *bp, ddt_entry_t *dde)
	{
	/* If the BP has no DVAs, then this entry is good */
	uint_t ndvas = BP_GET_NDVAS(bp);
	if (ndvas == 0)
	return (B_TRUE);
	/*
	* Only checking the phys for the copies. For flat, there's only one;
	* for trad it'll be the one that has the matching set of DVAs.
	*/
	const dva_t *dvas = (ddt->ddt_flags & DDT_FLAG_FLAT) ?
	dde->dde_phys->ddp_flat.ddp_dva :
	dde->dde_phys->ddp_trad[ndvas].ddp_dva;
	/*
	* Compare entry DVAs with the BP. They should all be there, but
	* there's not really anything we can do if its only partial anyway,
	* that's an error somewhere else, maybe long ago.
	*/
	uint_t d;
	for (d = 0; d < ndvas; d++)
	if (!DVA_EQUAL(&dvas[d], &bp->blk_dva[d]))
	return (B_FALSE);
	ASSERT3U(d, ==, ndvas);
	return (B_TRUE);
	}

and ddt_addref() makes the same assumption:

zfs/module/zfs/ddt.c

Lines 2692 to 2701 in 9bf75b4

	if ((dde->dde_type < DDT_TYPES) || (dde->dde_flags & DDE_FLAG_LOGGED)) {
	/*
	* This entry was either synced to a store object (dde_type is
	* real) or was logged. It must be properly on disk at this
	* point, so we can just bump its refcount.
	*/
	int p = DDT_PHYS_FOR_COPIES(ddt, BP_GET_NDVAS(bp));
	ddt_phys_variant_t v = DDT_PHYS_VARIANT(ddt, p);
	ddt_phys_addref(dde->dde_phys, v);

The two indexes diverge whenever a block's BP carries more DVAs than
its copies value — which is exactly what ganging does. A gang header
is stored in more copies than the data it gangs, so copies=1 blocks
get a two-DVA gang header BP: unconditionally through 2.3.3, and under
the default redundant_metadata=all since a46ce73 made gang copies
configurable in 2.3.4 and 9250403 in 2.4.0. From 2.3.4 that same
default also gives unencrypted copies=2 blocks a three-DVA header
BP:

https://github.com/openzfs/zfs/blob/9bf75b4b13949afe322387fc5572c5820ccL2556

Encrypted datasets diverge at the default copies=1 as well —
encryption caps gang_copies at 2 but leaves copies at 1:

zfs/module/zfs/dmu.c

Lines 2571 to 2573 in 9bf75b4

	if (DMU_OT_IS_ENCRYPTED(type)) {
	copies = MIN(copies, SPA_DVAS_PER_BP - 1);
	gang_copies = MIN(gang_copies, SPA_DVAS_PER_BP - 1);

Through 2.3.3 only copies=1 diverges; from 2.3.4 unencrypted
copies=2 diverges as well, leaving just copies=3 and encrypted
copies>=2 lined up.

For such a block, free-time validation checks the wrong slot, concludes
the entry was previously pruned, and zio_ddt_free() falls back to a
plain physical free — the shared gang header extents are freed once
per reference delete, with no refcount decrement:

https://github.com/openzfs/zfs/blob/9bf75b4b13949afe322387fc5572c5820ccL4204

Consequences per triggering delete:

• Remaining references point at freed header sectors; reads fail with
  cksum_algorithm=gang_header checksum errors once the space is
  reused (all header copies are freed together, so mirror/raidz
  redundancy does not protect them).
• The second and later deletes commit overlapping FREE records to the
  spacemaps — surfacing as zfs: rt=...: adding segment ... overlapping with existing one panics at import or runtime — or free unrelated
  live allocations if the extents were reallocated in between,
  silently cross-linking whatever lands there next.
• The gang members are never traversed (see defect 2) and leak
  permanently once the last reference is deleted.
• The entry becomes immortal (its refcount never reaches zero), so
  future identical writes dedup against freed/reused storage and are
  lost as written. dedup=verify mitigates this vector.
• If a block was cloned while the bug was live, the failing verify
  lookup made ddt_addref() fall back to BRT
  (module/zfs/brt.c#L1283),
  leaving a DDT/BRT accounting hybrid that no later code can
  disambiguate from the BP alone.

The free-path regression was introduced by d4d7945 ("Add DDT prune
command", #16277), first released in 2.3.0; before that the free
path selected the phys by DVA identity and was correct. So the hazard
is retroactive: traditional-format dedup gang blocks written safely
under 2.1/2.2 (or later, on pools without feature@fast_dedup) become
corruption sources the first time they are deleted under >= 2.3.0.
Exposed pools are those with a traditional-format DDT — dedup first
used while fast_dedup was not enabled (legacy pools that upgraded,
or compatibility= profiles) — that ever wrote a dedup'd block under
enough allocation pressure to gang. Flat (FDT) tables are not affected
by the slot-indexing defect.

A possibly related field report: #17297 — the same overlapping-segment
panic class on 2.3.1 with dedup + zstd + encryption + low free space
and repeated gang_header checksum ereports; no root cause was
identified there, and its DDT format was not reported, so no direct
link is established. I also hit an ms_defer overlapping-segment
panic at import on a pool with legacy-dedup history, which is what
prompted this investigation.

Defect 2: the DDT free pipeline discards gang stages.

zio_create() adds the gang stages to the pipeline of a logical free
of a gang BP:

zfs/module/zfs/zio.c

Lines 994 to 995 in 9bf75b4

	if (zio->io_child_type > ZIO_CHILD_GANG && BP_IS_GANG(bp))
	pipeline |= ZIO_GANG_STAGES;

but zio_free_bp_init() replaced the whole pipeline for dedup BPs,
discarding them:

https://github.com/openzfs/zfs/blob/9bf75b4b13949afe322387fc5572c5820ccL2141

On the legitimate pruned-entry fallback (zpool ddtprune), the plain
free that follows therefore releases only the gang header extents —
metaslab_free_dva() frees vdev_gang_header_asize() for a gang DVA
(module/zfs/metaslab.c#L5820-L5836)
— and every gang member leaks permanently. #17983 and #18520 fixed
adjacent problems in this fallback without covering the gang case.

Description

Two commits, independently backportable, each with its own regression
test:

1. **ddt: select traditional phys by block identity, not BP DVA count
   — ddt_entry_lookup_is_valid() now matches a phys by block identity
   (DVA[0] + physical birth) via ddt_phys_select(), the same
   predicate the decref path already uses (and what the existing
   XXX ... maybe can combine comment asked for). ddt_addref()
   selects the same way; both must change together because the addref
   mis-indexing is masked only by the broken lookup failing first, and
   the added VERIFY keeps DDT_PHYS_NONE from reaching
   ddt_phys_addref(), which would index out of bounds on release
   builds. A traditional-table miss on free — not expected on a
   consistent pool, since pruning only walks flat tables
   (module/zfs/ddt.c#L2841)
   — now leaves a zfs_dbgmsg trace instead of silently bypassing the
   refcount. Incidentally this also repairs frees of legacy
   DDT_PHYS_DITTO blocks (the old code could never select slot 0).
   Stale comments describing the trad slot as "the number of DVAs"
   (the mental model that produced the bug) are corrected.
2. zio: don't strip gang stages from the DDT free pipeline —
   zio_free_bp_init() now ORs ZIO_DDT_FREE_PIPELINE into the
   pipeline instead of assigning it, keeping the gang stages (and any
   other stage zio_create() added for this BP). Today this only adds
   ZIO_STAGE_DDT_FREE — zio_free_sync() already builds dedup frees
   with every other bit of that pipeline — but ORing the whole
   declaration keeps the site correct if the pipeline definition ever
   grows. The gang stages still run only on the true fall-through: a
   live DDT entry or a surviving BRT reference truncates the pipeline
   before the gang stages execute (since the restructuring in Fix double free for blocks cloned after DDT prune #18520;
   stage order DDT_FREE < BRT_FREE < GANG_ASSEMBLE/ISSUE < DVA_FREE), and the refcount-to-zero path is unaffected
   (ddt_phys_free() rebuilds a dedup-cleared BP whose free zio gets
   its own gang stages).

What this fix does not do. It is prevention only. Damage already
committed by the bug persists on disk and cannot be repaired in place:
leaked members (capacity only), committed spacemap double-frees (the
serious one — the allocator can hand doubly-freed regions to two
owners, and zfs_recover=1 merely warns and skips the overlapping
segment,
module/zfs/range_tree.c#L355-L366),
immortal stale entries (which keep matching future identical writes
even on fixed code — write-path lookups don't validate), and the
DDT/BRT clone hybrids. Pools with committed double-frees need
backup-and-rebuild; suggestions welcome on whether remediation guidance
belongs somewhere more durable than this PR text.

Auditing a pool:

• Table format: zdb -D <pool> prints version=0 [LEGACY] vs
  version=1 [FDT] per table (object names DDT-<cksum>-zap-* do not
  discriminate — FDT uses them too).
• Hard evidence, latent or triggered: zdb -DDDDD <pool> (five D's —
  at -DDDD the unique/refcnt-1 class is skipped, which is where
  never-deleted hazards live). Look for LEGACY-table entries whose
  phys slot contradicts the printed BP, e.g.
  refcnt 1 phys 1 ... gang ... double. Can be expensive on large
  DDTs.
• Corroborating: leaked space in zdb -bcc after last-reference
  deletes; gang_header checksum ereports; overlapping-segment panics.

Backports: the defective validation function is byte-identical in
every 2.3.x/2.4.x release; surrounding code differs slightly per
branch (2.3.0 and 2.3.1 have no verify lookup parameter — it arrives
in 2.3.2; the current 2.3-release tip predates master's phys-miss
fall-through), so backports need minor adaptation. The ddt_addref()
half predates 2.3.0 and exists in 2.2.x via block cloning, with no
verify-lookup masking it there; that branch deserves a separate audit.

How Has This Been Tested?

• Deterministic reproducer (forced ganging via the existing
  metaslab_force_ganging tunables on a feature@fast_dedup=disabled
  pool, userland libzpool): unpatched master leaks exactly 3,342,336
  bytes per run in zdb -bcc and retains stale DDT entries after all
  references are deleted; with this change the identical workload
  decrements refcounts correctly and repeated runs report no leak. The
  prune/gang case (defect 2) leaks 1,198,080 bytes unpatched and is
  clean with the fix.
• Randomized ztest: 15 runs with -o metaslab_force_ganging_pct=100 on the patched tree (ztest randomly enables dedup and creates legacy/FDT pools 50/50; four runs exercised traditional DDTs), all clean through ztest's built-in zdb -bccsv` verification.
• New ZTS coverage: dedup_legacy_gang (traditional DDT + forced
  ganging; three references — write, dedup copy, block clone; asserts
  the refcount of the copies=1 phys after every step, that the clone
  took a DDT reference and not BRT via bcloneused, survivor
  readability across an export/import, and a final empty-DDT +
  zdb -bcc gate) and a forced-ganging second phase in
  dedup_prune_leak (asserts the entries are ganged and actually
  pruned, then verifies the members are freed). dedup_legacy_gang
  takes its clone reference with FICLONE, so it is registered
  Linux-only alongside the other FICLONE tests. Both follow existing
  sibling-test idioms; I have not yet run the full suite against a
  live kernel with this change, so CI is the first executor — flagging
  that honestly.
• Userland build clean (-Werror, debug), make checkstyle clean
  (cstyle, shellcheck, commitcheck), git diff --check clean.

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Quality assurance (non-breaking change which makes the code more robust against bugs)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[Zaczero]

@behlendorf this is the bug I noticed when working on that recent panic, and this one was likely the cause of the pool corruption I mentioned in #18811 discussion.

My quick AI analysis says we can fix it proactively, before they explode, by expanding the scrub, but I haven't dug into it; just pasting as-is:

The cheap one: a DDT validation pass, wired into scrub's existing DDT walk. Here's the underexploited fact: scrub already walks the DDT and reads every entry's blocks (dsl_scan_ddt — it scrubs dedup'd blocks once per entry instead of once per reference). And DDT entries are self-validating: the entry is a checksum, so reading the phys's DVAs and comparing against the stored key tells you definitively whether the phys still points at its data. A stale entry — our immortal landmine — fails that check by construction. So the detection half is nearly free: it's a natural extension of a pass that already exists and already does the I/O.

The repair half is where the trap lives. The obvious repair — delete the stale entry — is wrong: any still-live BPs referencing it would then miss on free, take the pruned-entry fallback, and physically free already-freed space. You'd have rebuilt the double-free machine in the name of healing. The safe repair is the opposite: quarantine, don't delete — flag the entry so it never matches a new write (stops the future-write corruption) and never frees its extents (its refcount is inflated anyway and can't legitimately reach zero). That converts an active corruption source into a bounded, harmless leak — polarity-safe by design, because a leak is the one damage class that can't hurt you. That's small enough to be a realistic follow-up PR: a flag bit, a check in the write-path lookup and in ddt_phys_free, and the scrub hook.

[Zaczero]

flaky CI fix pending in #18820

[amotin]

Thanks. Those are interesting.

[amotin]

Thank you.