> -----Original Message----- > From: Sachin Kamat [mailto:sachin.kamat@xxxxxxxxxx] > Sent: Thursday, November 22, 2012 3:13 PM > To: Inki Dae > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; jy0922.shim@xxxxxxxxxxx; > patches@xxxxxxxxxx > Subject: Re: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > dereference in exynos_drm_encoder.c > > Hi Inki, > > On 19 November 2012 15:32, Sachin Kamat <sachin.kamat@xxxxxxxxxx> wrote: > > On 19 November 2012 15:30, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > >> > >> > >>> -----Original Message----- > >>> From: Sachin Kamat [mailto:sachin.kamat@xxxxxxxxxx] > >>> Sent: Monday, November 19, 2012 6:56 PM > >>> To: Inki Dae > >>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; jy0922.shim@xxxxxxxxxxx; > >>> patches@xxxxxxxxxx > >>> Subject: Re: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > >>> dereference in exynos_drm_encoder.c > >>> > >>> Hi Inki, > >>> > >>> Thanks for your review. My comments inline. > >>> > >>> On 19 November 2012 15:14, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > >>> > > >>> > > >>> >> -----Original Message----- > >>> >> From: Sachin Kamat [mailto:sachin.kamat@xxxxxxxxxx] > >>> >> Sent: Monday, November 19, 2012 6:21 PM > >>> >> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx > >>> >> Cc: inki.dae@xxxxxxxxxxx; jy0922.shim@xxxxxxxxxxx; > >>> > sachin.kamat@xxxxxxxxxx; > >>> >> patches@xxxxxxxxxx > >>> >> Subject: [PATCH 1/1] drm/exynos: Fix potential NULL pointer > dereference > >>> in > >>> >> exynos_drm_encoder.c > >>> >> > >>> >> Check overlay_ops is not NULL as checked in the previous 'if' > >> condition. > >>> >> Fixes the following smatch error: > >>> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c:509 > >>> >> exynos_drm_encoder_plane_disable() > >>> >> error: we previously assumed 'overlay_ops' could be null (see line > 499) > >>> >> > >>> >> Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx> > >>> >> --- > >>> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c | 2 +- > >>> >> 1 files changed, 1 insertions(+), 1 deletions(-) > >>> >> > >>> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> b/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> index e51503f..a44238e 100644 > >>> >> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c > >>> >> @@ -506,6 +506,6 @@ void exynos_drm_encoder_plane_disable(struct > >>> >> drm_encoder *encoder, void *data) > >>> >> * because the setting for disabling the overlay will be > updated > >>> >> * at vsync. > >>> >> */ > >>> >> - if (overlay_ops->wait_for_vblank) > >>> >> + if (overlay_ops && overlay_ops->wait_for_vblank) > >>> >> overlay_ops->wait_for_vblank(manager->dev); > >>> > > >>> > This code will be removed at -next. > >>> > >>> Since this code is already in mainline, I think this patch should be > >>> applied as a fix during this rc (for completeness). > >>> You may subsequently delete it in the next release as per your plan. > >>> > >> > >> And NULL pointer checking was already done above like below, > >> if (overlay_ops && overlay_ops->disable) > >> overlay_ops->disable(manager->dev, zpos); > > Correct. But that check is applicable only for that one statement > > (overlay_ops->disable(manager->dev, zpos);). > > > > Similar check needs to be added to below 'if' code too. > > What are your comments about this? > Left condition first is checked so as I mentioned before, it doesn't need overlay_ops checking because that was checked already. why do you think overlay_ops should be checked again? > > > >> > >> This is your missing point. > >> > >>> > > >>> > Thanks, > >>> > Inki Dae > >>> > > >>> >> } > >>> >> -- > >>> >> 1.7.4.1 > >>> > > >>> > >>> > >>> > >>> -- > >>> With warm regards, > >>> Sachin > >> > > > > > > > > -- > > With warm regards, > > Sachin > > > > -- > With warm regards, > Sachin _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
