Reviewed-by: Lyude Paul <[email protected]> Will push to drm-misc-next in just a moment
On Sun, 2026-05-10 at 20:31 +0000, Ashutosh Desai wrote: > Three sideband reply parsers read 16-bit fields as: > > val = (raw->msg[idx] << 8) | (raw->msg[idx+1]); > > and check bounds only after the fact. When idx == raw->curlen, > raw->msg[idx+1] reads one byte past the received message data into > the following struct fields (curchunk_len, curchunk_idx, curlen). > > Affected functions: > - drm_dp_sideband_parse_enum_path_resources_ack() > full_payload_bw_number and avail_payload_bw_number fields > - drm_dp_sideband_parse_allocate_payload_ack() > allocated_pbn field > - drm_dp_sideband_parse_query_payload_ack() > allocated_pbn field > > Fix by using a single combined check (idx + 2 > curlen) before each > 2-byte read. Since the check is strictly tighter than idx > curlen, > no separate step is needed. > > Cc: [email protected] > Signed-off-by: Ashutosh Desai <[email protected]> > --- > Changes in v2: > - Drop separate idx > curlen check immediately before idx + 2 > > curlen; > the combined check strictly subsumes it (Lyude Paul) > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c > b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 9416a48804c8..6e7896193772 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -925,16 +925,13 @@ static bool > drm_dp_sideband_parse_enum_path_resources_ack(struct drm_dp_sideband > repmsg->u.path_resources.port_number = (raw->msg[idx] >> 4) > & 0xf; > repmsg->u.path_resources.fec_capable = raw->msg[idx] & 0x1; > idx++; > - if (idx > raw->curlen) > + if (idx + 2 > raw->curlen) > goto fail_len; > repmsg->u.path_resources.full_payload_bw_number = (raw- > >msg[idx] << 8) | (raw->msg[idx+1]); > idx += 2; > - if (idx > raw->curlen) > + if (idx + 2 > raw->curlen) > goto fail_len; > repmsg->u.path_resources.avail_payload_bw_number = (raw- > >msg[idx] << 8) | (raw->msg[idx+1]); > - idx += 2; > - if (idx > raw->curlen) > - goto fail_len; > return true; > fail_len: > DRM_DEBUG_KMS("enum resource parse length fail %d %d\n", > idx, raw->curlen); > @@ -952,12 +949,9 @@ static bool > drm_dp_sideband_parse_allocate_payload_ack(struct drm_dp_sideband_ms > goto fail_len; > repmsg->u.allocate_payload.vcpi = raw->msg[idx]; > idx++; > - if (idx > raw->curlen) > + if (idx + 2 > raw->curlen) > goto fail_len; > repmsg->u.allocate_payload.allocated_pbn = (raw->msg[idx] << > 8) | (raw->msg[idx+1]); > - idx += 2; > - if (idx > raw->curlen) > - goto fail_len; > return true; > fail_len: > DRM_DEBUG_KMS("allocate payload parse length fail %d %d\n", > idx, raw->curlen); > @@ -971,12 +965,9 @@ static bool > drm_dp_sideband_parse_query_payload_ack(struct drm_dp_sideband_msg_r > > repmsg->u.query_payload.port_number = (raw->msg[idx] >> 4) & > 0xf; > idx++; > - if (idx > raw->curlen) > + if (idx + 2 > raw->curlen) > goto fail_len; > repmsg->u.query_payload.allocated_pbn = (raw->msg[idx] << 8) > | (raw->msg[idx + 1]); > - idx += 2; > - if (idx > raw->curlen) > - goto fail_len; > return true; > fail_len: > DRM_DEBUG_KMS("query payload parse length fail %d %d\n", > idx, raw->curlen);
