Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Buffer truncation and USB Babble errors due to undersized buffer allocation in `recv_hdcp` and `pace_cap_ack`. - [High] Premature pacing abort on short frames in `pace_cap_ack`. - [Low] Mismatch between `crypto_selftest` docstring and actual implemented test coverage. --
commit 102ea10e819242d5dcf97bac1a8fb013c88b76f5 Author: Mike Lothian <[email protected]> Subject: drm/vino: add the clean-room HDCP 2.2 AKE/LC/SKE After the plaintext session init, the DL3 dock requires an HDCP 2.2 session before it will accept any control-plane traffic. Add a clean-room implementation of the HDCP 2.2 authentication. > diff --git a/drivers/gpu/drm/vino/vino.rs b/drivers/gpu/drm/vino/vino.rs > index 79f446041b648..db4c38b6dc92f 100644 > --- a/drivers/gpu/drm/vino/vino.rs > +++ b/drivers/gpu/drm/vino/vino.rs [ ... ] > @@ -150,6 +150,44 @@ impl WorkItem for BringUp { > /// On-device crypto known-answer self-test. Confirms the IN-KERNEL crypto > path (which the CP seal > /// depends on) is byte-correct -- something only ever checked offline > (Python `verify-kdf.py`) > /// before. > /// Runs three checks and logs PASS/FAIL: > /// 1. AES-128-ECB vs the FIPS-197 test vector. > /// 2. AES-CMAC vs the RFC 4493 test vector (subkey + full-block path). > /// 3. The full `cp::seal_livemac` vs cold-ref's REAL msg0: known > plaintext + known `ks`/`riv` [Severity: Low] This isn't a bug, but the docstring mentions a third test for cp::seal_livemac that doesn't seem to be implemented in the function body. Could the comment be updated to reflect the actual tests? [ ... ] > + fn recv_hdcp(dev: &usb::Device) -> Result<(u8, KVec<u8>)> { > + const SUB_HDCP_RESP: u16 = 0x25; > + let mut buf = KVec::from_elem(0u8, 4096, GFP_KERNEL)?; [Severity: High] Does this buffer allocation correctly size the read for the endpoint? The dock's interleaved capability blocks can reach ~5.8 KiB. Because this buffer is only 4096 bytes, if the dock pushes a large capability block during the AKE sequence, it looks like it will result in a USB truncation error (-EOVERFLOW). Using the EP84_BUF constant (16384 bytes) here, similar to wait_cap_complete(), might prevent this. [ ... ] > + fn pace_cap_ack(dev: &usb::Device, want_ctr: u16) { > + let Ok(mut buf) = KVec::from_elem(0u8, 4096, GFP_KERNEL) else { > + return; > + }; [Severity: High] Can this buffer suffer from the same truncation issue as recv_hdcp()? If the capability block exceeds 4096 bytes, this looks like it could also trigger an -EOVERFLOW error. [ ... ] > + for _ in 0..8 { > + match dev.bulk_recv(EP_CTRL_IN, &mut buf, > Delta::from_millis(30)) { > + Ok(len) if len >= 22 => { > + let wsub = u16::from_le_bytes([buf[8], buf[9]]); > + let iid = u16::from_le_bytes([buf[16], buf[17]]); > + let ictr = u16::from_le_bytes([buf[20], buf[21]]); > + // The per-frame cap-ack: wsub=0x25, inner id=0x14 > sub=0x10 ctr=want. > + // An interleaved cap push (sub=0x84) or earlier ack -- > keep reading. > + if wsub == 0x25 && iid == 0x14 && ictr == want_ctr { > + return; > + } > + } > + // Nothing queued within the short window -- the dock is > idle, don't block. > + _ => return, [Severity: High] Will this catch-all arm prematurely abort the loop on short frames? If the dock sends short interleaved status or ACK frames (where len is less than 22), the match falls through to the catch-all arm instead of continuing to wait for the target ACK. This seems like it would cause the driver to transmit the next message while the dock is mid-NAK, which could trigger a NAK storm and prevent control-plane engagement. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
