Reviewed-by: Lyude Paul <[email protected]> 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: [email protected] > Signed-off-by: Ashutosh Desai <[email protected]> > --- > 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:
