From: Jani Nikula <jani.nikula@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Move cec_notifier to intel_connector
Date: Tue, 28 Jan 2020 10:55:04 +0200 [thread overview]
Message-ID: <87tv4g9d07.fsf@intel.com> (raw)
In-Reply-To: <20200128083851.2566893-1-maarten.lankhorst@linux.intel.com>
On Tue, 28 Jan 2020, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> This fixes the following KASAN splash on module reload:
Fixes: ?
Likely pretty old, and warrants Cc: stable. Which makes me slightly
unhappy about the drive-by cleanups not strictly part of the fix.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> [ 145.136327] ==================================================================
> [ 145.136502] BUG: KASAN: use-after-free in intel_hdmi_destroy+0x74/0x80 [i915]
> [ 145.136514] Read of size 8 at addr ffff888216641830 by task kworker/1:1/134
>
> [ 145.136535] CPU: 1 PID: 134 Comm: kworker/1:1 Tainted: G U T 5.5.0-rc7-valkyria+ #5783
> [ 145.136539] Hardware name: GIGABYTE GB-BKi3A-7100/MFLP3AP-00, BIOS F1 07/27/2016
> [ 145.136546] Workqueue: events drm_connector_free_work_fn
> [ 145.136551] Call Trace:
> [ 145.136560] dump_stack+0xa1/0xe0
> [ 145.136571] print_address_description.constprop.0+0x1e/0x210
> [ 145.136639] ? intel_hdmi_destroy+0x74/0x80 [i915]
> [ 145.136703] ? intel_hdmi_destroy+0x74/0x80 [i915]
> [ 145.136710] __kasan_report.cold+0x1b/0x37
> [ 145.136790] ? intel_hdmi_destroy+0x74/0x80 [i915]
> [ 145.136863] ? intel_hdmi_destroy+0x74/0x80 [i915]
> [ 145.136870] kasan_report+0x27/0x30
> [ 145.136881] __asan_report_load8_noabort+0x1c/0x20
> [ 145.136946] intel_hdmi_destroy+0x74/0x80 [i915]
> [ 145.136954] drm_connector_free_work_fn+0xd1/0x100
> [ 145.136967] process_one_work+0x86e/0x1610
> [ 145.136987] ? pwq_dec_nr_in_flight+0x2f0/0x2f0
> [ 145.137004] ? move_linked_works+0x128/0x2c0
> [ 145.137021] worker_thread+0x63e/0xc90
> [ 145.137048] kthread+0x2f6/0x3f0
> [ 145.137054] ? calculate_sigpending+0x81/0xa0
> [ 145.137059] ? process_one_work+0x1610/0x1610
> [ 145.137064] ? kthread_bind+0x40/0x40
> [ 145.137075] ret_from_fork+0x24/0x30
>
> [ 145.137111] Allocated by task 0:
> [ 145.137119] (stack is not available)
>
> [ 145.137137] Freed by task 5053:
> [ 145.137147] save_stack+0x28/0x90
> [ 145.137152] __kasan_slab_free+0x136/0x180
> [ 145.137157] kasan_slab_free+0x26/0x30
> [ 145.137161] kfree+0xe6/0x350
> [ 145.137242] intel_ddi_encoder_destroy+0x60/0x80 [i915]
> [ 145.137252] drm_mode_config_cleanup+0x11d/0x8f0
> [ 145.137329] intel_modeset_driver_remove+0x1f5/0x350 [i915]
> [ 145.137403] i915_driver_remove+0xc4/0x130 [i915]
> [ 145.137482] i915_pci_remove+0x3e/0x90 [i915]
> [ 145.137489] pci_device_remove+0x108/0x2d0
> [ 145.137494] device_release_driver_internal+0x1e6/0x4a0
> [ 145.137499] driver_detach+0xcb/0x198
> [ 145.137503] bus_remove_driver+0xde/0x204
> [ 145.137508] driver_unregister+0x6d/0xa0
> [ 145.137513] pci_unregister_driver+0x2e/0x230
> [ 145.137576] i915_exit+0x1f/0x26 [i915]
> [ 145.137157] kasan_slab_free+0x26/0x30
> [ 145.137161] kfree+0xe6/0x350
> [ 145.137242] intel_ddi_encoder_destroy+0x60/0x80 [i915]
> [ 145.137252] drm_mode_config_cleanup+0x11d/0x8f0
> [ 145.137329] intel_modeset_driver_remove+0x1f5/0x350 [i915]
> [ 145.137403] i915_driver_remove+0xc4/0x130 [i915]
> [ 145.137482] i915_pci_remove+0x3e/0x90 [i915]
> [ 145.137489] pci_device_remove+0x108/0x2d0
> [ 145.137494] device_release_driver_internal+0x1e6/0x4a0
> [ 145.137499] driver_detach+0xcb/0x198
> [ 145.137503] bus_remove_driver+0xde/0x204
> [ 145.137508] driver_unregister+0x6d/0xa0
> [ 145.137513] pci_unregister_driver+0x2e/0x230
> [ 145.137576] i915_exit+0x1f/0x26 [i915]
> [ 145.137581] __x64_sys_delete_module+0x35b/0x470
> [ 145.137586] do_syscall_64+0x99/0x4e0
> [ 145.137591] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> [ 145.137606] The buggy address belongs to the object at ffff888216640000
> which belongs to the cache kmalloc-8k of size 8192
> [ 145.137618] The buggy address is located 6192 bytes inside of
> 8192-byte region [ffff888216640000, ffff888216642000)
> [ 145.137630] The buggy address belongs to the page:
> [ 145.137640] page:ffffea0008599000 refcount:1 mapcount:0 mapping:ffff888107c02a80 index:0xffff888216644000 compound_mapcount: 0
> [ 145.137647] raw: 0200000000010200 0000000000000000 0000000100000001 ffff888107c02a80
> [ 145.137652] raw: ffff888216644000 0000000080020001 00000001ffffffff 0000000000000000
> [ 145.137656] page dumped because: kasan: bad access detected
>
> [ 145.137668] Memory state around the buggy address:
> [ 145.137678] ffff888216641700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ 145.137687] ffff888216641780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ 145.137697] >ffff888216641800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ 145.137706] ^
> [ 145.137715] ffff888216641880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ 145.137724] ffff888216641900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ 145.137733] ==================================================================
> [ 145.137742] Disabling lock debugging due to kernel taint
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> .../gpu/drm/i915/display/intel_connector.c | 2 +
> .../drm/i915/display/intel_display_types.h | 3 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 63 +++++++++----------
> 3 files changed, 32 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> index 54891a4ed2f3..28ac292d3455 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -99,6 +99,8 @@ void intel_connector_destroy(struct drm_connector *connector)
> if (!IS_ERR_OR_NULL(intel_connector->edid))
> kfree(intel_connector->edid);
>
> + cec_notifier_conn_unregister(intel_connector->cec_notifier);
> +
> intel_panel_fini(&intel_connector->panel);
>
> drm_connector_cleanup(connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 33ba93863488..231077b883cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -434,6 +434,8 @@ struct intel_connector {
> struct work_struct modeset_retry_work;
>
> struct intel_hdcp hdcp;
> +
> + struct cec_notifier *cec_notifier;
> };
>
> struct intel_digital_connector_state {
> @@ -1181,7 +1183,6 @@ struct intel_hdmi {
> bool has_hdmi_sink;
> bool has_audio;
> struct intel_connector *attached_connector;
> - struct cec_notifier *cec_notifier;
> };
>
> struct intel_dp_mst_encoder;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index ab13cf834cc4..730a0241434c 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2504,9 +2504,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
> }
>
> static void
> -intel_hdmi_unset_edid(struct drm_connector *connector)
> +intel_hdmi_unset_edid(struct intel_connector *connector)
> {
> - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
> + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>
> intel_hdmi->has_hdmi_sink = false;
> intel_hdmi->has_audio = false;
> @@ -2514,15 +2514,15 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
> intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
> intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
>
> - kfree(to_intel_connector(connector)->detect_edid);
> - to_intel_connector(connector)->detect_edid = NULL;
> + kfree(connector->detect_edid);
> + connector->detect_edid = NULL;
> }
>
> static void
> -intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool has_edid)
> +intel_hdmi_dp_dual_mode_detect(struct intel_connector *connector, bool has_edid)
> {
> - struct drm_i915_private *dev_priv = to_i915(connector->dev);
> - struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
> enum port port = hdmi_to_dig_port(hdmi)->base.port;
> struct i2c_adapter *adapter =
> intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
> @@ -2544,7 +2544,7 @@ intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool has_edid)
> /* An overridden EDID imply that we want this port for testing.
> * Make sure not to set limits for that port.
> */
> - if (has_edid && !connector->override_edid &&
> + if (has_edid && !connector->base.override_edid &&
> intel_bios_is_port_dp_dual_mode(dev_priv, port)) {
> DRM_DEBUG_KMS("Assuming DP dual mode adaptor presence based on VBT\n");
> type = DRM_DP_DUAL_MODE_TYPE1_DVI;
> @@ -2566,10 +2566,10 @@ intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool has_edid)
> }
>
> static bool
> -intel_hdmi_set_edid(struct drm_connector *connector)
> +intel_hdmi_set_edid(struct intel_connector *connector)
> {
> - struct drm_i915_private *dev_priv = to_i915(connector->dev);
> - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> intel_wakeref_t wakeref;
> struct edid *edid;
> bool connected = false;
> @@ -2579,12 +2579,12 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>
> i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
>
> - edid = drm_get_edid(connector, i2c);
> + edid = drm_get_edid(&connector->base, i2c);
>
> if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> intel_gmbus_force_bit(i2c, true);
> - edid = drm_get_edid(connector, i2c);
> + edid = drm_get_edid(&connector->base, i2c);
> intel_gmbus_force_bit(i2c, false);
> }
>
> @@ -2592,7 +2592,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>
> intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
>
> - to_intel_connector(connector)->detect_edid = edid;
> + connector->detect_edid = edid;
> if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
> intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
> @@ -2600,22 +2600,23 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> connected = true;
> }
>
> - cec_notifier_set_phys_addr_from_edid(intel_hdmi->cec_notifier, edid);
> + cec_notifier_set_phys_addr_from_edid(connector->cec_notifier, edid);
>
> return connected;
> }
>
> static enum drm_connector_status
> -intel_hdmi_detect(struct drm_connector *connector, bool force)
> +intel_hdmi_detect(struct drm_connector *_connector, bool force)
> {
> enum drm_connector_status status = connector_status_disconnected;
> - struct drm_i915_private *dev_priv = to_i915(connector->dev);
> - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
> + struct intel_connector *connector = to_intel_connector(_connector);
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct intel_encoder *encoder = &hdmi_to_dig_port(intel_hdmi)->base;
> intel_wakeref_t wakeref;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> - connector->base.id, connector->name);
> + connector->base.base.id, connector->base.name);
>
> wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> @@ -2632,7 +2633,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
>
> if (status != connector_status_connected)
> - cec_notifier_phys_addr_invalidate(intel_hdmi->cec_notifier);
> + cec_notifier_phys_addr_invalidate(connector->cec_notifier);
>
> /*
> * Make sure the refs for power wells enabled during detect are
> @@ -2644,14 +2645,15 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> }
>
> static void
> -intel_hdmi_force(struct drm_connector *connector)
> +intel_hdmi_force(struct drm_connector *_connector)
> {
> + struct intel_connector *connector = to_intel_connector(_connector);
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> - connector->base.id, connector->name);
> + connector->base.base.id, connector->base.name);
>
> intel_hdmi_unset_edid(connector);
>
> - if (connector->status != connector_status_connected)
> + if (connector->base.status != connector_status_connected)
> return;
>
> intel_hdmi_set_edid(connector);
> @@ -2824,15 +2826,6 @@ intel_hdmi_connector_register(struct drm_connector *connector)
> return ret;
> }
>
> -static void intel_hdmi_destroy(struct drm_connector *connector)
> -{
> - struct cec_notifier *n = intel_attached_hdmi(to_intel_connector(connector))->cec_notifier;
> -
> - cec_notifier_conn_unregister(n);
> -
> - intel_connector_destroy(connector);
> -}
> -
> static void intel_hdmi_connector_unregister(struct drm_connector *connector)
> {
> intel_hdmi_remove_i2c_symlink(connector);
> @@ -2848,7 +2841,7 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
> .atomic_set_property = intel_digital_connector_atomic_set_property,
> .late_register = intel_hdmi_connector_register,
> .early_unregister = intel_hdmi_connector_unregister,
> - .destroy = intel_hdmi_destroy,
> + .destroy = intel_connector_destroy,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> .atomic_duplicate_state = intel_digital_connector_duplicate_state,
> };
> @@ -3204,10 +3197,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>
> cec_fill_conn_info_from_drm(&conn_info, connector);
>
> - intel_hdmi->cec_notifier =
> + intel_connector->cec_notifier =
> cec_notifier_conn_register(dev->dev, port_identifier(port),
> &conn_info);
> - if (!intel_hdmi->cec_notifier)
> + if (!intel_connector->cec_notifier)
> DRM_DEBUG_KMS("CEC notifier get failed\n");
> }
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-01-28 8:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-28 8:38 [Intel-gfx] [PATCH] drm/i915: Move cec_notifier to intel_connector Maarten Lankhorst
2020-01-28 8:55 ` Jani Nikula [this message]
2020-01-28 18:06 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2020-01-30 4:34 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tv4g9d07.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.