On 11/24/23 11:04, Katya Orlova wrote:
> ltdc_load() calls functions drm_crtc_init_with_planes(),
> drm_universal_plane_init() and drm_encoder_init(). These functions
> should not be called with parameters allocated with devm_kzalloc()
> to avoid use-after-free issues [1].
>
> Use allocations managed by the DRM framework.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> [1]
> https://lore.kernel.org/lkml/u366i76e3qhh3ra5oxrtngjtm2u5lterkekcz6y2jkndhuxzli@diujon4h7qwb/
>
> Signed-off-by: Katya Orlova
> ---
> v2: use allocations managed by the DRM as
> Raphael Gallais-Pou suggested.
> Also add a fix for encoder.
> drivers/gpu/drm/stm/drv.c | 3 +-
> drivers/gpu/drm/stm/ltdc.c | 68 +-
> 2 files changed, 18 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index e8523abef27a..152bec2c0238 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
>
> #include "ltdc.h"
>
> @@ -75,7 +76,7 @@ static int drv_load(struct drm_device *ddev)
>
> DRM_DEBUG("%s\n", __func__);
>
> - ldev = devm_kzalloc(ddev->dev, sizeof(*ldev), GFP_KERNEL);
> + ldev = drmm_kzalloc(ddev, sizeof(*ldev), GFP_KERNEL);
> if (!ldev)
> return -ENOMEM;
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 5576fdae4962..02a7c8375f44 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -36,6 +36,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -1199,7 +1200,6 @@ static void ltdc_crtc_atomic_print_state(struct
> drm_printer *p,
> }
>
> static const struct drm_crtc_funcs ltdc_crtc_funcs = {
> - .destroy = drm_crtc_cleanup,
> .set_config = drm_atomic_helper_set_config,
> .page_flip = drm_atomic_helper_page_flip,
> .reset = drm_atomic_helper_crtc_reset,
> @@ -1212,7 +1212,6 @@ static const struct drm_crtc_funcs ltdc_crtc_funcs = {
> };
>
> static const struct drm_crtc_funcs ltdc_crtc_with_crc_support_funcs = {
> - .destroy = drm_crtc_cleanup,
> .set_config = drm_atomic_helper_set_config,
> .page_flip = drm_atomic_helper_page_flip,
> .reset = drm_atomic_helper_crtc_reset,
> @@ -1545,7 +1544,6 @@ static void ltdc_plane_atomic_print_state(struct
> drm_printer *p,
> static const struct drm_plane_funcs ltdc_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .destroy = drm_plane_cleanup,
> .reset = drm_atomic_helper_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> @@ -1572,7 +1570,6 @@ static struct drm_plane *ltdc_plane_create(struct
> drm_device *ddev,
> const u64 *modifiers = ltdc_format_modifiers;
> u32 lofs = index * LAY_OFS;
> u32 val;
> - int ret;
>
> /* Allocate the biggest size according to supported color formats */
> formats = devm_kzalloc(dev, (ldev->caps.pix_fmt_nb +
> @@ -1613,14 +1610,10 @@ static struct drm_plane *ltdc_plane_create(struct
> drm_device *ddev,
> }
> }
>
> - plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL);
> - if (!plane)
> - return NULL;
> -
> - ret = drm_universal_plane_init(ddev, plane, possible_crtcs,
> - -modifiers, type, NULL);
> - if (ret < 0)
> + plane = drmm_universal_plane_alloc(ddev, struct drm_plane, dev,
> +possible_crtcs, formats, nb_fmt,
> +modifiers, type, NULL);
Hi Katya,
Thanks for your submission, and sorry for the delay.
There is several alignment style problems, such as the lines above.
You can use "--strict" option with checkpatch script to show you all the faulty
alignment before sending a patch.
Other than that this patch looks pretty good to me.
Regards,
Raphaël
> + if (IS_ERR(plane))
> return NULL;
>
> if (ldev->caps.ycbcr_input) {
> @@ -1643,15 +1636,6 @@ static struct drm_plane *ltdc_plane_create(struct
> drm_device *ddev,
> return plane;
> }
>
> -static void ltdc_plane_destroy_all(struct drm_device *ddev)
> -{
> - struct drm_plane *plane, *plane_temp;
> -
> - list_for_each_entry_safe(plane, plane_temp,
> - &ddev->mode_config.plane_list, head)
> - drm_plane_cleanup(plane);
> -}
> -
> static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
> {
> struct ltdc_device *ldev = ddev->dev_private;
> @@ -1677,14 +1661,14 @@ static int ltdc_crtc_init(struct drm_device *ddev,
> struct drm_crtc *crtc)
>
> /* I
