[PATCH] drm/tidss: fix crash related to accessing freed memory

Tomi Valkeinen tomi.valkeinen at ti.com
Wed Apr 15 09:29:23 UTC 2020


(Adding Jyri, forgot to add him)

On 15/04/2020 12:20, Tomi Valkeinen wrote:
> tidss uses devm_kzalloc to allocate DRM plane, encoder and crtc objects.
> This is not correct as the lifetime of those objects should be longer
> than the underlying device's.
> 
> When unloading tidss module, the devm_kzalloc'ed objects have already
> been freed when tidss_release() is called, and the driver will accesses
> freed memory possibly causing a crash, a kernel WARN, or other undefined
> behavior, and also KASAN will give a bug.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
>   drivers/gpu/drm/tidss/tidss_crtc.c    | 16 +++++++++++++---
>   drivers/gpu/drm/tidss/tidss_encoder.c | 14 +++++++++++---
>   drivers/gpu/drm/tidss/tidss_plane.c   | 24 ++++++++++++++++++------
>   3 files changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index d4ce9bab8c7e..3221a707e073 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -379,9 +379,17 @@ static struct drm_crtc_state *tidss_crtc_duplicate_state(struct drm_crtc *crtc)
>   	return &state->base;
>   }
>   
> +static void tidss_crtc_destroy(struct drm_crtc *crtc)
> +{
> +	struct tidss_crtc *tcrtc = to_tidss_crtc(crtc);
> +
> +	drm_crtc_cleanup(crtc);
> +	kfree(tcrtc);
> +}
> +
>   static const struct drm_crtc_funcs tidss_crtc_funcs = {
>   	.reset = tidss_crtc_reset,
> -	.destroy = drm_crtc_cleanup,
> +	.destroy = tidss_crtc_destroy,
>   	.set_config = drm_atomic_helper_set_config,
>   	.page_flip = drm_atomic_helper_page_flip,
>   	.atomic_duplicate_state = tidss_crtc_duplicate_state,
> @@ -400,7 +408,7 @@ struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
>   	bool has_ctm = tidss->feat->vp_feat.color.has_ctm;
>   	int ret;
>   
> -	tcrtc = devm_kzalloc(tidss->dev, sizeof(*tcrtc), GFP_KERNEL);
> +	tcrtc = kzalloc(sizeof(*tcrtc), GFP_KERNEL);
>   	if (!tcrtc)
>   		return ERR_PTR(-ENOMEM);
>   
> @@ -411,8 +419,10 @@ struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
>   
>   	ret = drm_crtc_init_with_planes(&tidss->ddev, crtc, primary,
>   					NULL, &tidss_crtc_funcs, NULL);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		kfree(tcrtc);
>   		return ERR_PTR(ret);
> +	}
>   
>   	drm_crtc_helper_add(crtc, &tidss_crtc_helper_funcs);
>   
> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
> index 83785b0a66a9..30bf2a65949c 100644
> --- a/drivers/gpu/drm/tidss/tidss_encoder.c
> +++ b/drivers/gpu/drm/tidss/tidss_encoder.c
> @@ -55,12 +55,18 @@ static int tidss_encoder_atomic_check(struct drm_encoder *encoder,
>   	return 0;
>   }
>   
> +static void tidss_encoder_destroy(struct drm_encoder *encoder)
> +{
> +	drm_encoder_cleanup(encoder);
> +	kfree(encoder);
> +}
> +
>   static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
>   	.atomic_check = tidss_encoder_atomic_check,
>   };
>   
>   static const struct drm_encoder_funcs encoder_funcs = {
> -	.destroy = drm_encoder_cleanup,
> +	.destroy = tidss_encoder_destroy,
>   };
>   
>   struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
> @@ -69,7 +75,7 @@ struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
>   	struct drm_encoder *enc;
>   	int ret;
>   
> -	enc = devm_kzalloc(tidss->dev, sizeof(*enc), GFP_KERNEL);
> +	enc = kzalloc(sizeof(*enc), GFP_KERNEL);
>   	if (!enc)
>   		return ERR_PTR(-ENOMEM);
>   
> @@ -77,8 +83,10 @@ struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
>   
>   	ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs,
>   			       encoder_type, NULL);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		kfree(enc);
>   		return ERR_PTR(ret);
> +	}
>   
>   	drm_encoder_helper_add(enc, &encoder_helper_funcs);
>   
> diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c
> index ff99b2dd4a17..798488948fc5 100644
> --- a/drivers/gpu/drm/tidss/tidss_plane.c
> +++ b/drivers/gpu/drm/tidss/tidss_plane.c
> @@ -141,6 +141,14 @@ static void tidss_plane_atomic_disable(struct drm_plane *plane,
>   	dispc_plane_enable(tidss->dispc, tplane->hw_plane_id, false);
>   }
>   
> +static void drm_plane_destroy(struct drm_plane *plane)
> +{
> +	struct tidss_plane *tplane = to_tidss_plane(plane);
> +
> +	drm_plane_cleanup(plane);
> +	kfree(tplane);
> +}
> +
>   static const struct drm_plane_helper_funcs tidss_plane_helper_funcs = {
>   	.atomic_check = tidss_plane_atomic_check,
>   	.atomic_update = tidss_plane_atomic_update,
> @@ -151,7 +159,7 @@ static const struct drm_plane_funcs tidss_plane_funcs = {
>   	.update_plane = drm_atomic_helper_update_plane,
>   	.disable_plane = drm_atomic_helper_disable_plane,
>   	.reset = drm_atomic_helper_plane_reset,
> -	.destroy = drm_plane_cleanup,
> +	.destroy = drm_plane_destroy,
>   	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>   };
> @@ -175,7 +183,7 @@ struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
>   			   BIT(DRM_MODE_BLEND_COVERAGE));
>   	int ret;
>   
> -	tplane = devm_kzalloc(tidss->dev, sizeof(*tplane), GFP_KERNEL);
> +	tplane = kzalloc(sizeof(*tplane), GFP_KERNEL);
>   	if (!tplane)
>   		return ERR_PTR(-ENOMEM);
>   
> @@ -190,7 +198,7 @@ struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
>   				       formats, num_formats,
>   				       NULL, type, NULL);
>   	if (ret < 0)
> -		return ERR_PTR(ret);
> +		goto err;
>   
>   	drm_plane_helper_add(&tplane->plane, &tidss_plane_helper_funcs);
>   
> @@ -203,15 +211,19 @@ struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
>   						default_encoding,
>   						default_range);
>   	if (ret)
> -		return ERR_PTR(ret);
> +		goto err;
>   
>   	ret = drm_plane_create_alpha_property(&tplane->plane);
>   	if (ret)
> -		return ERR_PTR(ret);
> +		goto err;
>   
>   	ret = drm_plane_create_blend_mode_property(&tplane->plane, blend_modes);
>   	if (ret)
> -		return ERR_PTR(ret);
> +		goto err;
>   
>   	return tplane;
> +
> +err:
> +	kfree(tplane);
> +	return ERR_PTR(ret);
>   }
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


More information about the dri-devel mailing list