drm/nv04: fix null pointer dereferences of native_mode

PATCH New Anssi Hannula 1 participant 4 messages Aug 16, 2009 – Aug 23, 2009 2 messages not in the archive

Subsystems (3)

DRM drivers DRM misc nouveau

lore

Share & tools

lore

Anssi Hannula

author

2009-08-16 17:24 UTC #0 lore raw

PATCH 1/1 drm/nv04: fix null pointer dereferences of native_mode

Subject: [PATCH] drm/nv04: fix null pointer dereferences of native_mode

nv_connector->native_mode is not set when no modes were found for the connector, so its existence can't be assumed.

In nv04_dfp_mode_fixup, reject the mode if GPU scaling is enabled but native mode is not known.

In nv04_dfp_mode_set, use clock value from nv_encoder->mode instead of nv_connector->native_mode. If panel scaling is enabled on a TMDS display and the display did not have a valid EDID, native_mode is NULL.

In nv04_lvds_dpms and nv04_dfp_restore, refuse to turn on an LVDS panel if native mode is not known. While clock is not always required for turning panel on, having an LVDS without native mode means something went wrong already, so trying to turn panel on only in cases where clock is required would yield no added benefit.

This fixes http://bugs.freedesktop.org/show_bug.cgi?id=23295

Signed-off-by Anssi Hannula <[email protected]>

---

Please review especially the changes in nv04_dfp_mode_fixup. Previously (2 << 24) | (8 << 28) was set in regp->fp_control with dual link TMDS panel, even if we were using a single link mode with panel scaling. As I didn't know what it is for, I assumed it was a mistake and made it depend on the actual mode (i.e. native_mode with GPU scaling only) instead. Someone who knows this stuff should confirm this or fix it in another way :)

 nv04_dfp.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

drivers/gpu/drm/nouveau/nv04_dfp.c +16 -6

index 7913e5f..9dd4a98 100644
@@ -153,6 +153,8 @@ static bool nv04_dfp_mode_fixup(struct drm_encoder *encoder,
 
 	/* For internal panels and gpu scaling on DVI we need the native mode */
 	if (nv_connector->scaling_mode != DRM_MODE_SCALE_NON_GPU) {
+		if (!nv_connector->native_mode)
+			return false;
 		nv_encoder->mode = *nv_connector->native_mode;
 		adjusted_mode->clock = nv_connector->native_mode->clock;
 	} else {

@@ -305,7 +307,7 @@ static void nv04_dfp_mode_set(struct drm_encoder *encoder,
 	if (nvReadEXTDEV(dev, NV_PEXTDEV_BOOT_0) & NV_PEXTDEV_BOOT_0_STRAP_FP_IFACE_12BIT)
 		regp->fp_control |= NV_PRAMDAC_FP_TG_CONTROL_WIDTH_12;
 	if (nv_encoder->dcb->location != DCB_LOC_ON_CHIP &&
-	    nv_connector->native_mode->clock > 165000)
+	    nv_encoder->mode.clock > 165000)
 		regp->fp_control |= (2 << 24);
 	if (nv_encoder->dcb->type == OUTPUT_LVDS) {
 		bool duallink, dummy;

@@ -315,7 +317,7 @@ static void nv04_dfp_mode_set(struct drm_encoder *encoder,
 		if (duallink)
 			regp->fp_control |= (8 << 28);
 	} else
-	if (nv_connector->native_mode->clock > 165000)
+	if (nv_encoder->mode.clock > 165000)
 		regp->fp_control |= (8 << 28);
 
 	regp->fp_debug_0 = NV_PRAMDAC_FP_DEBUG_0_YWEIGHT_ROUND |

@@ -468,10 +470,14 @@ static void nv04_lvds_dpms(struct drm_encoder *encoder, int mode)
 		int head = crtc ? nouveau_crtc(crtc)->index :
 			   nv04_dfp_get_bound_head(dev, nv_encoder->dcb);
 
-		if (mode == DRM_MODE_DPMS_ON)
+		if (mode == DRM_MODE_DPMS_ON) {
+			if (!nv_connector->native_mode) {
+				NV_ERROR(dev, "Not turning on LVDS without native mode\n");
+				return;
+			}
 			call_lvds_script(dev, nv_encoder->dcb, head,
 					 LVDS_PANEL_ON, nv_connector->native_mode->clock);
-		else
+		} else
 			/* pxclk of 0 is fine for PANEL_OFF, and for a
 			 * disconnected LVDS encoder there is no native_mode
 			 */

@@ -523,8 +529,12 @@ static void nv04_dfp_restore(struct drm_encoder *encoder)
 	int head = nv_encoder->restore.head;
 
 	if (nv_encoder->dcb->type == OUTPUT_LVDS) {
-		call_lvds_script(dev, nv_encoder->dcb, head, LVDS_PANEL_ON,
-				 nouveau_encoder_connector_get(nv_encoder)->native_mode->clock);
+		struct drm_display_mode *native_mode = nouveau_encoder_connector_get(nv_encoder)->native_mode;
+		if (native_mode)
+			call_lvds_script(dev, nv_encoder->dcb, head, LVDS_PANEL_ON,
+					 native_mode->clock);
+		else
+			NV_ERROR(dev, "Not restoring LVDS without native mode\n");
 
 	} else if (nv_encoder->dcb->type == OUTPUT_TMDS) {
 		int clock = nouveau_hw_pllvals_to_clk

Signature · 1 line

Anssi Hannula

Anssi Hannula

author

2009-08-16 19:42 UTC #2 lore raw

PATCH 1/1 drm/nv04: fix null pointer dereferences of native_mode

In reply to a message not in the archive

nv_connector->native_mode is not set when no modes were found for the connector, so its existence can't be assumed.

In nv04_dfp_mode_fixup, reject the mode if GPU scaling is enabled but native mode is not known.

In nv04_dfp_mode_set, use clock value from output_mode (nv_encoder->mode) instead of nv_connector->native_mode. If panel scaling is enabled on a TMDS display and the display did not have a valid EDID, native_mode is NULL.

Signed-off-by Anssi Hannula <[email protected]>

---

Rev2: Use output_mode (&nv_encoder->mode) in nv04_dfp_mode_set; that shortcut was just added by Francisco Jerez so it didn't make it into my initial patch

Please review especially the changes in nv04_dfp_mode_fixup. Previously (2 << 24) | (8 << 28) was set in regp->fp_control with dual link TMDS panel, even if we were using a single link mode with panel scaling. As I didn't know what it is for, I assumed it was a mistake and made it depend on the actual output mode (i.e. native_mode with GPU scaling only) instead. Someone who knows this stuff should confirm this or fix it in another way :)

This fixes up http://bugs.freedesktop.org/show_bug.cgi?id=23295

 nv04_dfp.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

drivers/gpu/drm/nouveau/nv04_dfp.c +16 -6

index 7913e5f..9dd4a98 100644
@@ -153,6 +153,8 @@ static bool nv04_dfp_mode_fixup(struct drm_encoder *encoder,
 
 	/* For internal panels and gpu scaling on DVI we need the native mode */
 	if (nv_connector->scaling_mode != DRM_MODE_SCALE_NON_GPU) {
+		if (!nv_connector->native_mode)
+			return false;
 		nv_encoder->mode = *nv_connector->native_mode;
 		adjusted_mode->clock = nv_connector->native_mode->clock;
 	} else {

@@ -305,7 +307,7 @@ static void nv04_dfp_mode_set(struct drm_encoder *encoder,
 	if (nvReadEXTDEV(dev, NV_PEXTDEV_BOOT_0) & NV_PEXTDEV_BOOT_0_STRAP_FP_IFACE_12BIT)
 		regp->fp_control |= NV_PRAMDAC_FP_TG_CONTROL_WIDTH_12;
 	if (nv_encoder->dcb->location != DCB_LOC_ON_CHIP &&
-	    nv_connector->native_mode->clock > 165000)
+	    output_mode->clock > 165000)
 		regp->fp_control |= (2 << 24);
 	if (nv_encoder->dcb->type == OUTPUT_LVDS) {
 		bool duallink, dummy;

@@ -315,7 +317,7 @@ static void nv04_dfp_mode_set(struct drm_encoder *encoder,
 		if (duallink)
 			regp->fp_control |= (8 << 28);
 	} else
-	if (nv_connector->native_mode->clock > 165000)
+	if (output_mode->clock > 165000)
 		regp->fp_control |= (8 << 28);
 
 	regp->fp_debug_0 = NV_PRAMDAC_FP_DEBUG_0_YWEIGHT_ROUND |

@@ -468,10 +470,14 @@ static void nv04_lvds_dpms(struct drm_encoder *encoder, int mode)
 		int head = crtc ? nouveau_crtc(crtc)->index :
 			   nv04_dfp_get_bound_head(dev, nv_encoder->dcb);
 
-		if (mode == DRM_MODE_DPMS_ON)
+		if (mode == DRM_MODE_DPMS_ON) {
+			if (!nv_connector->native_mode) {
+				NV_ERROR(dev, "Not turning on LVDS without native mode\n");
+				return;
+			}
 			call_lvds_script(dev, nv_encoder->dcb, head,
 					 LVDS_PANEL_ON, nv_connector->native_mode->clock);
-		else
+		} else
 			/* pxclk of 0 is fine for PANEL_OFF, and for a
 			 * disconnected LVDS encoder there is no native_mode
 			 */

@@ -523,8 +529,12 @@ static void nv04_dfp_restore(struct drm_encoder *encoder)
 	int head = nv_encoder->restore.head;
 
 	if (nv_encoder->dcb->type == OUTPUT_LVDS) {
-		call_lvds_script(dev, nv_encoder->dcb, head, LVDS_PANEL_ON,
-				 nouveau_encoder_connector_get(nv_encoder)->native_mode->clock);
+		struct drm_display_mode *native_mode = nouveau_encoder_connector_get(nv_encoder)->native_mode;
+		if (native_mode)
+			call_lvds_script(dev, nv_encoder->dcb, head, LVDS_PANEL_ON,
+					 native_mode->clock);
+		else
+			NV_ERROR(dev, "Not restoring LVDS without native mode\n");
 
 	} else if (nv_encoder->dcb->type == OUTPUT_TMDS) {
 		int clock = nouveau_hw_pllvals_to_clk

Signature · 1 line

Anssi Hannula

Anssi Hannula

author

2009-08-17 04:11 UTC #4 lore raw

In reply to a message not in the archive

Quoted 5 lines · Anssi Hannula

In nv04_lvds_dpms and nv04_dfp_restore, refuse to turn on an LVDS panel if native mode is not known. While clock is not always required for turning panel on, having an LVDS without native mode means something went wrong already, so trying to turn panel on only in cases where clock is required would yield no added benefit.

^^^^^^^^
I meant "not required" :)

Signature · 1 line

Anssi Hannula

Anssi Hannula

author

2009-08-23 23:37 UTC #5 lore raw

In reply to a message not in the archive

Quoted 32 lines · Anssi Hannula

nv_connector->native_mode is not set when no modes were found for the connector, so its existence can't be assumed.
In nv04_dfp_mode_fixup, reject the mode if GPU scaling is enabled but native mode is not known.
In nv04_dfp_mode_set, use clock value from output_mode (nv_encoder->mode) instead of nv_connector->native_mode. If panel scaling is enabled on a TMDS display and the display did not have a valid EDID, native_mode is NULL.
In nv04_lvds_dpms and nv04_dfp_restore, refuse to turn on an LVDS panel if native mode is not known. While clock is not always required for turning panel on, having an LVDS without native mode means something went wrong already, so trying to turn panel on only in cases where clock is required would yield no added benefit.
Signed-off-by Anssi Hannula <[email protected]>
---
Rev2: Use output_mode (&nv_encoder->mode) in nv04_dfp_mode_set; that shortcut was just added by Francisco Jerez so it didn't make it into my initial patch
Please review especially the changes in nv04_dfp_mode_fixup. Previously (2 << 24) | (8 << 28) was set in regp->fp_control with dual link TMDS panel, even if we were using a single link mode with panel scaling. As I didn't know what it is for, I assumed it was a mistake and made it depend on the actual output mode (i.e. native_mode with GPU scaling only) instead. Someone who knows this stuff should confirm this or fix it in another way :)

Quoted 1 line

This fixes up http://bugs.freedesktop.org/show_bug.cgi?id=23295

Ping?

Signature · 1 line

Anssi Hannula