* [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®