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? > >> >> 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
