[Intel-gfx] [PATCH v2] drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails

[Intel-gfx] [PATCH v2] drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails

[Jani Nikula]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

We enable the DP aux channel during probe, but may free the connector
soon afterwards. Ensure the DP aux display power put (and any other
async put for that matter) is completed before everything is freed, to
prevent a use-after-free in icl_aux_pw_to_phy(), called from
icl_combo_phy_aux_power_well_disable.

v2 by Jani:
- do a regular flush before freeing dig_port

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

v2 of https://patchwork.freedesktop.org/patch/msgid/20221220094618.207126-1-maarten.lankhorst@linux.intel.com

The encoder cleanup paths could use some cleanup and unification, but do
what's needed here.
---
 drivers/gpu/drm/i915/display/g4x_dp.c    | 2 ++
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index 112d91d81fdc..e8147c18fa93 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -1379,6 +1379,8 @@ bool g4x_dp_init(struct drm_i915_private *dev_priv,
 	return true;
 
 err_init_connector:
+	/* aync put accesses the dig_port, ensure it's done before free */
+	intel_display_power_flush_work(dev_priv);
 	drm_encoder_cleanup(encoder);
 err_encoder_init:
 	kfree(intel_connector);
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index d1a9a3cf94b5..dfa1c44dc442 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4971,6 +4971,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	return;
 
 err:
+	/* aync put accesses the dig_port, ensure it's done before free */
+	intel_display_power_flush_work(dev_priv);
 	drm_encoder_cleanup(&encoder->base);
 	kfree(dig_port);
 }
-- 
2.39.2

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails (rev2)

[Patchwork]

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

== Series Details ==

Series: drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails (rev2)
URL   : https://patchwork.freedesktop.org/series/112091/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13202 -> Patchwork_112091v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_112091v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_112091v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/index.html

Participating hosts (38 -> 36)
------------------------------

  Missing    (2): fi-kbl-soraka bat-dg1-5 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_112091v2:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-vga-1:
    - fi-blb-e6850:       [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-vga-1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-vga-1.html

  
Known issues
------------

  Here are the changes found in Patchwork_112091v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-x1275:       [PASS][3] -> [DMESG-FAIL][4] ([i915#5334] / [i915#7872])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-kbl-x1275/igt@i915_selftest@live@gt_heartbeat.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/fi-kbl-x1275/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@guc:
    - bat-rpls-2:         NOTRUN -> [DMESG-WARN][5] ([i915#7852])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-rpls-2/igt@i915_selftest@live@guc.html

  * igt@i915_selftest@live@migrate:
    - bat-rpls-1:         [PASS][6] -> [DMESG-FAIL][7] ([i915#7699])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-rpls-1/igt@i915_selftest@live@migrate.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-rpls-1/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@slpc:
    - bat-adlp-9:         [PASS][8] -> [DMESG-WARN][9] ([i915#6367])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-adlp-9/igt@i915_selftest@live@slpc.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-adlp-9/igt@i915_selftest@live@slpc.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-rpls-2:         NOTRUN -> [ABORT][10] ([i915#6687])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        [SKIP][11] ([fdo#109271]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-hsw-4770/igt@i915_pm_rpm@basic-pci-d3-state.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/fi-hsw-4770/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@basic-rte:
    - fi-hsw-4770:        [FAIL][13] ([i915#7364]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_selftest@live@gt_mocs:
    - {bat-mtlp-8}:       [DMESG-FAIL][15] ([i915#7059]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-2:         [ABORT][17] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#8347]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-rpls-2/igt@i915_selftest@live@reset.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-rpls-2/igt@i915_selftest@live@reset.html

  * igt@i915_selftest@live@slpc:
    - {bat-mtlp-6}:       [DMESG-WARN][19] ([i915#6367]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-mtlp-6/igt@i915_selftest@live@slpc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-mtlp-6/igt@i915_selftest@live@slpc.html
    - {bat-mtlp-8}:       [DMESG-WARN][21] ([i915#6367]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-mtlp-8/igt@i915_selftest@live@slpc.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-mtlp-8/igt@i915_selftest@live@slpc.html
    - bat-rpls-1:         [DMESG-WARN][23] ([i915#6367]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-rpls-1/igt@i915_selftest@live@slpc.html

  
#### Warnings ####

  * igt@kms_psr@sprite_plane_onoff:
    - bat-rplp-1:         [SKIP][25] ([i915#1072]) -> [ABORT][26] ([i915#8442])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7364]: https://gitlab.freedesktop.org/drm/intel/issues/7364
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7852]: https://gitlab.freedesktop.org/drm/intel/issues/7852
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#8189]: https://gitlab.freedesktop.org/drm/intel/issues/8189
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442


Build changes
-------------

  * Linux: CI_DRM_13202 -> Patchwork_112091v2

  CI-20190529: 20190529
  CI_DRM_13202: cb4a9d17f1ae011ad60f6bf502b0c7216d6390d0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7311: c031030f39aff973330668a5a2f1593408da78ae @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112091v2: cb4a9d17f1ae011ad60f6bf502b0c7216d6390d0 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

dc34c06d5e24 drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112091v2/index.html

[-- Attachment #2: Type: text/html, Size: 8656 bytes --]

Re: [Intel-gfx] [PATCH v2] drm/i915/display: Fix a use-after-free when intel_edp_init_connector fails

[Jani Nikula]

On Mon, 29 May 2023, Jani Nikula <jani.nikula@intel.com> wrote:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> We enable the DP aux channel during probe, but may free the connector
> soon afterwards. Ensure the DP aux display power put (and any other
> async put for that matter) is completed before everything is freed, to
> prevent a use-after-free in icl_aux_pw_to_phy(), called from
> icl_combo_phy_aux_power_well_disable.
>
> v2 by Jani:
> - do a regular flush before freeing dig_port
>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> v2 of https://patchwork.freedesktop.org/patch/msgid/20221220094618.207126-1-maarten.lankhorst@linux.intel.com
>
> The encoder cleanup paths could use some cleanup and unification, but do
> what's needed here.

Pushed [1] instead, which I'd missed.

BR,
Jani.


[1] https://patchwork.freedesktop.org/patch/msgid/20221222201804.1380963-1-maarten.lankhorst@linux.intel.com


> ---
>  drivers/gpu/drm/i915/display/g4x_dp.c    | 2 ++
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 112d91d81fdc..e8147c18fa93 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -1379,6 +1379,8 @@ bool g4x_dp_init(struct drm_i915_private *dev_priv,
>  	return true;
>  
>  err_init_connector:
> +	/* aync put accesses the dig_port, ensure it's done before free */
> +	intel_display_power_flush_work(dev_priv);
>  	drm_encoder_cleanup(encoder);
>  err_encoder_init:
>  	kfree(intel_connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index d1a9a3cf94b5..dfa1c44dc442 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4971,6 +4971,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	return;
>  
>  err:
> +	/* aync put accesses the dig_port, ensure it's done before free */
> +	intel_display_power_flush_work(dev_priv);
>  	drm_encoder_cleanup(&encoder->base);
>  	kfree(dig_port);
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center