DF-5088 — Generic i2c HW-engine chunker boundary defects (latent, unit is dead code in this port)

File: sys/dev/drm/amd/display/dc/i2caux/i2c_generic_hw_engine.c (pass 2)

What

Three boundary/logic defects in dal_i2c_generic_hw_engine_submit_request() / dal_i2c_generic_hw_engine_get_transaction_timeout(), all latent because the entire unit is dead code in this DragonFly port (proven below):

1. max_payload_size == 1 degenerate chunking (lines 150-158). With the HW buffer down to its last free byte, the first sub-transaction computes current_transaction_size = 1, current_payload_size = 0 — an address-only transaction that transfers nothing — and every subsequent iteration takes the non-first branch (162-167) streaming 1-byte data-only transactions with start_bit = 0, i.e. bytes clocked to an unre-addressed slave. Boundary sibling of DF-2090 (which covers max_payload_size == 0 → max_payload_size - 1 underflow).
2. Zero-length payload rejected as FAILED_INCOMPLETE (line 138 skips the loop; operation_result keeps its initializer I2C_CHANNEL_OPERATION_FAILED at 103-104; switch maps it to FAILED_INCOMPLETE at 240-243). A zero-length I2C transaction (address probe / quick transfer) is legitimate and is handled fine by the live base path (dal_i2c_hw_engine_submit_request, i2c_hw_engine.c:91-120, where 0 >= available is false and the request is submitted). If this unit is ever wired, DDC probing through it silently degrades.
3. Timeout omits in-batch clock stretches (269-270). The generic get_transaction_timeout computes (1000*32/speed) * (2 + 8*len) but, unlike both tuned implementations it would serve (i2c_hw_engine_dce80.c: 755-761 and i2c_hw_engine_dce110.c:455-465), it omits the buffer_used_bytes*8 + transaction_count*2 stretch correction for data already queued in the shared HW FIFO — mid-batch chunks get a too-short timeout and can abort spuriously.

Dead-code proof (why Info / latent)

• i2caux->i2c_generic_hw_engine is only ever assigned NULL (sys/dev/drm/amd/display/dc/i2caux/i2caux.c:447); acquire_i2c_hw_engine in i2caux_dce80.c:130-136 therefore always returns NULL for it.
• No file instantiates struct i2c_generic_hw_engine_funcs or calls dal_i2c_generic_hw_engine_construct() — grep -rn dal_i2c_generic_hw_engine over sys/dev/drm/amd/ returns only the unit's own .c/.h.
• Both HW-engine families wire the base submit instead: dal_i2c_hw_engine_submit_request at i2c_hw_engine_dce80.c:815 and i2c_hw_engine_dce110.c:498.
• The unit is compiled into the dc module (sys/dev/drm/amd/amdgpu/Makefile line "SRCS+= ... i2c_generic_hw_engine.c ..."), so the code exists in the kernel image; it is simply unreachable.

Reproduce

Not reproducible at runtime on this tree: the code path is unreachable by construction (no consumer), and the verification guest has no AMD display hardware (QEMU std VGA, no /dev/dri, no drm modules — see env.txt/run.log). The defect claims are line-level source facts (VERDICT.md).

Fix

Either delete the dead unit or harden the boundary math before any future wiring — see fix.diff (reject max_payload_size < 2 with BUFFER_OVERFLOW; add the in-batch stretch correction to the timeout).

DF-5088 VERDICT — latent chunker boundary defects in a proven-dead unit

Status: untested (unreachable code — no runtime trigger exists on any build of this tree; guest also lacks the hardware). Impact: none today. Confidence: certain (every claim is a line-level source fact, cited below).

1. The unit is dead code in this port (stronger than DF-2090's note)

DF-2090 (pass 1) already described the max_payload_size - 1 underflow as "dead code today". Pass 2 confirms and strengthens: the whole unit is dead, not just that expression:

• dal_i2c_generic_hw_engine_submit_request, dal_i2c_generic_hw_engine_get_transaction_timeout, dal_i2c_generic_hw_engine_construct/destruct have zero callers outside the unit itself (grep -rn 'dal_i2c_generic_hw_engine' sys/dev/drm/amd/ matches only i2c_generic_hw_engine.c / .h).
• No engine ever instantiates struct i2c_generic_hw_engine_funcs (i2c_generic_hw_engine.h:39-55) — the write_address/write_data/read_data/ execute_transaction vtable has no implementation anywhere in the tree.
• i2caux->i2c_generic_hw_engine (i2caux.h:89) is assigned only NULL at i2caux.c:447; acquire_i2c_hw_engine() (i2caux_dce80.c:109-150) reads it at :132 only when no per-line engine matched and then bails at :135-136 (if (!engine) return NULL).
• The two real HW engines route .submit_request to the base implementation: i2c_hw_engine_dce80.c:815 and dce110/i2c_hw_engine_dce110.c:498 both use dal_i2c_hw_engine_submit_request (i2c_hw_engine.c:72).
• Yet the file is compiled in: sys/dev/drm/amd/amdgpu/Makefile lists i2c_generic_hw_engine.c in the dc/i2caux SRCS block.

Consequence: nothing in this file can be triggered by any input, privileged or not. (In upstream Linux amdgpu/dc the generic engine is the live engine for dce100+; if a future port wires it, the latent defects below activate.)

2. Defect (a) — max_payload_size == 1 degenerates into unaddressed 1-byte writes

dal_i2c_generic_hw_engine_submit_request chunks the payload against max_payload_size = base->funcs->get_hw_buffer_available_size(base) (i2c_generic_hw_engine.c:96-97). For the HW engines that exist in this tree that accessor is I2C_HW_BUFFER_SIZE - buffer_used_bytes = 144 - used (i2c_hw_engine_dce80.c:736-741, same in dce110:441-445).

First iteration (150-158):

current_transaction_size = (remaining_payload_size > max_payload_size - 1)
    ? max_payload_size : remaining_payload_size + 1;
current_payload_size = current_transaction_size - 1;

With max_payload_size == 1 (143 bytes already queued mid-batch): remaining > 0 is true → current_transaction_size = 1, current_payload_size = 0. Iteration 1 then writes only the slave address (:177-178) and executes a 1-byte, zero-payload transaction (:195). Progress is made (remaining -= 0 is skipped — no; remaining_payload_size -= 0 at :221 means iteration 1 makes zero progress), and from iteration 2 on the non-first branch (:159-168) computes current_transaction_size = 1, current_payload_size = 1 — but first_iteration is now false so write_address is never called again and attributes.start_bit = 0 (:190): every remaining byte is clocked as a standalone 1-byte transaction without the slave address. On a real bus this misaddresses the transfer; the loop also spins the HW engine once extra per byte. No kernel memory is corrupted (payload pointers/sizes stay within the caller's buffer), so even if wired this is a protocol-correctness/robustness defect, not memory unsafety.

Boundary family of DF-2090 (max == 0 underflow, known, not re-reported here).

3. Defect (b) — zero-length payload is reported FAILED_INCOMPLETE

operation_result is initialized to I2C_CHANNEL_OPERATION_FAILED (:103-104). For payload.length == 0 (a valid I2C quick/address-probe transaction) the while (remaining_payload_size) loop (:138) never runs, and the switch (:240-243) maps FAILED → I2CAUX_TRANSACTION_STATUS_FAILED_INCOMPLETE and returns false. The live base path handles length 0 correctly (i2c_hw_engine.c:91-96 passes the >= check; :113-123 submits the address-only request). Latent logic defect only.

4. Defect (c) — timeout omits in-batch clock stretches

dal_i2c_generic_hw_engine_get_transaction_timeout (:256-271) computes

return ((1000 * TRANSACTION_TIMEOUT_IN_I2C_CLOCKS) / speed) * (1 + (length << 3) + 1);

with TRANSACTION_TIMEOUT_IN_I2C_CLOCKS = 32 (i2c_hw_engine.h:30). Both real engines add the stretch accounting for bytes already queued in the shared DC_I2C FIFO before this sub-transaction: i2c_hw_engine_dce80.c:757-761 and i2c_hw_engine_dce110.c:462-465 add (buffer_used_bytes << 3) + (transaction_count << 1). The generic version's omission makes mid-batch sub-transaction timeouts too short → spurious TIMEOUT aborts of in-flight batches. (No u32 overflow: length ≤ 145, speed ≥ 1 ⇒ ≤ 32000 * 1154 ≈ 3.7e7.)

5. Classes considered and killed (pass-2 negative notes for this file)

• Infinite loop / no-progress chunking: only for max_payload_size == 0 (DF-2090, known). For max ≥ 2 every iteration advances remaining_payload_size by ≥ 1 (:220-221). For max == 1 iteration 1 stalls once but iteration 2+ progresses (defect (a) above).
• transaction-array indexing / count wrap: none exists in this file — the transaction_addr[4] array and the transaction_count == 3 clamp live in the dce80/dce110 engines (i2c_hw_engine_dce80.c:397-402, 436, 492-505, bounded: last_transaction forced at count 3 before any 4th index use).
• Kernel-memory OOB via write_data/read_data: chunk sizes are computed from remaining_payload_size which equals the caller's buffer size (payload struct: engine.h:41-46, u32 length + kernel buffer), so pointers stay in-bounds even in the DF-2090 underflow case; the overflow victim is the 144-byte DC_I2C MMIO FIFO, not kernel memory.
• set_speed/get_speed division by zero: this file's only division is guarded (if (!speed) return 0; :264-265); the engines' set_speed guards if (speed) (i2c_hw_engine_dce80.c:327).
• Refcount/lifetime: construct/destruct are trivial pass-throughs (:273-284); no allocation happens in this unit.

6. Why untested (honest status)

• The unit has no caller in this tree: no syscall/ioctl path reaches it, so no PoC can trigger it even as root.
• The verification guest additionally has no AMD display hardware (env.txt: QEMU std VGA 0x1234:1111, zero drm modules, no /dev/dri).
• Per contract, Info hardening findings are PoC-skippable; recorded as untested with this narrative as the evidence.

7. Fix

fix.diff (git-apply clean, self-contained) rejects max_payload_size < 2 with FAILED_BUFFER_OVERFLOW — this also closes the DF-2090 max==0 underflow at the same boundary. The zero-length behavior is left unchanged (its correct semantics depend on the caller contract if the unit is ever revived), and the timeout stretch term of §4 cannot be added without first extending struct i2c_generic_hw_engine with buffer_used_bytes/transaction_count (they live in the per-ASIC dce80/dce110 structs today) — documented as the follow-up; the alternative fix is deleting the dead unit outright.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.