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

[Dan Carpenter]

On Thu, Sep 12, 2024 at 07:56:27AM +0000, Jason-JH Lin (林睿祥) wrote:
> 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?
> 

Sorry, the bug report was a bit unclear.  The issue is
"mtk_crtc->cmdq_client.chan", not "cmdq_client".  I've done this analysis now,
and we do need to check for NULL.  I'll send a patch.

regards,
dan carpenter