mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] drm/dp/mst: fix OOB reads on 2-byte fields in sideband reply parsers
@ 2026-05-10 20:31 Ashutosh Desai
  2026-05-18 17:59 ` lyude
  0 siblings, 1 reply; 2+ messages in thread
From: Ashutosh Desai @ 2026-05-10 20:31 UTC (permalink / raw)
  To: dri-devel
  Cc: stable, lyude, airlied, daniel, maarten.lankhorst, mripard,
	tzimmermann, simona, linux-kernel, Ashutosh Desai

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: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] drm/dp/mst: fix OOB reads on 2-byte fields in sideband reply parsers
  2026-05-10 20:31 [PATCH v2] drm/dp/mst: fix OOB reads on 2-byte fields in sideband reply parsers Ashutosh Desai
@ 2026-05-18 17:59 ` lyude
  0 siblings, 0 replies; 2+ messages in thread
From: lyude @ 2026-05-18 17:59 UTC (permalink / raw)
  To: Ashutosh Desai, dri-devel
  Cc: stable, airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	simona, linux-kernel

Reviewed-by: Lyude Paul <lyude@redhat.com>

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: stable@vger.kernel.org
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---
> 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);


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-18 17:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-10 20:31 [PATCH v2] drm/dp/mst: fix OOB reads on 2-byte fields in sideband reply parsers Ashutosh Desai
2026-05-18 17:59 ` lyude

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®