drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
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 and also
lacked the post-increment idx bounds check (noted with a /* TODO check */
comment since the code was introduced).
Add the missing bounds checks in both functions, consistent with the
pattern used throughout the rest of the sideband parser.
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 170113520..9cef6c576 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -873,6 +873,8 @@ static bool drm_dp_sideband_parse_remote_dpcd_read(struct drm_dp_sideband_msg_rx
idx++;
if (idx > raw->curlen)
goto fail_len;
+ 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);
return true;
@@ -907,7 +909,11 @@ 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 > raw->curlen)
+ goto fail_len;
+ 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
Hi - one comment down below: On Fri, 2026-04-10 at 03:41 +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 and > also > lacked the post-increment idx bounds check (noted with a /* TODO > check */ > comment since the code was introduced). > > Add the missing bounds checks in both functions, consistent with the > pattern used throughout the rest of the sideband parser. > > Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com> > --- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c > b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 170113520..9cef6c576 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -873,6 +873,8 @@ static bool > drm_dp_sideband_parse_remote_dpcd_read(struct drm_dp_sideband_msg_rx > idx++; > if (idx > raw->curlen) > goto fail_len; > + if (idx + repmsg->u.remote_dpcd_read_ack.num_bytes > raw- > >curlen) > + goto fail_len; Do we need both checks here? It seems like we could just replace the first check with the second check. > > memcpy(repmsg->u.remote_dpcd_read_ack.bytes, &raw->msg[idx], > repmsg->u.remote_dpcd_read_ack.num_bytes); > return true; > @@ -907,7 +909,11 @@ 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 > raw->curlen) > + goto fail_len; > + if (idx + repmsg->u.remote_i2c_read_ack.num_bytes > raw- > >curlen) > + goto fail_len; Same here > + > 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.