Re: [PATCH] drm/amd/display: fix division by zero in get_estimated_bw()

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uncapped request_bw is assigned to allocated_bw, causing an oversized value that leads to an integer underflow during bandwidth validation.
--

--- Patch [1]: [PATCH] drm/amd/display: fix division by zero in get_estimated_bw() ---
commit d3870b3b2da2897f934f4033da62b969b6443914
Author: Hari Mishal <[email protected]>

drm/amd/display: fix division by zero in get_estimated_bw()

This commit fixes a division by zero in get_estimated_bw() by returning 0
if bw_granularity is zero. This prevents a crash when a connected DPIA
device reports an estimated bandwidth change before reporting a capability
change.

>     link_dpia_send_bw_alloc_request() already guards the same
>     bw_granularity division; add the identical guard here rather than
>     introducing a new pattern.

[Severity: High]
This is a pre-existing issue, but since the commit message references 
link_dpia_send_bw_alloc_request(), I noticed a potential integer underflow
issue involving allocated_bw.

In link_dpia_send_bw_alloc_request(), the uncapped request_bw variable
is assigned to allocated_bw:

drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c:link_dpia_send_bw_alloc_request() {
    ...
		temp = req_bw * link->dpia_bw_alloc_config.bw_granularity;
		request_reg_val = temp / Kbps_TO_Gbps;
		if (temp % Kbps_TO_Gbps)
			++request_reg_val;
	}

	link->dpia_bw_alloc_config.allocated_bw = request_bw;
    ...
}

Later, in link_dpia_validate_dp_tunnel_bandwidth(), remaining_bw is
calculated using unsigned 32-bit subtraction:

drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c:link_dpia_validate_dp_tunnel_bandwidth() {
    ...
			if (router_sets[j].cm_id == dp_tunnel_settings->cm_id) {
				uint32_t remaining_bw =
					dp_tunnel_settings->estimated_bw - dp_tunnel_settings->allocated_bw;
    ...
}

If a connected DPIA display requests more bandwidth than estimated,
allocated_bw will be strictly greater than estimated_bw.

Could this cause the subtraction to underflow to an extremely large
positive value (~4.29 billion), thereby bypassing the subsequent
capacity checks (required_bw > total_bw) and allowing illegal
configurations that oversubscribe the DP tunnel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.