diff mbox

drm/i915: Prevent kernel panic on reading out compliance debugfs files

Message ID 20170621105941.11617-1-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst June 21, 2017, 10:59 a.m. UTC
When reading all debugfs files on a system with DP-MST the kernel panics
on a null pointer dereference because intel_dp is null for a DP-MST
connector. Detect this case and skip those connectors.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

Comments

Dhinakaran Pandiyan June 21, 2017, 8:57 p.m. UTC | #1
On Wed, 2017-06-21 at 12:59 +0200, Maarten Lankhorst wrote:
> When reading all debugfs files on a system with DP-MST the kernel panics

> on a null pointer dereference because intel_dp is null for a DP-MST

> connector. Detect this case and skip those connectors.

> 

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> ---

>  drivers/gpu/drm/i915/i915_debugfs.c | 33 ++++++++++++++++++++++++---------

>  1 file changed, 24 insertions(+), 9 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c

> index 1f1176b6400e..30e2e168fe92 100644

> --- a/drivers/gpu/drm/i915/i915_debugfs.c

> +++ b/drivers/gpu/drm/i915/i915_debugfs.c


I think you missed i915_displayport_test_active_write(). Looks good
otherwise.

-DK



> @@ -3812,13 +3812,18 @@ static int i915_displayport_test_active_show(struct seq_file *m, void *data)

>  

>  	drm_connector_list_iter_begin(dev, &conn_iter);

>  	drm_for_each_connector_iter(connector, &conn_iter) {

> +		struct intel_encoder *encoder;

> +

>  		if (connector->connector_type !=

>  		    DRM_MODE_CONNECTOR_DisplayPort)

>  			continue;

>  

> -		if (connector->status == connector_status_connected &&

> -		    connector->encoder != NULL) {

> -			intel_dp = enc_to_intel_dp(connector->encoder);

> +		encoder = to_intel_encoder(connector->encoder);

> +		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)

> +			continue;

> +

> +		if (encoder && connector->status == connector_status_connected) {

> +			intel_dp = enc_to_intel_dp(&encoder->base);

>  			if (intel_dp->compliance.test_active)

>  				seq_puts(m, "1");

>  			else

> @@ -3858,13 +3863,18 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)

>  

>  	drm_connector_list_iter_begin(dev, &conn_iter);

>  	drm_for_each_connector_iter(connector, &conn_iter) {

> +		struct intel_encoder *encoder;

> +

>  		if (connector->connector_type !=

>  		    DRM_MODE_CONNECTOR_DisplayPort)

>  			continue;

>  

> -		if (connector->status == connector_status_connected &&

> -		    connector->encoder != NULL) {

> -			intel_dp = enc_to_intel_dp(connector->encoder);

> +		encoder = to_intel_encoder(connector->encoder);

> +		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)

> +			continue;

> +

> +		if (encoder && connector->status == connector_status_connected) {

> +			intel_dp = enc_to_intel_dp(&encoder->base);

>  			if (intel_dp->compliance.test_type ==

>  			    DP_TEST_LINK_EDID_READ)

>  				seq_printf(m, "%lx",

> @@ -3911,13 +3921,18 @@ static int i915_displayport_test_type_show(struct seq_file *m, void *data)

>  

>  	drm_connector_list_iter_begin(dev, &conn_iter);

>  	drm_for_each_connector_iter(connector, &conn_iter) {

> +		struct intel_encoder *encoder;

> +

>  		if (connector->connector_type !=

>  		    DRM_MODE_CONNECTOR_DisplayPort)

>  			continue;

>  

> -		if (connector->status == connector_status_connected &&

> -		    connector->encoder != NULL) {

> -			intel_dp = enc_to_intel_dp(connector->encoder);

> +		encoder = to_intel_encoder(connector->encoder);

> +		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)

> +			continue;

> +

> +		if (encoder && connector->status == connector_status_connected) {

> +			intel_dp = enc_to_intel_dp(&encoder->base);

>  			seq_printf(m, "%02lx", intel_dp->compliance.test_type);

>  		} else

>  			seq_puts(m, "0");
Navare, Manasi June 21, 2017, 10:08 p.m. UTC | #2
Thanks fo rthis patch Maarten. I had noticed this while running the intel_dp_compliance
IGT tool. But I saw this with DP SST systems as well. What could be happening in that case?

Manasi

On Wed, Jun 21, 2017 at 12:59:41PM +0200, Maarten Lankhorst wrote:
> When reading all debugfs files on a system with DP-MST the kernel panics
> on a null pointer dereference because intel_dp is null for a DP-MST
> connector. Detect this case and skip those connectors.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1f1176b6400e..30e2e168fe92 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3812,13 +3812,18 @@ static int i915_displayport_test_active_show(struct seq_file *m, void *data)
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
>  	drm_for_each_connector_iter(connector, &conn_iter) {
> +		struct intel_encoder *encoder;
> +
>  		if (connector->connector_type !=
>  		    DRM_MODE_CONNECTOR_DisplayPort)
>  			continue;
>  
> -		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> -			intel_dp = enc_to_intel_dp(connector->encoder);
> +		encoder = to_intel_encoder(connector->encoder);
> +		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)
> +			continue;
> +
> +		if (encoder && connector->status == connector_status_connected) {
> +			intel_dp = enc_to_intel_dp(&encoder->base);
>  			if (intel_dp->compliance.test_active)
>  				seq_puts(m, "1");
>  			else
> @@ -3858,13 +3863,18 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
>  	drm_for_each_connector_iter(connector, &conn_iter) {
> +		struct intel_encoder *encoder;
> +
>  		if (connector->connector_type !=
>  		    DRM_MODE_CONNECTOR_DisplayPort)
>  			continue;
>  
> -		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> -			intel_dp = enc_to_intel_dp(connector->encoder);
> +		encoder = to_intel_encoder(connector->encoder);
> +		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)
> +			continue;
> +
> +		if (encoder && connector->status == connector_status_connected) {
> +			intel_dp = enc_to_intel_dp(&encoder->base);
>  			if (intel_dp->compliance.test_type ==
>  			    DP_TEST_LINK_EDID_READ)
>  				seq_printf(m, "%lx",
> @@ -3911,13 +3921,18 @@ static int i915_displayport_test_type_show(struct seq_file *m, void *data)
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
>  	drm_for_each_connector_iter(connector, &conn_iter) {
> +		struct intel_encoder *encoder;
> +
>  		if (connector->connector_type !=
>  		    DRM_MODE_CONNECTOR_DisplayPort)
>  			continue;
>  
> -		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> -			intel_dp = enc_to_intel_dp(connector->encoder);
> +		encoder = to_intel_encoder(connector->encoder);
> +		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)
> +			continue;
> +
> +		if (encoder && connector->status == connector_status_connected) {
> +			intel_dp = enc_to_intel_dp(&encoder->base);
>  			seq_printf(m, "%02lx", intel_dp->compliance.test_type);
>  		} else
>  			seq_puts(m, "0");
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1f1176b6400e..30e2e168fe92 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3812,13 +3812,18 @@  static int i915_displayport_test_active_show(struct seq_file *m, void *data)
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
+		struct intel_encoder *encoder;
+
 		if (connector->connector_type !=
 		    DRM_MODE_CONNECTOR_DisplayPort)
 			continue;
 
-		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
-			intel_dp = enc_to_intel_dp(connector->encoder);
+		encoder = to_intel_encoder(connector->encoder);
+		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)
+			continue;
+
+		if (encoder && connector->status == connector_status_connected) {
+			intel_dp = enc_to_intel_dp(&encoder->base);
 			if (intel_dp->compliance.test_active)
 				seq_puts(m, "1");
 			else
@@ -3858,13 +3863,18 @@  static int i915_displayport_test_data_show(struct seq_file *m, void *data)
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
+		struct intel_encoder *encoder;
+
 		if (connector->connector_type !=
 		    DRM_MODE_CONNECTOR_DisplayPort)
 			continue;
 
-		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
-			intel_dp = enc_to_intel_dp(connector->encoder);
+		encoder = to_intel_encoder(connector->encoder);
+		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)
+			continue;
+
+		if (encoder && connector->status == connector_status_connected) {
+			intel_dp = enc_to_intel_dp(&encoder->base);
 			if (intel_dp->compliance.test_type ==
 			    DP_TEST_LINK_EDID_READ)
 				seq_printf(m, "%lx",
@@ -3911,13 +3921,18 @@  static int i915_displayport_test_type_show(struct seq_file *m, void *data)
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
+		struct intel_encoder *encoder;
+
 		if (connector->connector_type !=
 		    DRM_MODE_CONNECTOR_DisplayPort)
 			continue;
 
-		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
-			intel_dp = enc_to_intel_dp(connector->encoder);
+		encoder = to_intel_encoder(connector->encoder);
+		if (encoder && encoder->type == INTEL_OUTPUT_DP_MST)
+			continue;
+
+		if (encoder && connector->status == connector_status_connected) {
+			intel_dp = enc_to_intel_dp(&encoder->base);
 			seq_printf(m, "%02lx", intel_dp->compliance.test_type);
 		} else
 			seq_puts(m, "0");