lkml.org 
[lkml]   [2024]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH] drm/mediatek: Init `ddp_comp` with devm_kcalloc()
    Date
    In the case where `conn_routes` is true we allocate an extra slot in
    the `ddp_comp` array but mtk_drm_crtc_create() never seemed to
    initialize it in the test case I ran. For me, this caused a later
    crash when we looped through the array in mtk_drm_crtc_mode_valid().
    This showed up for me when I booted with `slub_debug=FZPUA` which
    poisons the memory initially. Without `slub_debug` I couldn't
    reproduce, presumably because the later code handles the value being
    NULL and in most cases (not guaranteed in all cases) the memory the
    allocator returned started out as 0.

    It really doesn't hurt to initialize the array with devm_kcalloc()
    since the array is small and the overhead of initting a handful of
    elements to 0 is small. In general initting memory to zero is a safer
    practice and usually it's suggested to only use the non-initting alloc
    functions if you really need to.

    Let's switch the function to use an allocation function that zeros the
    memory. For me, this avoids the crash.

    Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
    Signed-off-by: Douglas Anderson <dianders@chromium.org>
    ---
    I don't have a ton of experience with this driver to know if the fact
    that the array item was still uninitialized when
    mtk_drm_crtc_mode_valid() ran is the sign of a bug that should be
    fixed. However, even if it is a bug and that bug is fixed then zeroing
    memory when we allocate is still safer. If it's a bug that this memory
    wasn't initialized then please consider this patch a bug report. ;-)

    I'll also note that I reproduced this on a downstream 6.1-based
    kernel. It appears that only mt8188 uses `conn_routes` and, as far as
    I can tell, mt8188 isn't supported upstream yet.

    drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 ++++----
    1 file changed, 4 insertions(+), 4 deletions(-)

    diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
    index a04499c4f9ca..29207b2756c1 100644
    --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
    +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
    @@ -1009,10 +1009,10 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,

    mtk_crtc->mmsys_dev = priv->mmsys_dev;
    mtk_crtc->ddp_comp_nr = path_len;
    - mtk_crtc->ddp_comp = devm_kmalloc_array(dev,
    - mtk_crtc->ddp_comp_nr + (conn_routes ? 1 : 0),
    - sizeof(*mtk_crtc->ddp_comp),
    - GFP_KERNEL);
    + mtk_crtc->ddp_comp = devm_kcalloc(dev,
    + mtk_crtc->ddp_comp_nr + (conn_routes ? 1 : 0),
    + sizeof(*mtk_crtc->ddp_comp),
    + GFP_KERNEL);
    if (!mtk_crtc->ddp_comp)
    return -ENOMEM;

    --
    2.44.0.396.g6e790dbe36-goog

    \
     
     \ /
      Last update: 2024-05-27 16:14    [W:5.499 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site