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

[Romain Gantois <romain.gantois@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]

On Wednesday, 22 July 2026 12:00:28 CEST sashiko-bot@kernel.org wrote:
> 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.

Indeed, these two stack variables need to be initialized to NULL.

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]