[PATCH 1/1] drm/exynos: Fix potential NULL pointer dereference in exynos_drm_encoder.c

[Sachin Kamat]

Hi Inki,

On 19 November 2012 15:32, Sachin Kamat <sachin.kamat at linaro.org> wrote:
> On 19 November 2012 15:30, Inki Dae <inki.dae at samsung.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Sachin Kamat [mailto:sachin.kamat at linaro.org]
>>> Sent: Monday, November 19, 2012 6:56 PM
>>> To: Inki Dae
>>> Cc: dri-devel at lists.freedesktop.org; jy0922.shim at samsung.com;
>>> patches at linaro.org
>>> 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 at samsung.com> wrote:
>>> >
>>> >
>>> >> -----Original Message-----
>>> >> From: Sachin Kamat [mailto:sachin.kamat at linaro.org]
>>> >> Sent: Monday, November 19, 2012 6:21 PM
>>> >> To: dri-devel at lists.freedesktop.org
>>> >> Cc: inki.dae at samsung.com; jy0922.shim at samsung.com;
>>> > sachin.kamat at linaro.org;
>>> >> patches at linaro.org
>>> >> 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 at linaro.org>
>>> >> ---
>>> >>  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