[bug report] drm/mediatek: Add mbox_free_channel in mtk_drm_crtc_destroy

[Jason-JH Lin (林睿祥)]

Hi Dan,

Thanks for the bug report.

On Wed, 2024-09-11 at 14:02 +0300, Dan Carpenter wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hello jason-jh.lin,
> 
> Commit 593b655f0523 ("drm/mediatek: Add mbox_free_channel in
> mtk_drm_crtc_destroy") from Oct 28, 2021 (linux-next), leads to the
> following Smatch static checker warning:
> 

It's actaully caused by this Commit d7c66b5fbc70 ("drm/mediatek: Use
cmdq_pkt_create() and cmdq_pkt_destroy()").

> drivers/gpu/drm/mediatek/mtk_crtc.c:132 mtk_crtc_destroy()
> warn: variable dereferenced before check 'mtk_crtc->cmdq_client.chan' 
> (see line 130)
> 
> drivers/gpu/drm/mediatek/mtk_crtc.c
>     123 static void mtk_crtc_destroy(struct drm_crtc *crtc)
>     124 {
>     125         struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
>     126         int i;
>     127 
>     128         mtk_mutex_put(mtk_crtc->mutex);
>     129 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
>     130         cmdq_pkt_destroy(&mtk_crtc->cmdq_client, &mtk_crtc-
> >cmdq_handle);
>                                  ^^^^^^^^^^^^^^^^^^^^^^
> Dereferenced without checking.  We recently refactored these so the
> dereference
> is detectable by static analysis.

cmdq_client is a static variable in struct mtk_crtc, it's not a
pointer. Do we really need to check this?


Regards,
Jason-JH.Lin

> 
>     131 
> --> 132         if (mtk_crtc->cmdq_client.chan) {
>                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Checked too late.
> 
>     133                 mbox_free_channel(mtk_crtc-
> >cmdq_client.chan);
>     134                 mtk_crtc->cmdq_client.chan = NULL;
>     135         }
>     136 #endif
>     137 
>     138         for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
>     139                 struct mtk_ddp_comp *comp;
>     140 
>     141                 comp = mtk_crtc->ddp_comp[i];
>     142                 mtk_ddp_comp_unregister_vblank_cb(comp);
>     143         }
>     144 
>     145         drm_crtc_cleanup(crtc);
>     146 }
> 
> regards,
> dan carpenter