drm/tidss: fix crash related to accessing freed memory
authorTomi Valkeinen <tomi.valkeinen@ti.com>
Wed, 15 Apr 2020 09:20:06 +0000 (12:20 +0300)
committerTomi Valkeinen <tomi.valkeinen@ti.com>
Mon, 20 Apr 2020 07:07:35 +0000 (10:07 +0300)
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@ti.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200415092006.26675-1-tomi.valkeinen@ti.com
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/tidss/tidss_crtc.c
drivers/gpu/drm/tidss/tidss_encoder.c
drivers/gpu/drm/tidss/tidss_plane.c

index d4ce9bab8c7e61369ab52bc6617d3a6fddeb3610..3221a707e07366c68c03e2945a72b7ddd5298c20 100644 (file)
@@ -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);
 
index 83785b0a66a9d28740fed7848b6559bbb99dce0f..30bf2a65949cff34f14f5efb6abdf63d71ca71cc 100644 (file)
@@ -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);
 
index ff99b2dd4a17f8681ee0c9f7aa6ecfa2b6837803..798488948fc539716bfe53a5316583b1360ee5ee 100644 (file)
@@ -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);
 }