mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: Fix double cleanup in error paths
@ 2026-06-03  8:59 Kory Maincent
  2026-06-03  8:59 ` [PATCH 1/3] drm/i915/display/intel_sdvo: Fix double connector destroy " Kory Maincent
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Kory Maincent @ 2026-06-03  8:59 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	David Airlie, Simona Vetter, Chris Wilson, Eric Anholt,
	Dave Airlie, Jesse Barnes
  Cc: Thomas Petazzoni, Louis Chauvet, Mark Yacoub, Sean Paul,
	intel-gfx, intel-xe, dri-devel, linux-kernel, Simona Vetter,
	Kory Maincent, stable

Several error paths in the i915 driver incorrectly invoke cleanup
functions multiple times, potentially causing double-free errors.
This series corrects these paths to ensure cleanup is performed
only once.

Testing note: Only the DisplayPort fix has been hardware tested due
to lack of available hardware for the other components.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Kory Maincent (3):
      drm/i915/display/intel_sdvo: Fix double connector destroy in error paths
      drm/i915/display/intel_lvds: Drop redundant manual cleanup on init failure
      drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() on init failure

 drivers/gpu/drm/i915/display/intel_dp.c   |  4 +---
 drivers/gpu/drm/i915/display/intel_lvds.c | 14 ++++----------
 drivers/gpu/drm/i915/display/intel_sdvo.c | 16 ++++------------
 3 files changed, 9 insertions(+), 25 deletions(-)
---
base-commit: e1696f1fc99dc0ff761a012230587b23dec064fb
change-id: 20260603-fix_i915-5c3d8c09662a

Best regards,
--  
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] drm/i915/display/intel_sdvo: Fix double connector destroy in error paths
  2026-06-03  8:59 [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
@ 2026-06-03  8:59 ` Kory Maincent
  2026-06-03  8:59 ` [PATCH 2/3] drm/i915/display/intel_lvds: Drop redundant manual cleanup on init failure Kory Maincent
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Kory Maincent @ 2026-06-03  8:59 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	David Airlie, Simona Vetter, Chris Wilson, Eric Anholt,
	Dave Airlie, Jesse Barnes
  Cc: Thomas Petazzoni, Louis Chauvet, Mark Yacoub, Sean Paul,
	intel-gfx, intel-xe, dri-devel, linux-kernel, Simona Vetter,
	Kory Maincent, stable

intel_sdvo_connector_funcs registers intel_connector_destroy() as the
.destroy callback. Once drm_connector_init_with_ddc() succeeds inside
intel_sdvo_connector_init(), the DRM core takes ownership of the
connector object and will call .destroy on teardown.

The error labels in intel_sdvo_tv_init() and intel_sdvo_lvds_init()
call intel_connector_destroy() explicitly before returning false,
causing it to be invoked twice: once in the error path and again by
the DRM core through the registered .destroy callback.

Remove the manual intel_connector_destroy() calls from the error labels
and return false directly instead.

Cc: stable@vger.kernel.org
Fixes: 32aad86fe88e7 ("drm/i915/sdvo: Propagate errors from reading/writing control bus.")
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Not tested as I don't have such hardware.
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index d83d350959d88..0f3aa879e39e1 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2878,16 +2878,12 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, u16 type)
 	}
 
 	if (!intel_sdvo_tv_create_property(intel_sdvo, intel_sdvo_connector, type))
-		goto err;
+		return false;
 
 	if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector))
-		goto err;
+		return false;
 
 	return true;
-
-err:
-	intel_connector_destroy(connector);
-	return false;
 }
 
 static bool
@@ -2950,7 +2946,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, u16 type)
 	}
 
 	if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector))
-		goto err;
+		return false;
 
 	intel_bios_init_panel_late(display, &intel_connector->panel, NULL, NULL);
 
@@ -2972,13 +2968,9 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, u16 type)
 	intel_panel_init(intel_connector, NULL);
 
 	if (!intel_panel_preferred_fixed_mode(intel_connector))
-		goto err;
+		return false;
 
 	return true;
-
-err:
-	intel_connector_destroy(connector);
-	return false;
 }
 
 static u16 intel_sdvo_filter_output_flags(u16 flags)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] drm/i915/display/intel_lvds: Drop redundant manual cleanup on init failure
  2026-06-03  8:59 [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
  2026-06-03  8:59 ` [PATCH 1/3] drm/i915/display/intel_sdvo: Fix double connector destroy " Kory Maincent
@ 2026-06-03  8:59 ` Kory Maincent
  2026-06-03  8:59 ` [PATCH 3/3] drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() " Kory Maincent
  2026-06-03 16:35 ` [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
  3 siblings, 0 replies; 6+ messages in thread
From: Kory Maincent @ 2026-06-03  8:59 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	David Airlie, Simona Vetter, Chris Wilson, Eric Anholt,
	Dave Airlie, Jesse Barnes
  Cc: Thomas Petazzoni, Louis Chauvet, Mark Yacoub, Sean Paul,
	intel-gfx, intel-xe, dri-devel, linux-kernel, Simona Vetter,
	Kory Maincent, stable

intel_lvds_init() had a goto-based error path that manually called
drm_connector_cleanup(), drm_encoder_cleanup(), kfree() and
intel_connector_free() when no LVDS panel mode could be found.

Once drm_connector_init_with_ddc() and drm_encoder_init() have been
called, the DRM core takes ownership of these objects and will invoke
their .destroy callbacks (intel_connector_destroy and
intel_encoder_destroy) during device teardown. The manual cleanup in
the failed: label is therefore redundant.

Remove it and replace the goto with a simple early return.

Cc: stable@vger.kernel.org
Fixes: 79e539453b34e ("DRM: i915: add mode setting support")
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Not tested as I don't have such hardware.
---
 drivers/gpu/drm/i915/display/intel_lvds.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index c8098104d853a..2a1301eda5fe5 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -990,8 +990,10 @@ void intel_lvds_init(struct intel_display *display)
 	mutex_unlock(&display->drm->mode_config.mutex);
 
 	/* If we still don't have a mode after all that, give up. */
-	if (!intel_panel_preferred_fixed_mode(connector))
-		goto failed;
+	if (!intel_panel_preferred_fixed_mode(connector)) {
+		drm_dbg_kms(display->drm, "No LVDS modes found, disabling.\n");
+		return;
+	}
 
 	intel_panel_init(connector, drm_edid);
 
@@ -1004,12 +1006,4 @@ void intel_lvds_init(struct intel_display *display)
 	lvds_encoder->a3_power = lvds & LVDS_A3_POWER_MASK;
 
 	return;
-
-failed:
-	drm_dbg_kms(display->drm, "No LVDS modes found, disabling.\n");
-	drm_connector_cleanup(&connector->base);
-	drm_encoder_cleanup(&encoder->base);
-	kfree(lvds_encoder);
-	intel_connector_free(connector);
-	return;
 }

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() on init failure
  2026-06-03  8:59 [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
  2026-06-03  8:59 ` [PATCH 1/3] drm/i915/display/intel_sdvo: Fix double connector destroy " Kory Maincent
  2026-06-03  8:59 ` [PATCH 2/3] drm/i915/display/intel_lvds: Drop redundant manual cleanup on init failure Kory Maincent
@ 2026-06-03  8:59 ` Kory Maincent
  2026-06-03 11:58   ` Imre Deak
  2026-06-03 16:35 ` [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
  3 siblings, 1 reply; 6+ messages in thread
From: Kory Maincent @ 2026-06-03  8:59 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	David Airlie, Simona Vetter, Chris Wilson, Eric Anholt,
	Dave Airlie, Jesse Barnes
  Cc: Thomas Petazzoni, Louis Chauvet, Mark Yacoub, Sean Paul,
	intel-gfx, intel-xe, dri-devel, linux-kernel, Simona Vetter,
	Kory Maincent, stable

intel_dp_aux_fini() is already invoked via intel_dp_encoder_flush_work()
in the encoder destroy path (intel_dp_encoder_destroy() and
intel_ddi_encoder_destroy()). Calling it explicitly when
intel_edp_init_connector() fails before jumping to the fail label
therefore results in a double invocation. Drop the redundant call.

Cc: stable@vger.kernel.org
Fixes: c191eca110a37 ("drm/i915: Move intel_connector->unregister to connector->early_unregister")
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f01a6eed38395..f4fab568172f4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7310,10 +7310,8 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
 		connector->get_hw_state = intel_connector_get_hw_state;
 	connector->sync_state = intel_dp_connector_sync_state;
 
-	if (!intel_edp_init_connector(intel_dp, connector)) {
-		intel_dp_aux_fini(intel_dp);
+	if (!intel_edp_init_connector(intel_dp, connector))
 		goto fail;
-	}
 
 	intel_dp_set_source_rates(intel_dp);
 	intel_dp_set_common_rates(intel_dp);

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() on init failure
  2026-06-03  8:59 ` [PATCH 3/3] drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() " Kory Maincent
@ 2026-06-03 11:58   ` Imre Deak
  0 siblings, 0 replies; 6+ messages in thread
From: Imre Deak @ 2026-06-03 11:58 UTC (permalink / raw)
  To: Kory Maincent
  Cc: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	David Airlie, Simona Vetter, Chris Wilson, Eric Anholt,
	Dave Airlie, Jesse Barnes, Thomas Petazzoni, Louis Chauvet,
	Mark Yacoub, Sean Paul, intel-gfx, intel-xe, dri-devel,
	linux-kernel, Simona Vetter, stable

On Wed, Jun 03, 2026 at 10:59:54AM +0200, Kory Maincent wrote:
> intel_dp_aux_fini() is already invoked via intel_dp_encoder_flush_work()
> in the encoder destroy path (intel_dp_encoder_destroy() and
> intel_ddi_encoder_destroy()). Calling it explicitly when
> intel_edp_init_connector() fails before jumping to the fail label
> therefore results in a double invocation. Drop the redundant call.

Could you please describe the exact call chain leading to
intel_dp_encoder_flush_work() if intel_dp_init_connector() fails?

> Cc: stable@vger.kernel.org
> Fixes: c191eca110a37 ("drm/i915: Move intel_connector->unregister to connector->early_unregister")
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index f01a6eed38395..f4fab568172f4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7310,10 +7310,8 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
>  		connector->get_hw_state = intel_connector_get_hw_state;
>  	connector->sync_state = intel_dp_connector_sync_state;
>  
> -	if (!intel_edp_init_connector(intel_dp, connector)) {
> -		intel_dp_aux_fini(intel_dp);
> +	if (!intel_edp_init_connector(intel_dp, connector))
>  		goto fail;
> -	}
>  
>  	intel_dp_set_source_rates(intel_dp);
>  	intel_dp_set_common_rates(intel_dp);
> 
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] drm/i915: Fix double cleanup in error paths
  2026-06-03  8:59 [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
                   ` (2 preceding siblings ...)
  2026-06-03  8:59 ` [PATCH 3/3] drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() " Kory Maincent
@ 2026-06-03 16:35 ` Kory Maincent
  3 siblings, 0 replies; 6+ messages in thread
From: Kory Maincent @ 2026-06-03 16:35 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	David Airlie, Simona Vetter, Chris Wilson, Eric Anholt,
	Dave Airlie, Jesse Barnes
  Cc: Thomas Petazzoni, Louis Chauvet, Mark Yacoub, Sean Paul,
	intel-gfx, intel-xe, dri-devel, linux-kernel, Simona Vetter,
	stable

On Wed, 03 Jun 2026 10:59:51 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:

> Several error paths in the i915 driver incorrectly invoke cleanup
> functions multiple times, potentially causing double-free errors.
> This series corrects these paths to ensure cleanup is performed
> only once.
> 
> Testing note: Only the DisplayPort fix has been hardware tested due
> to lack of available hardware for the other components.

Don't pay attention to this series.
I missed the point that if drm_encoder/connector_cleanup() are called it will
remove the encoder/connector from the DRM core list, therefore the destroy
callback will be never called. Which means no double cleanup path.
Sorry for the noise.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-03 16:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-03  8:59 [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent
2026-06-03  8:59 ` [PATCH 1/3] drm/i915/display/intel_sdvo: Fix double connector destroy " Kory Maincent
2026-06-03  8:59 ` [PATCH 2/3] drm/i915/display/intel_lvds: Drop redundant manual cleanup on init failure Kory Maincent
2026-06-03  8:59 ` [PATCH 3/3] drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() " Kory Maincent
2026-06-03 11:58   ` Imre Deak
2026-06-03 16:35 ` [PATCH 0/3] drm/i915: Fix double cleanup in error paths Kory Maincent

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®