* [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null
2018-10-02 21:59 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
@ 2018-10-02 21:59 ` Giulio Benetti
2018-10-03 7:34 ` Chen-Yu Tsai
2018-10-02 22:00 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
2018-10-03 9:43 ` Chen-Yu Tsai
2 siblings, 1 reply; 19+ messages in thread
From: Giulio Benetti @ 2018-10-02 21:59 UTC (permalink / raw)
To: linux-arm-kernel
If using tcon with VGA, tcon->panel will be null(0), this will cause
segmentation fault when trying to dereference tcon->panel->connector.
Add tcon->panel null check before calling
sun4i_tcon0_mode_set_dithering().
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index e4b3bd0307ef..f949287d926c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -491,7 +491,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
sun4i_tcon0_mode_set_common(tcon, mode);
/* Set dithering if needed */
- sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
+ if (tcon->panel)
+ sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null
2018-10-02 21:59 ` [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
@ 2018-10-03 7:34 ` Chen-Yu Tsai
0 siblings, 0 replies; 19+ messages in thread
From: Chen-Yu Tsai @ 2018-10-03 7:34 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 3, 2018 at 5:59 AM Giulio Benetti
<giulio.benetti@micronovasrl.com> wrote:
>
> If using tcon with VGA, tcon->panel will be null(0), this will cause
> segmentation fault when trying to dereference tcon->panel->connector.
>
> Add tcon->panel null check before calling
> sun4i_tcon0_mode_set_dithering().
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
RGB565/RGB666 LCD panels")
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
2018-10-02 21:59 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
2018-10-02 21:59 ` [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
@ 2018-10-02 22:00 ` Giulio Benetti
2018-10-03 9:43 ` Chen-Yu Tsai
2 siblings, 0 replies; 19+ messages in thread
From: Giulio Benetti @ 2018-10-02 22:00 UTC (permalink / raw)
To: linux-arm-kernel
Sorry for sending twice(and top posting), but I was not subscribed to
dri-devel ML, so patchwork was not aware of these 2 patches.
Best regards
--
Giulio Benetti
CTO
MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale ? 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642
Il 02/10/2018 23:59, Giulio Benetti ha scritto:
> At the moment, the check of tcon->panel to be valid is wrong. IS_ERR()
> has been used, but that macro doesn't check if tcon->panel pointer is
> null or not, but check if tcon->panel is between -1 and -4095(MAX_ERRNO).
>
> Remove IS_ERR() from tcon->panel checking and let "if (tcon->panel)" as
> condition to check if it's a pointer not null.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index c78cd35a1294..e4b3bd0307ef 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -555,7 +555,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> * Following code is a way to avoid quirks all around TCON
> * and DOTCLOCK drivers.
> */
> - if (!IS_ERR(tcon->panel)) {
> + if (tcon->panel) {
> struct drm_panel *panel = tcon->panel;
> struct drm_connector *connector = panel->connector;
> struct drm_display_info display_info = connector->display_info;
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
2018-10-02 21:59 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
2018-10-02 21:59 ` [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
2018-10-02 22:00 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
@ 2018-10-03 9:43 ` Chen-Yu Tsai
2018-10-03 12:13 ` Giulio Benetti
2 siblings, 1 reply; 19+ messages in thread
From: Chen-Yu Tsai @ 2018-10-03 9:43 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 3, 2018 at 5:59 AM Giulio Benetti
<giulio.benetti@micronovasrl.com> wrote:
>
> At the moment, the check of tcon->panel to be valid is wrong. IS_ERR()
> has been used, but that macro doesn't check if tcon->panel pointer is
> null or not, but check if tcon->panel is between -1 and -4095(MAX_ERRNO).
>
> Remove IS_ERR() from tcon->panel checking and let "if (tcon->panel)" as
> condition to check if it's a pointer not null.
There's actually more than one occurance of this error:
drivers/gpu/drm/sun4i/sun4i_lvds.c: if (!IS_ERR(tcon->panel)) {
drivers/gpu/drm/sun4i/sun4i_lvds.c: if (!IS_ERR(tcon->panel)) {
drivers/gpu/drm/sun4i/sun4i_rgb.c: if (!IS_ERR(tcon->panel)) {
drivers/gpu/drm/sun4i/sun4i_rgb.c: if (!IS_ERR(tcon->panel)) {
These four are responsible for enabling and disabling the panel.
drivers/gpu/drm/sun4i/sun4i_tcon.c: if (!IS_ERR(tcon->panel)) {
All this looks like it was left over from commit ebc9446135671 ("drm: convert
drivers to use drm_of_find_panel_or_bridge"). We have checks against tcon->panel
in several places and not all of them were converted.
ChenYu
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index c78cd35a1294..e4b3bd0307ef 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -555,7 +555,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> * Following code is a way to avoid quirks all around TCON
> * and DOTCLOCK drivers.
> */
> - if (!IS_ERR(tcon->panel)) {
> + if (tcon->panel) {
> struct drm_panel *panel = tcon->panel;
> struct drm_connector *connector = panel->connector;
> struct drm_display_info display_info = connector->display_info;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
2018-10-03 9:43 ` Chen-Yu Tsai
@ 2018-10-03 12:13 ` Giulio Benetti
2018-10-03 14:24 ` [PATCH v2 " Giulio Benetti
0 siblings, 1 reply; 19+ messages in thread
From: Giulio Benetti @ 2018-10-03 12:13 UTC (permalink / raw)
To: linux-arm-kernel
Il 03/10/2018 11:43, Chen-Yu Tsai ha scritto:
> On Wed, Oct 3, 2018 at 5:59 AM Giulio Benetti
> <giulio.benetti@micronovasrl.com> wrote:
>>
>> At the moment, the check of tcon->panel to be valid is wrong. IS_ERR()
>> has been used, but that macro doesn't check if tcon->panel pointer is
>> null or not, but check if tcon->panel is between -1 and -4095(MAX_ERRNO).
>>
>> Remove IS_ERR() from tcon->panel checking and let "if (tcon->panel)" as
>> condition to check if it's a pointer not null.
>
> There's actually more than one occurance of this error:
>
> drivers/gpu/drm/sun4i/sun4i_lvds.c: if (!IS_ERR(tcon->panel)) {
> drivers/gpu/drm/sun4i/sun4i_lvds.c: if (!IS_ERR(tcon->panel)) {
> drivers/gpu/drm/sun4i/sun4i_rgb.c: if (!IS_ERR(tcon->panel)) {
> drivers/gpu/drm/sun4i/sun4i_rgb.c: if (!IS_ERR(tcon->panel)) {
>
> These four are responsible for enabling and disabling the panel.
>
> drivers/gpu/drm/sun4i/sun4i_tcon.c: if (!IS_ERR(tcon->panel)) {
>
> All this looks like it was left over from commit ebc9446135671 ("drm: convert
> drivers to use drm_of_find_panel_or_bridge"). We have checks against tcon->panel
> in several places and not all of them were converted.
>
Then, I'm going to send a patchset to correct them.
Best regards
--
Giulio Benetti
CTO
MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale ? 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
2018-10-03 12:13 ` Giulio Benetti
@ 2018-10-03 14:24 ` Giulio Benetti
2018-10-03 14:24 ` [PATCH v2 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
2018-10-04 19:54 ` [PATCH v2 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Maxime Ripard
0 siblings, 2 replies; 19+ messages in thread
From: Giulio Benetti @ 2018-10-03 14:24 UTC (permalink / raw)
To: linux-arm-kernel
At the moment, the check of tcon->panel to be valid is wrong. IS_ERR()
has been used, but that macro doesn't check if tcon->panel pointer is
null or not, but check if tcon->panel is between -1 and -4095(MAX_ERRNO).
Remove IS_ERR() from tcon->panel checking and let "if (tcon->panel)" as
condition to check if it's a pointer not null.
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
Changes V1->V2:
* correct same bug for all same occurences in drm/sun4i folder
drivers/gpu/drm/sun4i/sun4i_lvds.c | 4 ++--
drivers/gpu/drm/sun4i/sun4i_rgb.c | 4 ++--
drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c
index af7dcb6da351..e7eb0d1e17be 100644
--- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
+++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
@@ -75,7 +75,7 @@ static void sun4i_lvds_encoder_enable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Enabling LVDS output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_prepare(tcon->panel);
drm_panel_enable(tcon->panel);
}
@@ -88,7 +88,7 @@ static void sun4i_lvds_encoder_disable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Disabling LVDS output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_disable(tcon->panel);
drm_panel_unprepare(tcon->panel);
}
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index bf068da6b12e..f4a22689eb54 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -135,7 +135,7 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Enabling RGB output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_prepare(tcon->panel);
drm_panel_enable(tcon->panel);
}
@@ -148,7 +148,7 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Disabling RGB output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_disable(tcon->panel);
drm_panel_unprepare(tcon->panel);
}
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c78cd35a1294..e4b3bd0307ef 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -555,7 +555,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
* Following code is a way to avoid quirks all around TCON
* and DOTCLOCK drivers.
*/
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
struct drm_panel *panel = tcon->panel;
struct drm_connector *connector = panel->connector;
struct drm_display_info display_info = connector->display_info;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null
2018-10-03 14:24 ` [PATCH v2 " Giulio Benetti
@ 2018-10-03 14:24 ` Giulio Benetti
2018-10-04 19:56 ` Maxime Ripard
2018-10-04 19:54 ` [PATCH v2 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Maxime Ripard
1 sibling, 1 reply; 19+ messages in thread
From: Giulio Benetti @ 2018-10-03 14:24 UTC (permalink / raw)
To: linux-arm-kernel
If using tcon with VGA, tcon->panel will be null(0), this will cause
segmentation fault when trying to dereference tcon->panel->connector.
Add tcon->panel null check before calling
sun4i_tcon0_mode_set_dithering().
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
RGB565/RGB666 LCD panels")
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
Changes V1->V2:
* none
drivers/gpu/drm/sun4i/sun4i_tcon.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index e4b3bd0307ef..f949287d926c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -491,7 +491,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
sun4i_tcon0_mode_set_common(tcon, mode);
/* Set dithering if needed */
- sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
+ if (tcon->panel)
+ sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null
2018-10-03 14:24 ` [PATCH v2 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
@ 2018-10-04 19:56 ` Maxime Ripard
2018-10-05 21:38 ` Giulio Benetti
2018-10-05 21:59 ` [PATCH v3 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
0 siblings, 2 replies; 19+ messages in thread
From: Maxime Ripard @ 2018-10-04 19:56 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 03, 2018 at 04:24:58PM +0200, Giulio Benetti wrote:
> If using tcon with VGA,
We don't have support for VGA at the moment. Or are you talking about
using a VGA bridge?
> tcon->panel will be null(0), this will cause segmentation fault when
> trying to dereference tcon->panel->connector.
It's not a segmentation fault, but a null pointer dereference. And
that case will also happen with bridges.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181004/fc276c6c/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null
2018-10-04 19:56 ` Maxime Ripard
@ 2018-10-05 21:38 ` Giulio Benetti
2018-10-05 21:59 ` [PATCH v3 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
1 sibling, 0 replies; 19+ messages in thread
From: Giulio Benetti @ 2018-10-05 21:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Il 04/10/2018 21:56, Maxime Ripard ha scritto:
> On Wed, Oct 03, 2018 at 04:24:58PM +0200, Giulio Benetti wrote:
>> If using tcon with VGA,
>
> We don't have support for VGA at the moment. Or are you talking about
> using a VGA bridge?
You're right, in general VGA is not the point.
tcon->panel is retrieved by drm_of_find_panel_or_bridge() and panel can
be present or not.
>> tcon->panel will be null(0), this will cause segmentation fault when
>> trying to dereference tcon->panel->connector.
>
> It's not a segmentation fault, but a null pointer dereference. And
> that case will also happen with bridges.
Right.
Going to improve/rewrite commit logs and submit v2 patchset.
Thanks for reviewing.
Best regards
--
Giulio Benetti
CTO
MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale ? 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
2018-10-04 19:56 ` Maxime Ripard
2018-10-05 21:38 ` Giulio Benetti
@ 2018-10-05 21:59 ` Giulio Benetti
2018-10-05 21:59 ` [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL Giulio Benetti
1 sibling, 1 reply; 19+ messages in thread
From: Giulio Benetti @ 2018-10-05 21:59 UTC (permalink / raw)
To: linux-arm-kernel
Since tcon->panel is a pointer returned by of_drm_find_panel() need to
check if it is not NULL, hence a valid pointer.
IS_ERR() instead checks return error values, not NULL pointers.
Substitute "if (!IS_ERR(tcon->panel))" with "if (tcon->panel)".
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
Changes V1->V2:
* correct same bug for all same occurences in drm/sun4i folder
Changes V2->V3:
* Improve commit log
drivers/gpu/drm/sun4i/sun4i_lvds.c | 4 ++--
drivers/gpu/drm/sun4i/sun4i_rgb.c | 4 ++--
drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c
index af7dcb6da351..e7eb0d1e17be 100644
--- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
+++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
@@ -75,7 +75,7 @@ static void sun4i_lvds_encoder_enable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Enabling LVDS output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_prepare(tcon->panel);
drm_panel_enable(tcon->panel);
}
@@ -88,7 +88,7 @@ static void sun4i_lvds_encoder_disable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Disabling LVDS output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_disable(tcon->panel);
drm_panel_unprepare(tcon->panel);
}
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index bf068da6b12e..f4a22689eb54 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -135,7 +135,7 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Enabling RGB output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_prepare(tcon->panel);
drm_panel_enable(tcon->panel);
}
@@ -148,7 +148,7 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("Disabling RGB output\n");
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
drm_panel_disable(tcon->panel);
drm_panel_unprepare(tcon->panel);
}
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c78cd35a1294..e4b3bd0307ef 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -555,7 +555,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
* Following code is a way to avoid quirks all around TCON
* and DOTCLOCK drivers.
*/
- if (!IS_ERR(tcon->panel)) {
+ if (tcon->panel) {
struct drm_panel *panel = tcon->panel;
struct drm_connector *connector = panel->connector;
struct drm_display_info display_info = connector->display_info;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL
2018-10-05 21:59 ` [PATCH v3 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
@ 2018-10-05 21:59 ` Giulio Benetti
2018-10-08 9:21 ` Maxime Ripard
0 siblings, 1 reply; 19+ messages in thread
From: Giulio Benetti @ 2018-10-05 21:59 UTC (permalink / raw)
To: linux-arm-kernel
If tcon->panel pointer is NULL, trying to dereference from it
(i.e. tcon->panel->connector) will cause a null pointer dereference.
Add tcon->panel null pointer check before calling
sun4i_tcon0_mode_set_dithering().
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
RGB565/RGB666 LCD panels")
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
Changes V1->V2:
* None
Changes V2->V3:
* Rewrite commit log
drivers/gpu/drm/sun4i/sun4i_tcon.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index e4b3bd0307ef..f949287d926c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -491,7 +491,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
sun4i_tcon0_mode_set_common(tcon, mode);
/* Set dithering if needed */
- sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
+ if (tcon->panel)
+ sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL
2018-10-05 21:59 ` [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL Giulio Benetti
@ 2018-10-08 9:21 ` Maxime Ripard
2018-10-12 10:03 ` Chen-Yu Tsai
2018-11-05 13:23 ` Icenowy Zheng
0 siblings, 2 replies; 19+ messages in thread
From: Maxime Ripard @ 2018-10-08 9:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 05, 2018 at 11:59:51PM +0200, Giulio Benetti wrote:
> If tcon->panel pointer is NULL, trying to dereference from it
> (i.e. tcon->panel->connector) will cause a null pointer dereference.
>
> Add tcon->panel null pointer check before calling
> sun4i_tcon0_mode_set_dithering().
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
> RGB565/RGB666 LCD panels")
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Applied both, thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181008/46145777/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL
2018-10-08 9:21 ` Maxime Ripard
@ 2018-10-12 10:03 ` Chen-Yu Tsai
2018-11-05 13:23 ` Icenowy Zheng
1 sibling, 0 replies; 19+ messages in thread
From: Chen-Yu Tsai @ 2018-10-12 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 8, 2018 at 5:21 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Fri, Oct 05, 2018 at 11:59:51PM +0200, Giulio Benetti wrote:
> > If tcon->panel pointer is NULL, trying to dereference from it
> > (i.e. tcon->panel->connector) will cause a null pointer dereference.
> >
> > Add tcon->panel null pointer check before calling
> > sun4i_tcon0_mode_set_dithering().
> >
> > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
> > RGB565/RGB666 LCD panels")
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>
> Applied both, thanks!
I think this one could have been applied to drm-misc-next-fixes,
as the bug was only recently introduced.
We'll have to make sure it gets backported somehow.
ChenYu
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL
2018-10-08 9:21 ` Maxime Ripard
2018-10-12 10:03 ` Chen-Yu Tsai
@ 2018-11-05 13:23 ` Icenowy Zheng
2018-11-06 15:57 ` Maxime Ripard
1 sibling, 1 reply; 19+ messages in thread
From: Icenowy Zheng @ 2018-11-05 13:23 UTC (permalink / raw)
To: linux-arm-kernel
? 2018-10-08?? 11:21 +0200?Maxime Ripard???
> On Fri, Oct 05, 2018 at 11:59:51PM +0200, Giulio Benetti wrote:
> > If tcon->panel pointer is NULL, trying to dereference from it
> > (i.e. tcon->panel->connector) will cause a null pointer
> > dereference.
> >
> > Add tcon->panel null pointer check before calling
> > sun4i_tcon0_mode_set_dithering().
> >
> > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
> > RGB565/RGB666 LCD panels")
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>
> Applied both, thanks!
Please bring them to 4.20.
Currently in 4.20-rc1, bridge support of sun4i-drm is broken because of
this problem.
> Maxime
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v3 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if NULL
2018-11-05 13:23 ` Icenowy Zheng
@ 2018-11-06 15:57 ` Maxime Ripard
0 siblings, 0 replies; 19+ messages in thread
From: Maxime Ripard @ 2018-11-06 15:57 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Nov 05, 2018 at 09:23:08PM +0800, Icenowy Zheng wrote:
> ? 2018-10-08?? 11:21 +0200?Maxime Ripard???
> > On Fri, Oct 05, 2018 at 11:59:51PM +0200, Giulio Benetti wrote:
> > > If tcon->panel pointer is NULL, trying to dereference from it
> > > (i.e. tcon->panel->connector) will cause a null pointer
> > > dereference.
> > >
> > > Add tcon->panel null pointer check before calling
> > > sun4i_tcon0_mode_set_dithering().
> > >
> > > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > > Fixes: f11adcecbd5f ("drm/sun4i: tcon: Add dithering support for
> > > RGB565/RGB666 LCD panels")
> > > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> >
> > Applied both, thanks!
>
> Please bring them to 4.20.
>
> Currently in 4.20-rc1, bridge support of sun4i-drm is broken because of
> this problem.
I've merged them in drm-misc-fixes, thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181106/c369b5f1/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
2018-10-03 14:24 ` [PATCH v2 " Giulio Benetti
2018-10-03 14:24 ` [PATCH v2 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
@ 2018-10-04 19:54 ` Maxime Ripard
1 sibling, 0 replies; 19+ messages in thread
From: Maxime Ripard @ 2018-10-04 19:54 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Oct 03, 2018 at 04:24:57PM +0200, Giulio Benetti wrote:
> At the moment, the check of tcon->panel to be valid is wrong. IS_ERR()
> has been used, but that macro doesn't check if tcon->panel pointer is
> null or not, but check if tcon->panel is between -1 and -4095(MAX_ERRNO).
>
> Remove IS_ERR() from tcon->panel checking and let "if (tcon->panel)" as
> condition to check if it's a pointer not null.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
The commit log should be improved. The issue isn't really with the
IS_ERR macro as you suggest, but that what is returned by
of_drm_find_panel, and thus stored in tcon->panel. is not an error
pointer in the first place but a NULL pointer on error. So the check
doesn't check for the proper thing.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181004/5728b4a0/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread