linux-kernel - Re: [PATCH v2] drm/gem-shmem: When drm_gem_object

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2b4e38d8-d0ea-e85c-88f1-bb6a714ee0eb@suse.de>
Date:   Thu, 17 Nov 2022 14:42:36 +0100
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     ChunyouTang <tangchunyou@....com>,
        maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
        airlied@...il.com, daniel@...ll.ch, sumit.semwal@...aro.org,
        christian.koenig@....com
Cc:     dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should
 release object

Hi

Am 11.11.22 um 04:38 schrieb ChunyouTang:
> when goto err_free, the object had init, so it should be release when fail.
> 
> Signed-off-by: ChunyouTang <tangchunyou@....com>
> ---
>   drivers/gpu/drm/drm_gem.c              | 19 ++++++++++++++++---
>   drivers/gpu/drm/drm_gem_shmem_helper.c |  4 +++-
>   include/drm/drm_gem.h                  |  1 +
>   3 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 8b68a3c1e6ab..cba32c46bb05 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -169,6 +169,21 @@ void drm_gem_private_object_init(struct drm_device *dev,
>   }
>   EXPORT_SYMBOL(drm_gem_private_object_init);
>   
> +/**
> + * drm_gem_private_object_fini - Finalize a failed drm_gem_object
> + * @obj: drm_gem_object
> + *
> + * Uninitialize an already allocated GEM object when it initialized failed
> + */
> +void drm_gem_private_object_fini(struct drm_gem_object *obj)
> +{
> +	WARN_ON(obj->dma_buf);

Rather lease this in its original place.

> +
> +	dma_resv_fini(&obj->_resv);
> +	drm_gem_lru_remove(obj);

AFAICT drm_gem_lru_remove() doesn't belong into this function.

> +}
> +EXPORT_SYMBOL(drm_gem_private_object_fini);
> +
>   /**
>    * drm_gem_object_handle_free - release resources bound to userspace handles
>    * @obj: GEM object to clean up.
> @@ -930,14 +945,12 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private)
>   void
>   drm_gem_object_release(struct drm_gem_object *obj)
>   {
> -	WARN_ON(obj->dma_buf);
> +	drm_gem_private_object_fini(obj);
>   
>   	if (obj->filp)
>   		fput(obj->filp);
>   
> -	dma_resv_fini(&obj->_resv);

Please call drm_gem_private_object_fini() here.

>   	drm_gem_free_mmap_offset(obj);
> -	drm_gem_lru_remove(obj);

Please keep this line here.

Best regards
Thomas

>   }
>   EXPORT_SYMBOL(drm_gem_object_release);
>   
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 35138f8a375c..845e3d5d71eb 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -79,8 +79,10 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
>   	} else {
>   		ret = drm_gem_object_init(dev, obj, size);
>   	}
> -	if (ret)
> +	if (ret) {
> +		drm_gem_private_object_fini(obj)
>   		goto err_free;
> +	}
>   
>   	ret = drm_gem_create_mmap_offset(obj);
>   	if (ret)
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index bd42f25e449c..9b1feb03069d 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -405,6 +405,7 @@ int drm_gem_object_init(struct drm_device *dev,
>   			struct drm_gem_object *obj, size_t size);
>   void drm_gem_private_object_init(struct drm_device *dev,
>   				 struct drm_gem_object *obj, size_t size);
> +void drm_gem_private_object_fini(struct drm_gem_object *obj);
>   void drm_gem_vm_open(struct vm_area_struct *vma);
>   void drm_gem_vm_close(struct vm_area_struct *vma);
>   int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)