[Intel-gfx] [PATCH] drm/i915: Prevent kernel panic when reading/writing compliance debugfs files, v2.

Pandiyan, Dhinakaran dhinakaran.pandiyan at intel.com
Mon Jun 26 17:07:59 UTC 2017




On Mon, 2017-06-26 at 10:18 +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.
> 
> Also fix the write for the DP compliance file in the same way.
> 
> Changes since v1:
> - Fix i915_displayport_test_active_write too. (DK)
> 

Avoids NULL pointer dereferences in all instances of enc_to_intel_dp()
inside compliance related functions, so 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>

This is the second instance where enc_to_intel_dp(mst_encoder) has
caused kernel panics. I wonder if we should change enc_to_intel_dp() to
check for encoder type.


-DK


> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Cc: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 44 +++++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f7aa6cbe3a2e..76b07a9b3548 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3778,13 +3778,18 @@ static ssize_t i915_displayport_test_active_write(struct file *file,
>  
>  	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);
>  			status = kstrtoint(input_buffer, 10, &val);
>  			if (status < 0)
>  				break;
> @@ -3816,13 +3821,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
> @@ -3862,13 +3872,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",
> @@ -3915,13 +3930,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");


More information about the Intel-gfx mailing list