Re: [PATCH v9 09/22] drm/bridge: synopsys: dw-dp: Free output_fmts when none are valid

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The DRM core's bridge bus format negotiation protocol contains a memory leak when a driver returns an allocated format array but 0 valid formats. This problem wasn't introduced by this patch, but this patch adds a driver-local workaround instead of fixing the core API.
--

--- Patch [9]: [PATCH v9 09/22] drm/bridge: synopsys: dw-dp: Free output_fmts when none are valid ---
commit 91ba4a99e0fef78bbe1a7632011fef0a483a9e57
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

drm/bridge: synopsys: dw-dp: Free output_fmts when none are valid

This commit modifies dw_dp_bridge_atomic_get_output_bus_fmts() to free
the allocated output_fmts array if the number of valid formats is 0,
avoiding a memory leak.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 22a244de08aa1..bde938b0d116d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1813,6 +1813,11 @@ static u32 *dw_dp_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>  		output_fmts[j++] = fmt->bus_format;
>  	}
>  
> +	if (j == 0) {
> +		kfree(output_fmts);
> +		output_fmts = NULL;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but should this memory leak be
fixed in the DRM core instead of adding a driver-local workaround?

Looking at the DRM core's bridge bus format negotiation protocol in
drivers/gpu/drm/drm_bridge.c, the core leaks the array when a driver returns
an allocated format array but 0 valid formats:

drivers/gpu/drm/drm_bridge.c:drm_atomic_bridge_chain_select_bus_fmts() {
    ...
		out_bus_fmts = funcs->atomic_get_output_bus_fmts(last_bridge,
							last_bridge_state,
							crtc_state,
							conn_state,
							&num_out_bus_fmts);
		if (!num_out_bus_fmts)
			return -ENOTSUPP;
    ...
}

The core returns an error immediately without calling kfree(out_bus_fmts),
silently leaking the dynamically allocated array.

The same issue appears in select_bus_fmt_recursive() for
atomic_get_input_bus_fmts:

drivers/gpu/drm/drm_bridge.c:select_bus_fmt_recursive() {
    ...
	in_bus_fmts = cur_bridge->funcs->atomic_get_input_bus_fmts(cur_bridge,
							cur_state,
							crtc_state,
							conn_state,
							out_bus_fmt,
							&num_in_bus_fmts);
	if (!num_in_bus_fmts)
		return -ENOTSUPP;
    ...
}

Would it be better to fix the core API to handle freeing the array when
num_out_bus_fmts or num_in_bus_fmts is 0, rather than patching each
individual bridge driver?

>  	*num_output_fmts = j;
>  
>  	return output_fmts;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-synopsys-dw-dp-improvements-v9-0-2e6e46135312@collabora.com?part=9