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 <[email protected]>

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 <[email protected]>
> Cc: Dhinakaran Pandiyan <[email protected]>
> Cc: Manasi Navare <[email protected]>
> ---
>  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");
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to