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 499)
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.
This is your missing point.
Thanks, Inki Dae
}
1.7.4.1
-- With warm regards, Sachin