Re: [PATCH v5 07/13] drm/client: splash: add a device tree image source

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] of: use-after-free of DT property data due to premature of_node_put()
--

commit 98dbc79d3322b530cd1f8ff31d16dc494f5db1c5
Author: Màxim Pedraza Padilla <maximpedraza@gmail.com>

    drm/client: splash: add a device tree image source

    This commit adds a device tree image source for the splash client,
    allowing it to load a BMP image from a boot-logo node under /chosen
    in the device tree.

> diff --git a/drivers/gpu/drm/clients/drm_splash.c b/drivers/gpu/drm/clients/drm_splash.c
> index a7d6279541587..50842922a7f82 100644
> --- a/drivers/gpu/drm/clients/drm_splash.c
> +++ b/drivers/gpu/drm/clients/drm_splash.c
[ ... ]
> @@ -212,6 +223,114 @@ static int drm_splash_fw_request_bmp(struct drm_splash *splash)
[ ... ]
> +static int drm_splash_dt_load(struct drm_splash *splash, const u8 **data,
> +			      size_t *size)
> +{
> +	struct drm_client_dev *client = &splash->client;
> +	struct device_node *np;
> +	struct resource res;
> +	const void *prop;
> +	int len, ret = 0;
> +
> +	np = drm_splash_dt_node(splash);
> +	if (!np)
> +		return -ENOENT;
> +
> +	/*
> +	 * An image in the node is used in place: the device tree stays in
> +	 * memory for as long as the system runs, so there is nothing to copy
> +	 * and nothing to release.
> +	 */
> +	prop = of_get_property(np, "image", &len);
> +	if (prop) {
> +		*data = prop;
> +		*size = len;
> +		goto out_put;
> +	}
[ ... ]
> +out_put:
> +	of_node_put(np);
> +	return ret;
> +}

[Severity: High]
Does this code risk a use-after-free if the device tree node is dynamically
removed?

The comment notes the device tree stays in memory, but if device tree overlays
are used and the node is dynamically removed, its properties will be freed
when the reference count reaches zero.

Here, drm_splash_dt_load() assigns the pointer from of_get_property() to
*data, but then immediately drops the reference to the device tree node by
calling of_node_put() before returning. 

Later, the asynchronous render thread could read this freed memory when parsing
the BMP header and blitting the image via drm_splash_bmp_to_scanout().

Should the node reference be kept in struct drm_splash and only released
later in drm_splash_image_cleanup()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013093243.505776-1-maximpedraza@gmail.com?part=7