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);
-- 
2.34.1

Reply via email to