From: Harry Wentland <[email protected]>
[Why]
amdgpu_dm_process_dmub_aux_transfer_sync() copies p_notify->aux_reply.length
bytes into payload->data without clamping. payload->data is typically a 16-byte
DPCD scratch buffer, while aux_reply.length is echoed from the sink via the DMUB
ring. While this is clamped by DMUB it's prudent to ensure we validate
this in the driver as well.
[How]
Clamp the copy to sizeof(aux_reply.data), the scratch buffer the reply was read
into, and use that for both the memcpy and the return value. For regular
transfers additionally clamp to payload->length to cover callers whose
destination buffer is smaller than 16 bytes. The write-status-update retry path
(dce_aux_transfer_with_retries) deliberately zeroes payload->length while still
expecting the partial-write status byte, so that bound is skipped in that case
to avoid dropping the reply. Also guard against a NULL payload->data.
Fixes: 81927e2808be ("drm/amd/display: Support for DMUB AUX")
Cc: [email protected]
Assisted-by: Copilot:claude-opus-4.8
Reviewed-by: Alex Hung <[email protected]>
Signed-off-by: Harry Wentland <[email protected]>
Signed-off-by: George Zhang <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_dmub.c | 24 +++++++++++++++----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
index 97cb2a09153d..1f9830ce2ea6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_dmub.c
@@ -798,12 +798,26 @@ int amdgpu_dm_process_dmub_aux_transfer_sync(
payload->reply[0] = (adev->dm.dmub_notify->aux_reply.command >>
4) & 0xF;
/*write req may receive a byte indicating partially written number as
well*/
- if (p_notify->aux_reply.length)
- memcpy(payload->data, p_notify->aux_reply.data,
- p_notify->aux_reply.length);
+ if (p_notify->aux_reply.length && payload->data) {
+ /* Bound the reply to the scratch buffer it was read into. */
+ ret = min((uint32_t)p_notify->aux_reply.length,
+ (uint32_t)sizeof(p_notify->aux_reply.data));
+
+ /*
+ * During a write-status-update retry the caller zeroes
+ * payload->length while still expecting the partial-write
+ * status byte in payload->data (see
dce_aux_transfer_with_retries),
+ * so only clamp to payload->length for regular transfers.
+ */
+ if (!payload->write_status_update)
+ ret = min(ret, payload->length);
+
+ memcpy(payload->data, p_notify->aux_reply.data, ret);
+ } else {
+ /* success */
+ ret = p_notify->aux_reply.length;
+ }
- /* success */
- ret = p_notify->aux_reply.length;
*operation_result = p_notify->result;
out:
reinit_completion(&adev->dm.dmub_aux_transfer_done);
--
2.53.0
