2023-08-14 15:09:11

by Karol Herbst

[permalink] [raw]
Subject: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create

We can't simply free the connector after calling drm_connector_init on it.
We need to clean up the drm side first.

It might not fix all regressions from 2b5d1c29f6c4 ("drm/nouveau/disp:
PIOR DP uses GPIO for HPD, not PMGR AUX interrupts"), but at least it
fixes a memory corruption in error handling related to that commit.

Link: https://lore.kernel.org/lkml/20230806213107.GFZNARG6moWpFuSJ9W@fat_crate.local/
Fixes: 95983aea8003 ("drm/nouveau/disp: add connector class")
Signed-off-by: Karol Herbst <[email protected]>
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index a2e0033e8a260..622f6eb9a8bfd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1408,8 +1408,7 @@ nouveau_connector_create(struct drm_device *dev,
ret = nvif_conn_ctor(&disp->disp, nv_connector->base.name, nv_connector->index,
&nv_connector->conn);
if (ret) {
- kfree(nv_connector);
- return ERR_PTR(ret);
+ goto drm_conn_err;
}

ret = nvif_conn_event_ctor(&nv_connector->conn, "kmsHotplug",
@@ -1426,8 +1425,7 @@ nouveau_connector_create(struct drm_device *dev,
if (ret) {
nvif_event_dtor(&nv_connector->hpd);
nvif_conn_dtor(&nv_connector->conn);
- kfree(nv_connector);
- return ERR_PTR(ret);
+ goto drm_conn_err;
}
}
}
@@ -1475,4 +1473,9 @@ nouveau_connector_create(struct drm_device *dev,

drm_connector_register(connector);
return connector;
+
+drm_conn_err:
+ drm_connector_cleanup(connector);
+ kfree(nv_connector);
+ return ERR_PTR(ret);
}
--
2.41.0



2023-08-18 19:08:10

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create

On Wed, Aug 16, 2023 at 04:57:28PM +0200, Karol Herbst wrote:
> Do you have any connectors listed in "/sys/class/drm"?

tree /sys/class/drm/
/sys/class/drm/
├── card0 -> ../../devices/pci0000:00/0000:00:02.0/0000:03:00.0/drm/card0
├── renderD128 -> ../../devices/pci0000:00/0000:00:02.0/0000:03:00.0/drm/renderD128
└── version

> Also, mind sharing your vbios.rom file from
> "/sys/kernel/debug/dri/0/vbios.rom"?

Attached.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette


Attachments:
(No filename) (547.00 B)
vbios.rom.gz (44.38 kB)
Download all attachments

2023-08-19 10:35:11

by Karol Herbst

[permalink] [raw]
Subject: Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create

On Wed, Aug 16, 2023 at 4:54 PM Borislav Petkov <[email protected]> wrote:
>
> On Wed, Aug 16, 2023 at 11:51:50AM +0200, Karol Herbst wrote:
> > Mind sharing your kernel logs with that patch applied? I suspect your
> > system boots up but you might just not have the connector available or
> > something? It could be that you have one of those GPUs affected by the
> > original change and then we'd have to figure out what to do with that.
>
> Close. With your patch applied, the machine is up and I can log in and
> use it. However, the output on the connected monitor stops after...
>
> [ 6.815167] ACPI: \_PR_.CP05: Found 4 idle states
> [ 6.825438] ACPI: \_PR_.CP06: Found 4 idle states
> [ 6.835661] ACPI: \_PR_.CP07: Found 4 idle states
> [ 7.280093] Freeing initrd memory: 8328K
> [ 7.601986] tsc: Refined TSC clocksource calibration: 3591.346 MHz
> [ 7.608360] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x33c46403b59, max_idle_ns: 440795293818 ns
> [ 7.620254] clocksource: Switched to clocksource tsc
> [ 8.337724] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
> [ 8.350553] 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
> [ 8.375311] serial 0000:00:16.3: enabling device (0000 -> 0003)
> [ 8.403681] 0000:00:16.3: ttyS1 at I/O 0xf0a0 (irq = 17, base_baud = 115200) is a 16550A
> [ 8.424951] Linux agpgart interface v0.103
> [ 8.432456] ACPI: bus type drm_connector registered
>
> ... this line here above. It is the last one output. What you see here
> below what I'm catching from serial.
>
> [ 8.456734] Console: switching to colour dummy device 80x25
> [ 8.464414] nouveau 0000:03:00.0: vgaarb: deactivate vga console
> [ 8.473063] nouveau 0000:03:00.0: NVIDIA GT218 (0a8c00b1)
> [ 8.594096] nouveau 0000:03:00.0: bios: version 70.18.83.00.08
> [ 8.607906] nouveau 0000:03:00.0: fb: 512 MiB DDR3
> [ 8.926721] nouveau 0000:03:00.0: DRM: VRAM: 512 MiB
> [ 8.931763] nouveau 0000:03:00.0: DRM: GART: 1048576 MiB
> [ 8.937156] nouveau 0000:03:00.0: DRM: TMDS table version 2.0
> [ 8.942969] nouveau 0000:03:00.0: DRM: DCB version 4.0
> [ 8.948173] nouveau 0000:03:00.0: DRM: DCB outp 00: 02000360 00000000
> [ 8.954696] nouveau 0000:03:00.0: DRM: DCB outp 01: 02000362 00020010
> [ 8.961211] nouveau 0000:03:00.0: DRM: DCB outp 02: 028003a6 0f220010
> [ 8.967739] nouveau 0000:03:00.0: DRM: DCB outp 03: 01011380 00000000
> [ 8.974261] nouveau 0000:03:00.0: DRM: DCB outp 04: 08011382 00020010
> [ 8.980769] nouveau 0000:03:00.0: DRM: DCB outp 05: 088113c6 0f220010
> [ 8.987293] nouveau 0000:03:00.0: DRM: DCB conn 00: 00101064
> [ 8.993015] nouveau 0000:03:00.0: DRM: DCB conn 01: 00202165
> [ 9.005724] nouveau 0000:03:00.0: DRM: MM: using COPY for buffer copies
> [ 9.023889] [drm] Initialized nouveau 1.3.1 20120801 for 0000:03:00.0 on minor 0
> [ 9.032044] nouveau 0000:03:00.0: [drm] Cannot find any crtc or sizes
> [ 9.162909] megasas: 07.725.01.00-rc1
> [ 9.167537] st: Version 20160209, fixed bufsize 32768, s/g segs 256
> [ 9.176058] ahci 0000:00:1f.2: version 3.0
> [ 9.194078] ahci 0000:00:1f.2: AHCI 0001.0300 32 slots 6 ports 6 Gbps 0x3 impl SATA mode
> [ 9.202487] ahci 0000:00:1f.2: flags: 64bit ncq sntf pm led clo pio slum part ems apst
> [ 9.243154] scsi host0: ahci
> [ 9.252090] scsi host1: ahci
> [ 9.260389] scsi host2: ahci
> [ 9.268061] scsi host3: ahci
> [ 9.273542] scsi host4: ahci
> [ 9.279071] scsi host5: ahci
> ...
>
> and so on until full boot.
>

okay, so the patch at least fixes the memory corruption, which is
good, so I'd go ahead and push it out as it might also fix other
unrelated crashes.

Do you have any connectors listed in "/sys/class/drm"? Also, mind
sharing your vbios.rom file from "/sys/kernel/debug/dri/0/vbios.rom"?

Thanks

> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
>


2023-08-19 12:22:35

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create

On Wed, Aug 16, 2023 at 11:27:05PM +0200, Karol Herbst wrote:
> that GPU has only a `DMS-59` connector, is that right?

No clue. How do I figure that out?

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-08-20 15:07:37

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create

On Thu, Aug 17, 2023 at 01:18:12AM +0200, Karol Herbst wrote:
> do you have one of these? https://en.wikipedia.org/wiki/DMS-59

Ah, DMS == Dual Monitor Solution :-)

Yap, that's exactly what the GPU has. And the Y-cable is 2xDVI. It is
a Dell workstation and it came this way, meaning I haven't done any
changes there.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette