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 <[email protected]> > --- > 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:
