2018-07-13 16:25:48

by Lyude Paul

[permalink] [raw]
Subject: [PATCH 0/2] drm/nouveau: Fix connector memory corruption issues

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 <[email protected]>
Cc: [email protected]

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



2018-07-13 16:25:49

by Lyude Paul

[permalink] [raw]
Subject: [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors

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 <[email protected]>
Cc: [email protected]
Cc: Karol Herbst <[email protected]>
---
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


2018-07-13 16:26:08

by Lyude Paul

[permalink] [raw]
Subject: [PATCH 1/2] drm/nouveau: Use drm_connector_list_iter_* for iterating connectors

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 <[email protected]>
Cc: [email protected]
Cc: Karol Herbst <[email protected]>
---
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


2018-07-13 16:47:02

by Daniel Vetter

[permalink] [raw]
Subject: Re: [Nouveau] [PATCH 2/2] drm/nouveau: Avoid looping through fake MST connectors

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 <[email protected]>
> Cc: [email protected]
> Cc: Karol Herbst <[email protected]>
> ---
> 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/nouveau

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