linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
       [not found] <20181001093612.GA13672@mwanda>
@ 2018-10-02 20:48 ` Giulio Benetti
  2018-10-02 20:49   ` [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
  2018-10-02 21:59 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
  1 sibling, 1 reply; 19+ messages in thread
From: Giulio Benetti @ 2018-10-02 20:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter, Giulio Benetti

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;
-- 
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 20:48 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
@ 2018-10-02 20:49   ` Giulio Benetti
  0 siblings, 0 replies; 19+ messages in thread
From: Giulio Benetti @ 2018-10-02 20:49 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter, Giulio Benetti

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 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer
       [not found] <20181001093612.GA13672@mwanda>
  2018-10-02 20:48 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
@ 2018-10-02 21:59 ` Giulio Benetti
  2018-10-02 21:59   ` [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
                     ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Giulio Benetti @ 2018-10-02 21:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter, Giulio Benetti

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;
-- 
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 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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter, Giulio Benetti

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

* Re: [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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter

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

* Re: [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: Giulio Benetti
  Cc: Maxime Ripard, David Airlie, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter

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

* Re: [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: Giulio Benetti
  Cc: Maxime Ripard, David Airlie, dri-devel, linux-arm-kernel,
	linux-kernel, Dan Carpenter

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

* Re: [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: Chen-Yu Tsai
  Cc: Maxime Ripard, linux-kernel, dri-devel, David Airlie,
	linux-arm-kernel, Dan Carpenter

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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Giulio Benetti

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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Giulio Benetti

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

* Re: [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: Giulio Benetti
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [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: Giulio Benetti
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, linux-arm-kernel, dri-devel, linux-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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Giulio Benetti

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: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Giulio Benetti

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

* Re: [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: Giulio Benetti
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [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: Maxime Ripard
  Cc: Giulio Benetti, David Airlie, dri-devel, linux-arm-kernel, linux-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

* Re: [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: Maxime Ripard, Giulio Benetti
  Cc: David Airlie, Chen-Yu Tsai, linux-arm-kernel, dri-devel, linux-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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [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: Icenowy Zheng
  Cc: Giulio Benetti, David Airlie, Chen-Yu Tsai, linux-arm-kernel,
	dri-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-11-06 15:57 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20181001093612.GA13672@mwanda>
2018-10-02 20:48 ` [PATCH 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Giulio Benetti
2018-10-02 20:49   ` [PATCH 2/2] drm/sun4i: tcon: prevent tcon->panel dereference if null Giulio Benetti
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-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
2018-10-03 12:13     ` Giulio Benetti
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: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
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
2018-11-06 15:57                     ` Maxime Ripard
2018-10-04 19:54         ` [PATCH v2 1/2] drm/sun4i: tcon: fix check of tcon->panel null pointer Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).