linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/nouveau: Fix connector memory corruption issues
@ 2018-07-13 16:24 Lyude Paul
  2018-07-13 16:24 ` [PATCH 1/2] drm/nouveau: Use drm_connector_list_iter_* for iterating connectors Lyude Paul
  2018-07-13 16:24 ` [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors Lyude Paul
  0 siblings, 2 replies; 4+ messages in thread
From: Lyude Paul @ 2018-07-13 16:24 UTC (permalink / raw)
  To: nouveau
  Cc: Karol Herbst, stable, David Airlie, linux-kernel, dri-devel, Ben Skeggs

This fixes some nasty issues I found in nouveau that were being caused
looping through connectors using racy legacy methods, along with some
caused by making incorrect assumptions about the drm_connector structs
in nouveau's connector list. Most of these memory corruption issues
could be reproduced by using an MST hub with nouveau.

Cc: Karol Herbst <karolherbst@gmail.com>
Cc: stable@vger.kernel.org

Lyude Paul (2):
  drm/nouveau: Use drm_connector_list_iter_* for iterating ues connectors
  drm/nouveau: Avoid looping through fake MST connectors

 drivers/gpu/drm/nouveau/nouveau_backlight.c |  6 ++--
 drivers/gpu/drm/nouveau/nouveau_connector.c |  9 ++++--
 drivers/gpu/drm/nouveau/nouveau_connector.h | 36 ++++++++++++++++++---
 drivers/gpu/drm/nouveau/nouveau_display.c   | 10 ++++--
 4 files changed, 51 insertions(+), 10 deletions(-)

-- 
2.17.1


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

* [PATCH 1/2] drm/nouveau: Use drm_connector_list_iter_* for iterating connectors
  2018-07-13 16:24 [PATCH 0/2] drm/nouveau: Fix connector memory corruption issues Lyude Paul
@ 2018-07-13 16:24 ` Lyude Paul
  2018-07-13 16:24 ` [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors Lyude Paul
  1 sibling, 0 replies; 4+ messages in thread
From: Lyude Paul @ 2018-07-13 16:24 UTC (permalink / raw)
  To: nouveau
  Cc: stable, Karol Herbst, Ben Skeggs, David Airlie, dri-devel, linux-kernel

Every codepath in nouveau that loops through the connector list
currently does so using the old method, which is prone to race
conditions from MST connectors being created and destroyed. This has
been causing a multitude of problems, including memory corruption from
trying to access connectors that have already been freed!

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
Cc: Karol Herbst <karolherbst@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c |  6 ++++--
 drivers/gpu/drm/nouveau/nouveau_connector.c |  9 +++++++--
 drivers/gpu/drm/nouveau/nouveau_connector.h | 14 ++++++++++----
 drivers/gpu/drm/nouveau/nouveau_display.c   | 10 ++++++++--
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index debbbf0fd4bd..408b955e5c39 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -267,6 +267,7 @@ nouveau_backlight_init(struct drm_device *dev)
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nvif_device *device = &drm->client.device;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
 	INIT_LIST_HEAD(&drm->bl_connectors);
 
@@ -275,7 +276,8 @@ nouveau_backlight_init(struct drm_device *dev)
 		return 0;
 	}
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
 		    connector->connector_type != DRM_MODE_CONNECTOR_eDP)
 			continue;
@@ -292,7 +294,7 @@ nouveau_backlight_init(struct drm_device *dev)
 			break;
 		}
 	}
-
+	drm_connector_list_iter_end(&conn_iter);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 7b557c354307..7dc380449232 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1208,14 +1208,19 @@ nouveau_connector_create(struct drm_device *dev, int index)
 	struct nouveau_display *disp = nouveau_display(dev);
 	struct nouveau_connector *nv_connector = NULL;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 	int type, ret = 0;
 	bool dummy;
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		nv_connector = nouveau_connector(connector);
-		if (nv_connector->index == index)
+		if (nv_connector->index == index) {
+			drm_connector_list_iter_end(&conn_iter);
 			return connector;
+		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	nv_connector = kzalloc(sizeof(*nv_connector), GFP_KERNEL);
 	if (!nv_connector)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h
index a4d1a059bd3d..a8cbb4b56fc7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -65,14 +65,20 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
 {
 	struct drm_device *dev = nv_crtc->base.dev;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	struct nouveau_connector *nv_connector = NULL;
 	struct drm_crtc *crtc = to_drm_crtc(nv_crtc);
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		if (connector->encoder && connector->encoder->crtc == crtc)
-			return nouveau_connector(connector);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		if (connector->encoder && connector->encoder->crtc == crtc) {
+			nv_connector = nouveau_connector(connector);
+			break;
+		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
-	return NULL;
+	return nv_connector;
 }
 
 struct drm_connector *
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 774b429142bc..46b8430ef4aa 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -404,6 +404,7 @@ nouveau_display_init(struct drm_device *dev)
 	struct nouveau_display *disp = nouveau_display(dev);
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 	int ret;
 
 	ret = disp->init(dev);
@@ -411,10 +412,12 @@ nouveau_display_init(struct drm_device *dev)
 		return ret;
 
 	/* enable hotplug interrupts */
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct nouveau_connector *conn = nouveau_connector(connector);
 		nvif_notify_get(&conn->hpd);
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	/* enable flip completion events */
 	nvif_notify_get(&drm->flip);
@@ -427,6 +430,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend)
 	struct nouveau_display *disp = nouveau_display(dev);
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
 	if (!suspend) {
 		if (drm_drv_uses_atomic_modeset(dev))
@@ -439,10 +443,12 @@ nouveau_display_fini(struct drm_device *dev, bool suspend)
 	nvif_notify_put(&drm->flip);
 
 	/* disable hotplug interrupts */
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct nouveau_connector *conn = nouveau_connector(connector);
 		nvif_notify_put(&conn->hpd);
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	drm_kms_helper_poll_disable(dev);
 	disp->fini(dev);
-- 
2.17.1


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

* [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors
  2018-07-13 16:24 [PATCH 0/2] drm/nouveau: Fix connector memory corruption issues Lyude Paul
  2018-07-13 16:24 ` [PATCH 1/2] drm/nouveau: Use drm_connector_list_iter_* for iterating connectors Lyude Paul
@ 2018-07-13 16:24 ` Lyude Paul
  2018-07-13 16:45   ` [Nouveau] " Daniel Vetter
  1 sibling, 1 reply; 4+ messages in thread
From: Lyude Paul @ 2018-07-13 16:24 UTC (permalink / raw)
  To: nouveau
  Cc: stable, Karol Herbst, Ben Skeggs, David Airlie, dri-devel, linux-kernel

When MST and atomic were introduced to nouveau, another structure that
could contain a drm_connector embedded within it was introduced; struct
nv50_mstc. This meant that we no longer would be able to simply loop
through our connector list and assume that nouveau_connector() would
return a proper pointer for each connector, since the assertion that
all connectors coming from nouveau have a full nouveau_connector struct
became invalid.

Unfortunately, none of the actual code that looped through connectors
ever got updated, which means that we've been causing invalid memory
accesses for quite a while now.

An example that was caught by KASAN:

[  201.038698] ==================================================================
[  201.038792] BUG: KASAN: slab-out-of-bounds in nvif_notify_get+0x190/0x1a0 [nouveau]
[  201.038797] Read of size 4 at addr ffff88076738c650 by task kworker/0:3/718
[  201.038800]
[  201.038822] CPU: 0 PID: 718 Comm: kworker/0:3 Tainted: G           O      4.18.0-rc4Lyude-Test+ #1
[  201.038825] Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET78W (1.51 ) 05/18/2018
[  201.038882] Workqueue: events nouveau_display_hpd_work [nouveau]
[  201.038887] Call Trace:
[  201.038894]  dump_stack+0xa4/0xfd
[  201.038900]  print_address_description+0x71/0x239
[  201.038929]  ? nvif_notify_get+0x190/0x1a0 [nouveau]
[  201.038935]  kasan_report.cold.6+0x242/0x2fe
[  201.038942]  __asan_report_load4_noabort+0x19/0x20
[  201.038970]  nvif_notify_get+0x190/0x1a0 [nouveau]
[  201.038998]  ? nvif_notify_put+0x1f0/0x1f0 [nouveau]
[  201.039003]  ? kmsg_dump_rewind_nolock+0xe4/0xe4
[  201.039049]  nouveau_display_init.cold.12+0x34/0x39 [nouveau]
[  201.039089]  ? nouveau_user_framebuffer_create+0x120/0x120 [nouveau]
[  201.039133]  nouveau_display_resume+0x5c0/0x810 [nouveau]
[  201.039173]  ? nvkm_client_ioctl+0x20/0x20 [nouveau]
[  201.039215]  nouveau_do_resume+0x19f/0x570 [nouveau]
[  201.039256]  nouveau_pmops_runtime_resume+0xd8/0x2a0 [nouveau]
[  201.039264]  pci_pm_runtime_resume+0x130/0x250
[  201.039269]  ? pci_restore_standard_config+0x70/0x70
[  201.039275]  __rpm_callback+0x1f2/0x5d0
[  201.039279]  ? rpm_resume+0x560/0x18a0
[  201.039283]  ? pci_restore_standard_config+0x70/0x70
[  201.039287]  ? pci_restore_standard_config+0x70/0x70
[  201.039291]  ? pci_restore_standard_config+0x70/0x70
[  201.039296]  rpm_callback+0x175/0x210
[  201.039300]  ? pci_restore_standard_config+0x70/0x70
[  201.039305]  rpm_resume+0xcc3/0x18a0
[  201.039312]  ? rpm_callback+0x210/0x210
[  201.039317]  ? __pm_runtime_resume+0x9e/0x100
[  201.039322]  ? kasan_check_write+0x14/0x20
[  201.039326]  ? do_raw_spin_lock+0xc2/0x1c0
[  201.039333]  __pm_runtime_resume+0xac/0x100
[  201.039374]  nouveau_display_hpd_work+0x67/0x1f0 [nouveau]
[  201.039380]  process_one_work+0x7a0/0x14d0
[  201.039388]  ? cancel_delayed_work_sync+0x20/0x20
[  201.039392]  ? lock_acquire+0x113/0x310
[  201.039398]  ? kasan_check_write+0x14/0x20
[  201.039402]  ? do_raw_spin_lock+0xc2/0x1c0
[  201.039409]  worker_thread+0x86/0xb50
[  201.039418]  kthread+0x2e9/0x3a0
[  201.039422]  ? process_one_work+0x14d0/0x14d0
[  201.039426]  ? kthread_create_worker_on_cpu+0xc0/0xc0
[  201.039431]  ret_from_fork+0x3a/0x50
[  201.039441]
[  201.039444] Allocated by task 79:
[  201.039449]  save_stack+0x43/0xd0
[  201.039452]  kasan_kmalloc+0xc4/0xe0
[  201.039456]  kmem_cache_alloc_trace+0x10a/0x260
[  201.039494]  nv50_mstm_add_connector+0x9a/0x340 [nouveau]
[  201.039504]  drm_dp_add_port+0xff5/0x1fc0 [drm_kms_helper]
[  201.039511]  drm_dp_send_link_address+0x4a7/0x740 [drm_kms_helper]
[  201.039518]  drm_dp_check_and_send_link_address+0x1a7/0x210 [drm_kms_helper]
[  201.039525]  drm_dp_mst_link_probe_work+0x71/0xb0 [drm_kms_helper]
[  201.039529]  process_one_work+0x7a0/0x14d0
[  201.039533]  worker_thread+0x86/0xb50
[  201.039537]  kthread+0x2e9/0x3a0
[  201.039541]  ret_from_fork+0x3a/0x50
[  201.039543]
[  201.039546] Freed by task 0:
[  201.039549] (stack is not available)
[  201.039551]
[  201.039555] The buggy address belongs to the object at ffff88076738c1a8
                                 which belongs to the cache kmalloc-2048 of size 2048
[  201.039559] The buggy address is located 1192 bytes inside of
                                 2048-byte region [ffff88076738c1a8, ffff88076738c9a8)
[  201.039563] The buggy address belongs to the page:
[  201.039567] page:ffffea001d9ce200 count:1 mapcount:0 mapping:ffff88084000d0c0 index:0x0 compound_mapcount: 0
[  201.039573] flags: 0x8000000000008100(slab|head)
[  201.039578] raw: 8000000000008100 ffffea001da3be08 ffffea001da25a08 ffff88084000d0c0
[  201.039582] raw: 0000000000000000 00000000000d000d 00000001ffffffff 0000000000000000
[  201.039585] page dumped because: kasan: bad access detected
[  201.039588]
[  201.039591] Memory state around the buggy address:
[  201.039594]  ffff88076738c500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  201.039598]  ffff88076738c580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  201.039601] >ffff88076738c600: 00 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc
[  201.039604]                                                  ^
[  201.039607]  ffff88076738c680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[  201.039611]  ffff88076738c700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[  201.039613] ==================================================================

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
Cc: Karol Herbst <karolherbst@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.h | 24 ++++++++++++++++++++-
 drivers/gpu/drm/nouveau/nouveau_display.c   |  4 ++--
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 7dc380449232..af68eae4c626 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1213,7 +1213,7 @@ nouveau_connector_create(struct drm_device *dev, int index)
 	bool dummy;
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
+	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
 		nv_connector = nouveau_connector(connector);
 		if (nv_connector->index == index) {
 			drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h
index a8cbb4b56fc7..aaf3a213c685 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -33,6 +33,7 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_dp_helper.h>
 #include "nouveau_crtc.h"
+#include "nouveau_encoder.h"
 
 struct nvkm_i2c_port;
 
@@ -60,6 +61,27 @@ static inline struct nouveau_connector *nouveau_connector(
 	return container_of(con, struct nouveau_connector, base);
 }
 
+static inline bool
+nouveau_connector_is_mst(struct drm_connector *connector)
+{
+	const struct nouveau_encoder *nv_encoder;
+	const struct drm_encoder *encoder;
+
+	if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
+		return false;
+
+	nv_encoder = find_encoder(connector, DCB_OUTPUT_ANY);
+	if (!nv_encoder)
+		return false;
+
+	encoder = &nv_encoder->base.base;
+	return encoder->encoder_type == DRM_MODE_ENCODER_DPMST;
+}
+
+#define nouveau_for_each_non_mst_connector_iter(connector, iter) \
+	drm_for_each_connector_iter(connector, iter) \
+		if (!nouveau_connector_is_mst(connector))
+
 static inline struct nouveau_connector *
 nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
 {
@@ -70,7 +92,7 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
 	struct drm_crtc *crtc = to_drm_crtc(nv_crtc);
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
+	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
 		if (connector->encoder && connector->encoder->crtc == crtc) {
 			nv_connector = nouveau_connector(connector);
 			break;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 46b8430ef4aa..ec7861457b84 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -413,7 +413,7 @@ nouveau_display_init(struct drm_device *dev)
 
 	/* enable hotplug interrupts */
 	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
+	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
 		struct nouveau_connector *conn = nouveau_connector(connector);
 		nvif_notify_get(&conn->hpd);
 	}
@@ -444,7 +444,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend)
 
 	/* disable hotplug interrupts */
 	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
+	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
 		struct nouveau_connector *conn = nouveau_connector(connector);
 		nvif_notify_put(&conn->hpd);
 	}
-- 
2.17.1


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

* Re: [Nouveau] [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors
  2018-07-13 16:24 ` [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors Lyude Paul
@ 2018-07-13 16:45   ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2018-07-13 16:45 UTC (permalink / raw)
  To: Lyude Paul
  Cc: nouveau, David Airlie, linux-kernel, dri-devel, stable, Ben Skeggs

On Fri, Jul 13, 2018 at 12:24:41PM -0400, Lyude Paul wrote:
> When MST and atomic were introduced to nouveau, another structure that
> could contain a drm_connector embedded within it was introduced; struct
> nv50_mstc. This meant that we no longer would be able to simply loop
> through our connector list and assume that nouveau_connector() would
> return a proper pointer for each connector, since the assertion that
> all connectors coming from nouveau have a full nouveau_connector struct
> became invalid.
> 
> Unfortunately, none of the actual code that looped through connectors
> ever got updated, which means that we've been causing invalid memory
> accesses for quite a while now.
> 
> An example that was caught by KASAN:
> 
> [  201.038698] ==================================================================
> [  201.038792] BUG: KASAN: slab-out-of-bounds in nvif_notify_get+0x190/0x1a0 [nouveau]
> [  201.038797] Read of size 4 at addr ffff88076738c650 by task kworker/0:3/718
> [  201.038800]
> [  201.038822] CPU: 0 PID: 718 Comm: kworker/0:3 Tainted: G           O      4.18.0-rc4Lyude-Test+ #1
> [  201.038825] Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET78W (1.51 ) 05/18/2018
> [  201.038882] Workqueue: events nouveau_display_hpd_work [nouveau]
> [  201.038887] Call Trace:
> [  201.038894]  dump_stack+0xa4/0xfd
> [  201.038900]  print_address_description+0x71/0x239
> [  201.038929]  ? nvif_notify_get+0x190/0x1a0 [nouveau]
> [  201.038935]  kasan_report.cold.6+0x242/0x2fe
> [  201.038942]  __asan_report_load4_noabort+0x19/0x20
> [  201.038970]  nvif_notify_get+0x190/0x1a0 [nouveau]
> [  201.038998]  ? nvif_notify_put+0x1f0/0x1f0 [nouveau]
> [  201.039003]  ? kmsg_dump_rewind_nolock+0xe4/0xe4
> [  201.039049]  nouveau_display_init.cold.12+0x34/0x39 [nouveau]
> [  201.039089]  ? nouveau_user_framebuffer_create+0x120/0x120 [nouveau]
> [  201.039133]  nouveau_display_resume+0x5c0/0x810 [nouveau]
> [  201.039173]  ? nvkm_client_ioctl+0x20/0x20 [nouveau]
> [  201.039215]  nouveau_do_resume+0x19f/0x570 [nouveau]
> [  201.039256]  nouveau_pmops_runtime_resume+0xd8/0x2a0 [nouveau]
> [  201.039264]  pci_pm_runtime_resume+0x130/0x250
> [  201.039269]  ? pci_restore_standard_config+0x70/0x70
> [  201.039275]  __rpm_callback+0x1f2/0x5d0
> [  201.039279]  ? rpm_resume+0x560/0x18a0
> [  201.039283]  ? pci_restore_standard_config+0x70/0x70
> [  201.039287]  ? pci_restore_standard_config+0x70/0x70
> [  201.039291]  ? pci_restore_standard_config+0x70/0x70
> [  201.039296]  rpm_callback+0x175/0x210
> [  201.039300]  ? pci_restore_standard_config+0x70/0x70
> [  201.039305]  rpm_resume+0xcc3/0x18a0
> [  201.039312]  ? rpm_callback+0x210/0x210
> [  201.039317]  ? __pm_runtime_resume+0x9e/0x100
> [  201.039322]  ? kasan_check_write+0x14/0x20
> [  201.039326]  ? do_raw_spin_lock+0xc2/0x1c0
> [  201.039333]  __pm_runtime_resume+0xac/0x100
> [  201.039374]  nouveau_display_hpd_work+0x67/0x1f0 [nouveau]
> [  201.039380]  process_one_work+0x7a0/0x14d0
> [  201.039388]  ? cancel_delayed_work_sync+0x20/0x20
> [  201.039392]  ? lock_acquire+0x113/0x310
> [  201.039398]  ? kasan_check_write+0x14/0x20
> [  201.039402]  ? do_raw_spin_lock+0xc2/0x1c0
> [  201.039409]  worker_thread+0x86/0xb50
> [  201.039418]  kthread+0x2e9/0x3a0
> [  201.039422]  ? process_one_work+0x14d0/0x14d0
> [  201.039426]  ? kthread_create_worker_on_cpu+0xc0/0xc0
> [  201.039431]  ret_from_fork+0x3a/0x50
> [  201.039441]
> [  201.039444] Allocated by task 79:
> [  201.039449]  save_stack+0x43/0xd0
> [  201.039452]  kasan_kmalloc+0xc4/0xe0
> [  201.039456]  kmem_cache_alloc_trace+0x10a/0x260
> [  201.039494]  nv50_mstm_add_connector+0x9a/0x340 [nouveau]
> [  201.039504]  drm_dp_add_port+0xff5/0x1fc0 [drm_kms_helper]
> [  201.039511]  drm_dp_send_link_address+0x4a7/0x740 [drm_kms_helper]
> [  201.039518]  drm_dp_check_and_send_link_address+0x1a7/0x210 [drm_kms_helper]
> [  201.039525]  drm_dp_mst_link_probe_work+0x71/0xb0 [drm_kms_helper]
> [  201.039529]  process_one_work+0x7a0/0x14d0
> [  201.039533]  worker_thread+0x86/0xb50
> [  201.039537]  kthread+0x2e9/0x3a0
> [  201.039541]  ret_from_fork+0x3a/0x50
> [  201.039543]
> [  201.039546] Freed by task 0:
> [  201.039549] (stack is not available)
> [  201.039551]
> [  201.039555] The buggy address belongs to the object at ffff88076738c1a8
>                                  which belongs to the cache kmalloc-2048 of size 2048
> [  201.039559] The buggy address is located 1192 bytes inside of
>                                  2048-byte region [ffff88076738c1a8, ffff88076738c9a8)
> [  201.039563] The buggy address belongs to the page:
> [  201.039567] page:ffffea001d9ce200 count:1 mapcount:0 mapping:ffff88084000d0c0 index:0x0 compound_mapcount: 0
> [  201.039573] flags: 0x8000000000008100(slab|head)
> [  201.039578] raw: 8000000000008100 ffffea001da3be08 ffffea001da25a08 ffff88084000d0c0
> [  201.039582] raw: 0000000000000000 00000000000d000d 00000001ffffffff 0000000000000000
> [  201.039585] page dumped because: kasan: bad access detected
> [  201.039588]
> [  201.039591] Memory state around the buggy address:
> [  201.039594]  ffff88076738c500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  201.039598]  ffff88076738c580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  201.039601] >ffff88076738c600: 00 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc
> [  201.039604]                                                  ^
> [  201.039607]  ffff88076738c680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [  201.039611]  ffff88076738c700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [  201.039613] ==================================================================
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Karol Herbst <karolherbst@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.h | 24 ++++++++++++++++++++-
>  drivers/gpu/drm/nouveau/nouveau_display.c   |  4 ++--
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 7dc380449232..af68eae4c626 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1213,7 +1213,7 @@ nouveau_connector_create(struct drm_device *dev, int index)
>  	bool dummy;
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
> -	drm_for_each_connector_iter(connector, &conn_iter) {
> +	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
>  		nv_connector = nouveau_connector(connector);
>  		if (nv_connector->index == index) {
>  			drm_connector_list_iter_end(&conn_iter);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h
> index a8cbb4b56fc7..aaf3a213c685 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
> @@ -33,6 +33,7 @@
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_dp_helper.h>
>  #include "nouveau_crtc.h"
> +#include "nouveau_encoder.h"
>  
>  struct nvkm_i2c_port;
>  
> @@ -60,6 +61,27 @@ static inline struct nouveau_connector *nouveau_connector(
>  	return container_of(con, struct nouveau_connector, base);
>  }
>  
> +static inline bool
> +nouveau_connector_is_mst(struct drm_connector *connector)
> +{
> +	const struct nouveau_encoder *nv_encoder;
> +	const struct drm_encoder *encoder;
> +
> +	if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
> +		return false;
> +
> +	nv_encoder = find_encoder(connector, DCB_OUTPUT_ANY);
> +	if (!nv_encoder)
> +		return false;
> +
> +	encoder = &nv_encoder->base.base;
> +	return encoder->encoder_type == DRM_MODE_ENCODER_DPMST;
> +}
> +
> +#define nouveau_for_each_non_mst_connector_iter(connector, iter) \
> +	drm_for_each_connector_iter(connector, iter) \
> +		if (!nouveau_connector_is_mst(connector))

for_each_if() is more robust since it avoids the issues with dangling
else blocks.
-Daniel

> +
>  static inline struct nouveau_connector *
>  nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
>  {
> @@ -70,7 +92,7 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
>  	struct drm_crtc *crtc = to_drm_crtc(nv_crtc);
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
> -	drm_for_each_connector_iter(connector, &conn_iter) {
> +	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
>  		if (connector->encoder && connector->encoder->crtc == crtc) {
>  			nv_connector = nouveau_connector(connector);
>  			break;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 46b8430ef4aa..ec7861457b84 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -413,7 +413,7 @@ nouveau_display_init(struct drm_device *dev)
>  
>  	/* enable hotplug interrupts */
>  	drm_connector_list_iter_begin(dev, &conn_iter);
> -	drm_for_each_connector_iter(connector, &conn_iter) {
> +	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
>  		struct nouveau_connector *conn = nouveau_connector(connector);
>  		nvif_notify_get(&conn->hpd);
>  	}
> @@ -444,7 +444,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend)
>  
>  	/* disable hotplug interrupts */
>  	drm_connector_list_iter_begin(dev, &conn_iter);
> -	drm_for_each_connector_iter(connector, &conn_iter) {
> +	nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
>  		struct nouveau_connector *conn = nouveau_connector(connector);
>  		nvif_notify_put(&conn->hpd);
>  	}
> -- 
> 2.17.1
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

end of thread, other threads:[~2018-07-13 16:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 16:24 [PATCH 0/2] drm/nouveau: Fix connector memory corruption issues Lyude Paul
2018-07-13 16:24 ` [PATCH 1/2] drm/nouveau: Use drm_connector_list_iter_* for iterating connectors Lyude Paul
2018-07-13 16:24 ` [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors Lyude Paul
2018-07-13 16:45   ` [Nouveau] " Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).