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

[Jason-JH Lin (林睿祥)]

On Thu, 2024-09-12 at 11:43 +0300, Dan Carpenter wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  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.
> 

Got it. Thanks a lot!

Regards,
Jason-JH.Lin

> regards,
> dan carpenter
>