[PATCH v2 1/2] drm: adv7511: Fix use-after-free in adv7533_attach_dsi()

[Laurent Pinchart]

Hi Biju,

Thank you for the patch.

On Tue, Nov 05, 2024 at 11:12:18AM +0000, Biju Das wrote:
> The host_node pointer assigned and freed in adv7533_parse_dt()
> and later adv7533_attach_dsi() uses the same. Fix this issue
> by freeing the host_node in adv7533_attach_dsi() instead of
> adv7533_parse_dt().
> 
> Fixes: 1e4d58cd7f88 ("drm/bridge: adv7533: Create a MIPI DSI device")
> Cc: stable at vger.kernel.org
> Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> ---
> Changes in v2:
>  - Added the tag "Cc: stable at vger.kernel.org" in the sign-off area.
>  - Dropped Archit Taneja invalid Mail address
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7533.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 4481489aaf5e..3e57ba838e5e 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -133,6 +133,7 @@ int adv7533_patch_cec_registers(struct adv7511 *adv)
>  
>  int adv7533_attach_dsi(struct adv7511 *adv)
>  {
> +	struct device_node *np __free(device_node) = adv->host_node;

This raises so many red flags. The function will free the node while the
adv->host_node variable still points to it, opening the door to
use-after-free. Furthermore, there's nothing in the function name that
hints it will do this, callers can get surprised by this behaviour.

I'm sure you can do better than this and fix the problem with readable
code, not cryptic and error-prone constructs :-)

>  	struct device *dev = &adv->i2c_main->dev;
>  	struct mipi_dsi_host *host;
>  	struct mipi_dsi_device *dsi;
> @@ -181,8 +182,6 @@ int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv)
>  	if (!adv->host_node)
>  		return -ENODEV;
>  
> -	of_node_put(adv->host_node);
> -
>  	adv->use_timing_gen = !of_property_read_bool(np,
>  						"adi,disable-timing-generator");
>  

-- 
Regards,

Laurent Pinchart