Fixup drm-misc vs i915->unload()

Fixup drm-misc vs i915->unload()

[Chris Wilson]

In order to quieten the warning about out-of-order unload, we need to
move our connector unregistrations actions from our custom callback to
earlier when the core unregister the connectors.

The change from earlier is that I missed a vital patch to prevent a
use-after-free during DP connector destroy.
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915/dp: Free the drm_dp_aux along with the encoder

[Chris Wilson]

The drm_dp_ax object is stored on the encoder, and freeing it from the
connector causes a use-after-free error since the encoder is destroy
first:

[  112.356952] ==================================================================
[  112.357065] BUG: KASAN: use-after-free in intel_dp_connector_destroy+0x68/0xb0 [i915] at addr ffff880386960168
[  112.357130] Read of size 8 by task rmmod/6293
[  112.357159] =============================================================================
[  112.357221] BUG kmalloc-8192 (Tainted: G        W   E  ): kasan: bad access detected
[  112.357268] -----------------------------------------------------------------------------
[  112.357268]
[  112.357333] Disabling lock debugging due to kernel taint
[  112.357362] INFO: Allocated in 0xffff8803869642a8 age=18446744052234668220 cpu=0 pid=0
[  112.357472] 	intel_ddi_init+0xea/0x540 [i915]
[  112.357502] 	___slab_alloc+0x4a3/0x530
[  112.357529] 	__slab_alloc+0x4c/0x90
[  112.357571] 	kmem_cache_alloc+0x180/0x1c0
[  112.357657] 	intel_ddi_init+0xea/0x540 [i915]
[  112.357739] 	intel_modeset_init+0x1e51/0x2150 [i915]
[  112.357806] 	__kms_init_async+0x33/0x50 [i915]
[  112.357880] 	do_initcall_async+0x6e/0xa0 [i915]
[  112.357918] 	async_run_entry_fn+0x60/0x230
[  112.357947] 	process_one_work+0x315/0x6d0
[  112.357989] 	worker_thread+0x86/0x780
[  112.358017] 	kthread+0x141/0x160
[  112.358065] 	ret_from_fork+0x1f/0x40
[  112.358111] INFO: Freed in 0xfffef6db age=18446717049775278270 cpu=2173493056 pid=-1
[  112.358212] 	intel_dp_encoder_destroy+0x2e/0xa0 [i915]
[  112.358242] 	__slab_free+0x17a/0x310
[  112.358268] 	kfree+0x164/0x170
[  112.358361] 	intel_dp_encoder_destroy+0x2e/0xa0 [i915]
[  112.358395] 	drm_mode_config_cleanup+0x63/0x370
[  112.358481] 	intel_modeset_cleanup+0x65/0x90 [i915]
[  112.358550] 	i915_driver_unload+0xbf/0x380 [i915]
[  112.358624] 	i915_pci_remove+0x23/0x30 [i915]
[  112.358663] 	pci_device_remove+0x5c/0x110
[  112.358691] 	__device_release_driver+0xd6/0x1e0
[  112.358734] 	driver_detach+0x112/0x120
[  112.358761] 	bus_remove_driver+0x93/0x160
[  112.358803] 	driver_unregister+0x3e/0x70
[  112.358832] 	pci_unregister_driver+0x24/0xd0
[  112.358932] 	i915_exit+0x1a/0x88c [i915]
[  112.358961] 	SyS_delete_module+0x20a/0x250

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be083519dac9..3ffedf472f35 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4496,6 +4496,9 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 			intel_dp->edp_notifier.notifier_call = NULL;
 		}
 	}
+
+	intel_dp_aux_fini(intel_dp);
+
 	drm_encoder_cleanup(encoder);
 	kfree(intel_dig_port);
 }
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Move intel_connector->unregister to connector->early_unregister

[Chris Wilson]

We now have a connector->func that serves the same purpose as our own
intel_connector->unregister vfunc allowing us to unwrap ourselves and
use drm_connector_register() (and friends) as the central function.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +-
 drivers/gpu/drm/i915/intel_crt.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 10 ++--------
 drivers/gpu/drm/i915/intel_dp.c      | 21 ++++++++++-----------
 drivers/gpu/drm/i915/intel_dp_mst.c  |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h     |  8 --------
 drivers/gpu/drm/i915/intel_dsi.c     |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c     |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c    | 26 +++++++++++---------------
 drivers/gpu/drm/i915/intel_tv.c      |  2 +-
 12 files changed, 32 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9fa9698fe247..e1cb102f2806 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3704,7 +3704,7 @@ extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
-extern void intel_connector_unregister(struct intel_connector *);
+extern void intel_connector_unregister(struct drm_connector *);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void intel_display_resume(struct drm_device *dev);
 extern void i915_redisable_vga(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 9465de4135aa..e115bcc6766f 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -743,6 +743,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_crt_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_crt_destroy,
 	.set_property = intel_crt_set_property,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -875,7 +876,6 @@ void intel_crt_init(struct drm_device *dev)
 		crt->base.get_hw_state = intel_crt_get_hw_state;
 	}
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
-	intel_connector->unregister = intel_connector_unregister;
 
 	drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 029456cfc57e..6cad3ccc2752 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16313,18 +16313,14 @@ void intel_modeset_gem_init(struct drm_device *dev)
 	intel_backlight_register(dev);
 }
 
-void intel_connector_unregister(struct intel_connector *intel_connector)
+void intel_connector_unregister(struct drm_connector *connector)
 {
-	struct drm_connector *connector = &intel_connector->base;
-
 	intel_panel_destroy_backlight(connector);
-	drm_connector_unregister(connector);
 }
 
 void intel_modeset_cleanup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_connector *connector;
 
 	intel_disable_gt_powersave(dev_priv);
 
@@ -16350,9 +16346,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	/* flush any delayed tasks or pending work */
 	flush_scheduled_work();
 
-	/* destroy the backlight and sysfs files before encoders/connectors */
-	for_each_intel_connector(dev, connector)
-		connector->unregister(connector);
+	drm_connector_unregister_all(dev);
 
 	drm_mode_config_cleanup(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3ffedf472f35..17de1499af4a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1177,7 +1177,6 @@ static void intel_aux_reg_init(struct intel_dp *intel_dp)
 static void
 intel_dp_aux_fini(struct intel_dp *intel_dp)
 {
-	drm_dp_aux_unregister(&intel_dp->aux);
 	kfree(intel_dp->aux.name);
 }
 
@@ -1212,15 +1211,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	return 0;
 }
 
-static void
-intel_dp_connector_unregister(struct intel_connector *intel_connector)
-{
-	struct intel_dp *intel_dp = intel_attached_dp(&intel_connector->base);
-
-	intel_dp_aux_fini(intel_dp);
-	intel_connector_unregister(intel_connector);
-}
-
 static int
 intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
 {
@@ -4457,6 +4447,13 @@ done:
 }
 
 static void
+intel_dp_connector_unregister(struct drm_connector *connector)
+{
+	drm_dp_aux_unregister(&intel_attached_dp(connector)->aux);
+	intel_connector_unregister(connector);
+}
+
+static void
 intel_dp_connector_destroy(struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
@@ -4466,6 +4463,8 @@ intel_dp_connector_destroy(struct drm_connector *connector)
 	if (!IS_ERR_OR_NULL(intel_connector->edid))
 		kfree(intel_connector->edid);
 
+	intel_dp_aux_fini(intel_attached_dp(connector));
+
 	/* Can't call is_edp() since the encoder may have been destroyed
 	 * already. */
 	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
@@ -4575,6 +4574,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.early_unregister = intel_dp_connector_unregister,
 	.destroy = intel_dp_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -5490,7 +5490,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
 	else
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
-	intel_connector->unregister = intel_dp_connector_unregister;
 
 	/* Set up the hotplug pin. */
 	switch (port) {
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index f62ca9a126b3..9646816604be 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -336,6 +336,7 @@ static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_mst_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dp_mst_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -455,7 +456,6 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	drm_connector_init(dev, connector, &intel_dp_mst_connector_funcs, DRM_MODE_CONNECTOR_DisplayPort);
 	drm_connector_helper_add(connector, &intel_dp_mst_connector_helper_funcs);
 
-	intel_connector->unregister = intel_connector_unregister;
 	intel_connector->get_hw_state = intel_dp_mst_get_hw_state;
 	intel_connector->mst_port = intel_dp;
 	intel_connector->port = port;
@@ -489,7 +489,7 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_device *dev = connector->dev;
 
-	intel_connector->unregister(intel_connector);
+	drm_connector_unregister(connector);
 
 	/* need to nuke the connector */
 	drm_modeset_lock_all(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0c1dc9bae170..adb6b9eb9f2f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -242,14 +242,6 @@ struct intel_connector {
 	 * and active (i.e. dpms ON state). */
 	bool (*get_hw_state)(struct intel_connector *);
 
-	/*
-	 * Removes all interfaces through which the connector is accessible
-	 * - like sysfs, debugfs entries -, so that no new operations can be
-	 * started on the connector. Also makes sure all currently pending
-	 * operations finish before returing.
-	 */
-	void (*unregister)(struct intel_connector *);
-
 	/* Panel info for eDP and LVDS */
 	struct intel_panel panel;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 10e12829b13f..b444d0e35a98 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1390,6 +1390,7 @@ static const struct drm_connector_helper_funcs intel_dsi_connector_helper_funcs
 static const struct drm_connector_funcs intel_dsi_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_dsi_detect,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dsi_connector_destroy,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dsi_set_property,
@@ -1466,7 +1467,6 @@ void intel_dsi_init(struct drm_device *dev)
 	intel_encoder->get_config = intel_dsi_get_config;
 
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
-	intel_connector->unregister = intel_connector_unregister;
 
 	/*
 	 * On BYT/CHV, pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index c86f88ed92fd..60e4ddf2ec6d 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -341,6 +341,7 @@ static void intel_dvo_destroy(struct drm_connector *connector)
 static const struct drm_connector_funcs intel_dvo_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_dvo_detect,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dvo_destroy,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_get_property = intel_connector_atomic_get_property,
@@ -447,7 +448,6 @@ void intel_dvo_init(struct drm_device *dev)
 	intel_encoder->compute_config = intel_dvo_compute_config;
 	intel_encoder->pre_enable = intel_dvo_pre_enable;
 	intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
-	intel_connector->unregister = intel_connector_unregister;
 
 	/* Now, try to find a controller */
 	for (i = 0; i < ARRAY_SIZE(intel_dvo_devices); i++) {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ad0c71b3e242..fb21626ada64 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1774,6 +1774,7 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_hdmi_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_hdmi_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -1909,7 +1910,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
 	else
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
-	intel_connector->unregister = intel_connector_unregister;
 
 	intel_hdmi_add_properties(intel_hdmi, connector);
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index e06b9036bebc..e9082185a375 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -555,6 +555,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_lvds_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_lvds_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -991,7 +992,6 @@ void intel_lvds_init(struct drm_device *dev)
 	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
 	intel_encoder->get_config = intel_lvds_get_config;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
-	intel_connector->unregister = intel_connector_unregister;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	intel_encoder->type = INTEL_OUTPUT_LVDS;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index ab2d0658abe6..02b4a6695528 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2177,12 +2177,23 @@ done:
 #undef CHECK_PROPERTY
 }
 
+static void
+intel_sdvo_connector_unregister(struct drm_connector *connector)
+{
+	struct intel_sdvo *sdvo = intel_attached_sdvo(connector);
+
+	sysfs_remove_link(&connector->kdev->kobj,
+			  sdvo->ddc.dev.kobj.name);
+	intel_connector_unregister(connector);
+}
+
 static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_sdvo_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_sdvo_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.early_unregister = intel_sdvo_connector_unregister,
 	.destroy = intel_sdvo_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -2345,20 +2356,6 @@ intel_sdvo_get_slave_addr(struct drm_device *dev, struct intel_sdvo *sdvo)
 		return 0x72;
 }
 
-static void
-intel_sdvo_connector_unregister(struct intel_connector *intel_connector)
-{
-	struct drm_connector *drm_connector;
-	struct intel_sdvo *sdvo_encoder;
-
-	drm_connector = &intel_connector->base;
-	sdvo_encoder = intel_attached_sdvo(&intel_connector->base);
-
-	sysfs_remove_link(&drm_connector->kdev->kobj,
-			  sdvo_encoder->ddc.dev.kobj.name);
-	intel_connector_unregister(intel_connector);
-}
-
 static int
 intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 			  struct intel_sdvo *encoder)
@@ -2381,7 +2378,6 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 	connector->base.base.doublescan_allowed = 0;
 	connector->base.base.display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->base.get_hw_state = intel_sdvo_connector_get_hw_state;
-	connector->base.unregister = intel_sdvo_connector_unregister;
 
 	intel_connector_attach_encoder(&connector->base, &encoder->base);
 	ret = drm_connector_register(drm_connector);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 7ac9e9b0e2c3..4ce70a9f9df2 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1501,6 +1501,7 @@ out:
 static const struct drm_connector_funcs intel_tv_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_tv_detect,
+	.early_unregister = intel_connector_unregister,
 	.destroy = intel_tv_destroy,
 	.set_property = intel_tv_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
@@ -1599,7 +1600,6 @@ intel_tv_init(struct drm_device *dev)
 	intel_encoder->disable = intel_disable_tv;
 	intel_encoder->get_hw_state = intel_tv_get_hw_state;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
-	intel_connector->unregister = intel_connector_unregister;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	intel_encoder->type = INTEL_OUTPUT_TVOUT;
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Move backlight unregistration to connector unregistration

[Chris Wilson]

Currently the backlight is being unregistered in the unload phase (after
the display and its objects are unregistered). Move the backlight
unregistration into the analogous phase by performing it from the
connector unregistration, just prior to its deletion.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  5 +++--
 drivers/gpu/drm/i915/intel_drv.h     |  9 ++++++++-
 drivers/gpu/drm/i915/intel_panel.c   | 13 +------------
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6cad3ccc2752..0b2cd669ac05 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16315,6 +16315,9 @@ void intel_modeset_gem_init(struct drm_device *dev)
 
 void intel_connector_unregister(struct drm_connector *connector)
 {
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+
+	intel_backlight_device_unregister(intel_connector);
 	intel_panel_destroy_backlight(connector);
 }
 
@@ -16324,8 +16327,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_disable_gt_powersave(dev_priv);
 
-	intel_backlight_unregister(dev);
-
 	/*
 	 * Interrupts and polling as the first thing to avoid creating havoc.
 	 * Too much stuff here (turning of connectors, ...) would
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index adb6b9eb9f2f..7d0e071fe355 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1501,7 +1501,14 @@ extern struct drm_display_mode *intel_find_panel_downclock(
 				struct drm_display_mode *fixed_mode,
 				struct drm_connector *connector);
 void intel_backlight_register(struct drm_device *dev);
-void intel_backlight_unregister(struct drm_device *dev);
+
+#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+void intel_backlight_device_unregister(struct intel_connector *connector);
+#else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+static inline void intel_backlight_device_unregister(struct intel_connector *connector)
+{
+}
+#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
 
 
 /* intel_psr.c */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index f0b1602c3258..bf721781c259 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1216,7 +1216,7 @@ static int intel_backlight_device_register(struct intel_connector *connector)
 	return 0;
 }
 
-static void intel_backlight_device_unregister(struct intel_connector *connector)
+void intel_backlight_device_unregister(struct intel_connector *connector)
 {
 	struct intel_panel *panel = &connector->panel;
 
@@ -1230,9 +1230,6 @@ static int intel_backlight_device_register(struct intel_connector *connector)
 {
 	return 0;
 }
-static void intel_backlight_device_unregister(struct intel_connector *connector)
-{
-}
 #endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
 
 /*
@@ -1820,11 +1817,3 @@ void intel_backlight_register(struct drm_device *dev)
 	for_each_intel_connector(dev, connector)
 		intel_backlight_device_register(connector);
 }
-
-void intel_backlight_unregister(struct drm_device *dev)
-{
-	struct intel_connector *connector;
-
-	for_each_intel_connector(dev, connector)
-		intel_backlight_device_unregister(connector);
-}
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/dp: Free the drm_dp_aux along with the encoder

[Daniel Vetter]

On Fri, Jun 17, 2016 at 04:53:55PM +0100, Chris Wilson wrote:
> The drm_dp_ax object is stored on the encoder, and freeing it from the
> connector causes a use-after-free error since the encoder is destroy
> first:
> 
> [  112.356952] ==================================================================
> [  112.357065] BUG: KASAN: use-after-free in intel_dp_connector_destroy+0x68/0xb0 [i915] at addr ffff880386960168
> [  112.357130] Read of size 8 by task rmmod/6293
> [  112.357159] =============================================================================
> [  112.357221] BUG kmalloc-8192 (Tainted: G        W   E  ): kasan: bad access detected
> [  112.357268] -----------------------------------------------------------------------------
> [  112.357268]
> [  112.357333] Disabling lock debugging due to kernel taint
> [  112.357362] INFO: Allocated in 0xffff8803869642a8 age=18446744052234668220 cpu=0 pid=0
> [  112.357472] 	intel_ddi_init+0xea/0x540 [i915]
> [  112.357502] 	___slab_alloc+0x4a3/0x530
> [  112.357529] 	__slab_alloc+0x4c/0x90
> [  112.357571] 	kmem_cache_alloc+0x180/0x1c0
> [  112.357657] 	intel_ddi_init+0xea/0x540 [i915]
> [  112.357739] 	intel_modeset_init+0x1e51/0x2150 [i915]
> [  112.357806] 	__kms_init_async+0x33/0x50 [i915]
> [  112.357880] 	do_initcall_async+0x6e/0xa0 [i915]
> [  112.357918] 	async_run_entry_fn+0x60/0x230
> [  112.357947] 	process_one_work+0x315/0x6d0
> [  112.357989] 	worker_thread+0x86/0x780
> [  112.358017] 	kthread+0x141/0x160
> [  112.358065] 	ret_from_fork+0x1f/0x40
> [  112.358111] INFO: Freed in 0xfffef6db age=18446717049775278270 cpu=2173493056 pid=-1
> [  112.358212] 	intel_dp_encoder_destroy+0x2e/0xa0 [i915]
> [  112.358242] 	__slab_free+0x17a/0x310
> [  112.358268] 	kfree+0x164/0x170
> [  112.358361] 	intel_dp_encoder_destroy+0x2e/0xa0 [i915]
> [  112.358395] 	drm_mode_config_cleanup+0x63/0x370
> [  112.358481] 	intel_modeset_cleanup+0x65/0x90 [i915]
> [  112.358550] 	i915_driver_unload+0xbf/0x380 [i915]
> [  112.358624] 	i915_pci_remove+0x23/0x30 [i915]
> [  112.358663] 	pci_device_remove+0x5c/0x110
> [  112.358691] 	__device_release_driver+0xd6/0x1e0
> [  112.358734] 	driver_detach+0x112/0x120
> [  112.358761] 	bus_remove_driver+0x93/0x160
> [  112.358803] 	driver_unregister+0x3e/0x70
> [  112.358832] 	pci_unregister_driver+0x24/0xd0
> [  112.358932] 	i915_exit+0x1a/0x88c [i915]
> [  112.358961] 	SyS_delete_module+0x20a/0x250
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index be083519dac9..3ffedf472f35 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4496,6 +4496,9 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  			intel_dp->edp_notifier.notifier_call = NULL;
>  		}
>  	}
> +
> +	intel_dp_aux_fini(intel_dp);

Seems to miss the hunk to remove it from intel_dp_connector_unregister?
-Daniel

> +
>  	drm_encoder_cleanup(encoder);
>  	kfree(intel_dig_port);
>  }
> -- 
> 2.8.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/dp: Free the drm_dp_aux along with the encoder

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/dp: Free the drm_dp_aux along with the encoder
URL   : https://patchwork.freedesktop.org/series/8847/
State : failure

== Summary ==

Applying: drm/i915/dp: Free the drm_dp_aux along with the encoder
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_dp.c
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: drm/i915: Move intel_connector->unregister to connector->early_unregister
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.h
M	drivers/gpu/drm/i915/intel_crt.c
M	drivers/gpu/drm/i915/intel_display.c
M	drivers/gpu/drm/i915/intel_dp.c
M	drivers/gpu/drm/i915/intel_dp_mst.c
M	drivers/gpu/drm/i915/intel_drv.h
M	drivers/gpu/drm/i915/intel_dsi.c
M	drivers/gpu/drm/i915/intel_dvo.c
M	drivers/gpu/drm/i915/intel_hdmi.c
M	drivers/gpu/drm/i915/intel_lvds.c
M	drivers/gpu/drm/i915/intel_sdvo.c
M	drivers/gpu/drm/i915/intel_tv.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_dp.c
Auto-merging drivers/gpu/drm/i915/intel_display.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_display.c
error: Failed to merge in the changes.
Patch failed at 0002 drm/i915: Move intel_connector->unregister to connector->early_unregister
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx