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