drivers/gpu/drm/display/drm_dp_mst_topology.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drm_dp_sideband_parse_remote_dpcd_read() reads num_bytes from the raw
message and then unconditionally does:
memcpy(bytes, &raw->msg[idx], num_bytes);
without checking that idx + num_bytes <= raw->curlen. raw->msg[] is
256 bytes; if a malicious or misbehaving MST hub sets num_bytes larger
than the remaining payload, the memcpy reads past the received data
into whatever follows in raw->msg[].
drm_dp_sideband_parse_remote_i2c_read_ack() has the same flaw (noted
with a /* TODO check */ comment since the code was introduced).
Fix both functions by using a single combined check
(idx + num_bytes > curlen) before each memcpy. Since num_bytes is u8,
it is always >= 0, so this strictly subsumes the simpler idx > curlen
form and no separate step is needed.
Cc: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
Changes in v2:
- Drop separate idx > curlen check; idx + num_bytes > curlen with u8
num_bytes (always >= 0) strictly subsumes it (Lyude Paul)
drivers/gpu/drm/display/drm_dp_mst_topology.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 170113520a43..9416a48804c8 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -871,7 +871,7 @@ static bool drm_dp_sideband_parse_remote_dpcd_read(struct drm_dp_sideband_msg_rx
goto fail_len;
repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx];
idx++;
- if (idx > raw->curlen)
+ if (idx + repmsg->u.remote_dpcd_read_ack.num_bytes > raw->curlen)
goto fail_len;
memcpy(repmsg->u.remote_dpcd_read_ack.bytes, &raw->msg[idx], repmsg->u.remote_dpcd_read_ack.num_bytes);
@@ -907,7 +907,9 @@ static bool drm_dp_sideband_parse_remote_i2c_read_ack(struct drm_dp_sideband_msg
goto fail_len;
repmsg->u.remote_i2c_read_ack.num_bytes = raw->msg[idx];
idx++;
- /* TODO check */
+ if (idx + repmsg->u.remote_i2c_read_ack.num_bytes > raw->curlen)
+ goto fail_len;
+
memcpy(repmsg->u.remote_i2c_read_ack.bytes, &raw->msg[idx], repmsg->u.remote_i2c_read_ack.num_bytes);
return true;
fail_len:
--
2.34.1
Reviewed-by: Lyude Paul <lyude@redhat.com> Will push to drm-misc in just a moment On Sun, 2026-05-10 at 20:17 +0000, Ashutosh Desai wrote: > drm_dp_sideband_parse_remote_dpcd_read() reads num_bytes from the raw > message and then unconditionally does: > > memcpy(bytes, &raw->msg[idx], num_bytes); > > without checking that idx + num_bytes <= raw->curlen. raw->msg[] is > 256 bytes; if a malicious or misbehaving MST hub sets num_bytes > larger > than the remaining payload, the memcpy reads past the received data > into whatever follows in raw->msg[]. > > drm_dp_sideband_parse_remote_i2c_read_ack() has the same flaw (noted > with a /* TODO check */ comment since the code was introduced). > > Fix both functions by using a single combined check > (idx + num_bytes > curlen) before each memcpy. Since num_bytes is u8, > it is always >= 0, so this strictly subsumes the simpler idx > curlen > form and no separate step is needed. > > Cc: stable@vger.kernel.org > Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com> > --- > Changes in v2: > - Drop separate idx > curlen check; idx + num_bytes > curlen with u8 > num_bytes (always >= 0) strictly subsumes it (Lyude Paul) > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c > b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 170113520a43..9416a48804c8 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -871,7 +871,7 @@ static bool > drm_dp_sideband_parse_remote_dpcd_read(struct drm_dp_sideband_msg_rx > goto fail_len; > repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx]; > idx++; > - if (idx > raw->curlen) > + if (idx + repmsg->u.remote_dpcd_read_ack.num_bytes > raw- > >curlen) > goto fail_len; > > memcpy(repmsg->u.remote_dpcd_read_ack.bytes, &raw->msg[idx], > repmsg->u.remote_dpcd_read_ack.num_bytes); > @@ -907,7 +907,9 @@ static bool > drm_dp_sideband_parse_remote_i2c_read_ack(struct drm_dp_sideband_msg > goto fail_len; > repmsg->u.remote_i2c_read_ack.num_bytes = raw->msg[idx]; > idx++; > - /* TODO check */ > + if (idx + repmsg->u.remote_i2c_read_ack.num_bytes > raw- > >curlen) > + goto fail_len; > + > memcpy(repmsg->u.remote_i2c_read_ack.bytes, &raw->msg[idx], > repmsg->u.remote_i2c_read_ack.num_bytes); > return true; > fail_len:
© 2016 - 2026 Red Hat, Inc.