Re: [PATCH] drm/dp/mst: fix buffer overflows in sideband chunk accumulation

[lyude@redhat.com]

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

Will push to drm-misc in a moment

On Fri, 2026-04-10 at 04:19 +0000, Ashutosh Desai wrote:
> drm_dp_sideband_append_payload() has three related bugs when
> processing
> device-provided sideband reply data:
> 
> 1. Zero-length curchunk_len underflow: msg_len is a 6-bit field taken
>    directly from the DP sideband header. If a device sends msg_len=0,
>    curchunk_len is set to zero. The condition (curchunk_idx >=
> curchunk_len)
>    is immediately true, and curchunk_len-1 wraps to 255 (u8
> underflow).
>    drm_dp_msg_data_crc4() reads 255 bytes from chunk[48], then
> memcpy()
>    writes 255 bytes into msg[], both far out of bounds.
> 
> 2. chunk[48] overflow: curchunk_len can reach 63 (6-bit field).
> chunk[] is
>    only 48 bytes. Multi-iteration payload assembly appends 16-byte
> blocks
>    until curchunk_idx reaches curchunk_len, writing up to 15 bytes
> past
>    the end of chunk[] into msg[].
> 
> 3. msg[256] overflow: each chunk contributes (curchunk_len-1) bytes
> to
>    msg[]. No check ensures curlen + (curchunk_len-1) stays within
> msg[256],
>    so the memcpy can spill into adjacent struct fields.
> 
> All three are reachable from any DP MST device that can forge
> sideband
> reply messages on a physical connection.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index f2a7dbc5e..5261a4a54 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -789,6 +789,12 @@ static bool
> drm_dp_sideband_append_payload(struct drm_dp_sideband_msg_rx *msg,
>  {
>  	u8 crc4;
>  
> +	/* curchunk_len must be >= 1 (min 1 CRC byte) and fit in
> chunk[] */
> +	if (!msg->curchunk_len ||
> +	    msg->curchunk_len > ARRAY_SIZE(msg->chunk) ||
> +	    msg->curchunk_idx + replybuflen > ARRAY_SIZE(msg-
> >chunk))
> +		return false;
> +
>  	memcpy(&msg->chunk[msg->curchunk_idx], replybuf,
> replybuflen);
>  	msg->curchunk_idx += replybuflen;
>  
> @@ -799,6 +805,9 @@ static bool drm_dp_sideband_append_payload(struct
> drm_dp_sideband_msg_rx *msg,
>  			print_hex_dump(KERN_DEBUG, "wrong crc",
>  				       DUMP_PREFIX_NONE, 16, 1,
>  				       msg->chunk,  msg-
> >curchunk_len, false);
> +		/* Guard against accumulated msg[] overflow */
> +		if (msg->curlen + msg->curchunk_len - 1 >
> ARRAY_SIZE(msg->msg))
> +			return false;
>  		/* copy chunk into bigger msg */
>  		memcpy(&msg->msg[msg->curlen], msg->chunk, msg-
> >curchunk_len - 1);
>  		msg->curlen += msg->curchunk_len - 1;