linux-kernel - Re: [PATCH v6 6/8] drm: writeback: Add missing cleanup in case of initialization failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250106-airborne-vicugna-of-effort-534a4f@houat>
Date: Mon, 6 Jan 2025 13:58:39 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>, 
	Melissa Wen <melissa.srw@...il.com>, Maíra Canal <mairacanal@...eup.net>, 
	Haneen Mohammed <hamohammed.sa@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Simona Vetter <simona.vetter@...ll.ch>, 
	dri-devel@...ts.freedesktop.org, arthurgrillo@...eup.net, linux-kernel@...r.kernel.org, 
	jeremie.dautheribes@...tlin.com, miquel.raynal@...tlin.com, thomas.petazzoni@...tlin.com, 
	seanpaul@...gle.com, nicolejadeyee@...gle.com
Subject: Re: [PATCH v6 6/8] drm: writeback: Add missing cleanup in case of
 initialization failure

On Mon, Dec 30, 2024 at 07:37:36PM +0100, Louis Chauvet wrote:
> The current implementation of drm_writeback_connector initialization does
> not properly clean up all resources in case of failure (allocated
> properties and possible_encoders). Add this cleaning in case of failure.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 494400b09796d37ed89145da45d5f1e029632de5..9c69f7181e02c23dabce488405608c40d4184af5 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -247,18 +247,20 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>  	int ret = create_writeback_properties(dev);
>  
>  	if (ret != 0)
> -		return ret;
> +		goto failed_properties;
>  
>  	connector->interlace_allowed = 0;
>  
>  	ret = drm_connector_attach_encoder(connector, enc);
>  	if (ret)
> -		return ret;
> +		goto failed_properties;
>  
>  	blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
>  					formats);
> -	if (IS_ERR(blob))
> -		return PTR_ERR(blob);
> +	if (IS_ERR(blob)) {
> +		ret = PTR_ERR(blob);
> +		goto failed_blob;
> +	}
>  
>  	INIT_LIST_HEAD(&wb_connector->job_queue);
>  	spin_lock_init(&wb_connector->job_lock);
> @@ -281,6 +283,11 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>  	wb_connector->pixel_formats_blob_ptr = blob;
>  
>  	return 0;
> +failed_blob:
> +	connector->possible_encoders &= ~drm_encoder_mask(enc);

I don't think it's worth it to uninitialize that particular field. All
the structure fields are going to be in an undefined state, and
shouldn't be used anymore.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)