Re: [PATCH v3 1/2] drm/logicvc: Avoid use-after-free with devm_kzalloc()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The local variable `bridge` is passed to `drm_of_find_panel_or_bridge()` without initialization, leading to a crash when the function returns an error without setting it.
--

--- Patch [1]: [PATCH v3 1/2] drm/logicvc: Avoid use-after-free with devm_kzalloc() ---
commit 95ef468f51e9bfed648e230bd616663ec7bac106
Author: Romain Gantois <romain.gantois@bootlin.com>

drm/logicvc: Avoid use-after-free with devm_kzalloc()

This commit transitions the panel, CRTC, and encoder objects to DRM-managed
allocations. It removes devm_kzalloc usages in order to fix a use-after-free
bug.

> diff --git a/drivers/gpu/drm/logicvc/logicvc_interface.c b/drivers/gpu/drm/logicvc/logicvc_interface.c
> --- a/drivers/gpu/drm/logicvc/logicvc_interface.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_interface.c
> @@ -147,36 +143,35 @@ int logicvc_interface_init(struct logicvc_drm *logicvc)
>  	int encoder_type = logicvc_interface_encoder_type(logicvc);
>  	int connector_type = logicvc_interface_connector_type(logicvc);
>  	bool native_connector = logicvc_interface_native_connector(logicvc);
> +	struct drm_bridge *bridge;
> +	struct drm_panel *panel;
>  	int ret;
>  
> -	interface = devm_kzalloc(dev, sizeof(*interface), GFP_KERNEL);
> -	if (!interface) {
> -		ret = -ENOMEM;
> -		goto error_early;
> -	}
> -
> -	ret = drm_of_find_panel_or_bridge(of_node, 0, 0, &interface->drm_panel,
> -					  &interface->drm_bridge);
> +	ret = drm_of_find_panel_or_bridge(of_node, 0, 0, &panel,
> +					  &bridge);
>  	if (ret == -EPROBE_DEFER)
> -		goto error_early;

[Severity: High]
Are panel and bridge guaranteed to be initialized if this call fails?

When drm_of_find_panel_or_bridge() returns an error like -ENODEV (which
is valid for native DVI connectors without an OF graph), it might not
initialize these variables. Previously, interface was allocated with
devm_kzalloc(), so they were safely initialized to NULL. Now, they are
uninitialized stack variables.

> -	ret = drm_encoder_init(drm_dev, &interface->drm_encoder,
> -			       &logicvc_encoder_funcs, encoder_type, NULL);
> -	if (ret) {
> +	interface = drmm_encoder_alloc(drm_dev, struct logicvc_interface, drm_encoder,
> +				       NULL, encoder_type, NULL);
> +	if (IS_ERR(interface)) {
>  		drm_err(drm_dev, "Failed to initialize encoder\n");
> -		goto error_early;
> +		return PTR_ERR(interface);
>  	}
>  
> +	interface->drm_panel = panel;
> +	interface->drm_bridge = bridge;

[Severity: High]
Could this assignment cause a crash later during driver initialization?

If drm_of_find_panel_or_bridge() returned -ENODEV, bridge will contain
stack garbage. Later in logicvc_interface_init(), the code evaluates
if (interface->drm_bridge) and calls drm_bridge_attach(), which would
dereference this uninitialized pointer and trigger a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-logicvc-uaf-v3-0-f3b2319d58a6@bootlin.com?part=1