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); }
-----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.
Thanks, Inki Dae
}
1.7.4.1
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.
Thanks, Inki Dae
}
1.7.4.1
-----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);
This is your missing point.
Thanks, Inki Dae
}
1.7.4.1
-- With warm regards, Sachin
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
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 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.
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
-----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
[snip]
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?
Consider the case when overlay_ops is NULL.
if (overlay_ops && overlay_ops->disable) overlay_ops->disable(manager->dev, zpos);
It does not enter this condition as overlay_ops is NULL and moves to the next statement, if (overlay_ops->wait_for_vblank) where it gets dereferenced.
Please note we are not returning back from the first condition if overlay_ops is NULL. Hence we need to check the condition in second case too.
-----Original Message----- From: Sachin Kamat [mailto:sachin.kamat@linaro.org] Sent: Thursday, November 22, 2012 5:19 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
[snip]
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?
Consider the case when overlay_ops is NULL.
if (overlay_ops && overlay_ops->disable) overlay_ops->disable(manager->dev, zpos);
It does not enter this condition as overlay_ops is NULL and moves to the next statement, if (overlay_ops->wait_for_vblank) where it gets dereferenced.
Please note we are not returning back from the first condition if overlay_ops is NULL. Hence we need to check the condition in second case too.
Ah~ Right. I didn't check it surely. :)
Thanks, Inki Dae
-- With warm regards, Sachin
dri-devel@lists.freedesktop.org
-
Inki Dae -
Sachin Kamat