[PATCH] drm/amd/display: fix NULL deref when opening a closed DDC

[PATCH] drm/amd/display: fix NULL deref when opening a closed DDC

[Arthur Liberman]

dal_ddc_open() uses gpio->pin after dal_gpio_service_open() installs
it. Close clears that pointer. HPD sink detection and a KWin atomic
check can both open the same DDC: the check reads DPCD for DSC while
the HPD worker closes the pin. The load of hw_gpio.store.en then
oopses at offset 0x28 and kwin_wayland exits with IRQs disabled.

If either pin is gone after open, close what this call holds and
return GPIO_RESULT_NULL_HANDLE so the AUX transfer fails instead.
This does not serialize the two threads.

Cc: stable@vger.kernel.org
Assisted-by: Cursor:xai-grok-4.7
Signed-off-by: Arthur Liberman <arthur_liberman@hotmail.com>
---
 drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
index 95f8b7c7d657..a3145cd82d8b 100644
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
@@ -577,6 +577,17 @@ enum gpio_result dal_ddc_open(
 		goto failure;
 	}
 
+	/*
+	 * dal_gpio_service_open() installs gpio->pin, and close clears it.
+	 * MST bring-up can AUX-read for DSC while HPD teardown closes the
+	 * same DDC. pin is NULL again in that window; do not dereference it.
+	 */
+	if (!ddc->pin_data->pin || !ddc->pin_clock->pin) {
+		dal_gpio_close(ddc->pin_clock);
+		dal_gpio_close(ddc->pin_data);
+		return GPIO_RESULT_NULL_HANDLE;
+	}
+
 	/* DDC clock and data pins should belong
 	 * to the same DDC block id,
 	 * we use the data pin to set the pad mode. */

base-commit: 41505ac433cc4c5179deee8860d197b04f6d1c1d
-- 
2.55.0

Re: [PATCH] drm/amd/display: fix NULL deref when opening a closed DDC

[Alex Hung]

Hi Arthur,

When AUX already holds the DDC, dal_ddc_open() fails with
GPIO_RESULT_ALREADY_OPENED, and the HPD path then calls dal_ddc_close()
on pins it does not own.

Please try whether the following solution fix your KWin issue?


diff --git 
drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c 
drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
index ad1a499b0d9d..1f53b33fc875 100644
--- drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
+++ drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
@@ -2604,8 +2604,6 @@ bool dp_is_sink_present(struct dc_link *link)

  	if (dal_ddc_open(ddc, GPIO_MODE_INPUT,
  			 GPIO_DDC_CONFIG_TYPE_MODE_I2C) != GPIO_RESULT_OK) {
-		dal_ddc_close(ddc);
-
  		return present;
  	}



On 10/7/26 11:47, Arthur Liberman wrote:
> dal_ddc_open() uses gpio->pin after dal_gpio_service_open() installs
> it. Close clears that pointer. HPD sink detection and a KWin atomic
> check can both open the same DDC: the check reads DPCD for DSC while
> the HPD worker closes the pin. The load of hw_gpio.store.en then
> oopses at offset 0x28 and kwin_wayland exits with IRQs disabled.
> 
> If either pin is gone after open, close what this call holds and
> return GPIO_RESULT_NULL_HANDLE so the AUX transfer fails instead.
> This does not serialize the two threads.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: Cursor:xai-grok-4.7
> Signed-off-by: Arthur Liberman <arthur_liberman@hotmail.com>
> ---
>   drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
> index 95f8b7c7d657..a3145cd82d8b 100644
> --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
> +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
> @@ -577,6 +577,17 @@ enum gpio_result dal_ddc_open(
>   		goto failure;
>   	}
>   
> +	/*
> +	 * dal_gpio_service_open() installs gpio->pin, and close clears it.
> +	 * MST bring-up can AUX-read for DSC while HPD teardown closes the
> +	 * same DDC. pin is NULL again in that window; do not dereference it.
> +	 */
> +	if (!ddc->pin_data->pin || !ddc->pin_clock->pin) {
> +		dal_gpio_close(ddc->pin_clock);
> +		dal_gpio_close(ddc->pin_data);
> +		return GPIO_RESULT_NULL_HANDLE;
> +	}
> +
>   	/* DDC clock and data pins should belong
>   	 * to the same DDC block id,
>   	 * we use the data pin to set the pad mode. */
> 
> base-commit: 41505ac433cc4c5179deee8860d197b04f6d1c1d

Re: [PATCH] drm/amd/display: fix NULL deref when opening a closed DDC

[Arthur Liberman]

Hi Alex,

I've only had this problem happen once so far, when I turned on my monitor after it was off for a day.
Unfortunately, I don't have a reliable way of reproducing this problem to see if your proposed patch fixes the issue.
I'm not sure whether this helps, but here's my setup:
Ryzen 9 9950X on a X870 board, no dedicated GPU.
Using the on-board USB-C output in DP alt-mode to connect my Dell UP2516D, daisy chaining a second display to it.
This setup has had a few other amdgpu related issues, for which I submitted patches, they also appear to be additional guards.

- Arthur









________________________________________
From: Alex Hung <alex.hung@amd.com>
Sent: Friday, October 16, 2026 7:39 PM
To: Arthur Liberman; amd-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; harry.wentland@amd.com; sunpeng.li@amd.com; siqueira@igalia.com; alexander.deucher@amd.com; christian.koenig@amd.com; airlied@gmail.com; simona@ffwll.ch
Subject: Re: [PATCH] drm/amd/display: fix NULL deref when opening a closed DDC

Hi Arthur,

When AUX already holds the DDC, dal_ddc_open() fails with
GPIO_RESULT_ALREADY_OPENED, and the HPD path then calls dal_ddc_close()
on pins it does not own.

Please try whether the following solution fix your KWin issue?


diff --git
drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
index ad1a499b0d9d..1f53b33fc875 100644
--- drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
+++ drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
@@ -2604,8 +2604,6 @@ bool dp_is_sink_present(struct dc_link *link)

        if (dal_ddc_open(ddc, GPIO_MODE_INPUT,
                         GPIO_DDC_CONFIG_TYPE_MODE_I2C) != GPIO_RESULT_OK) {
-               dal_ddc_close(ddc);
-
                return present;
        }



On 10/7/26 11:47, Arthur Liberman wrote:
> dal_ddc_open() uses gpio->pin after dal_gpio_service_open() installs
> it. Close clears that pointer. HPD sink detection and a KWin atomic
> check can both open the same DDC: the check reads DPCD for DSC while
> the HPD worker closes the pin. The load of hw_gpio.store.en then
> oopses at offset 0x28 and kwin_wayland exits with IRQs disabled.
>
> If either pin is gone after open, close what this call holds and
> return GPIO_RESULT_NULL_HANDLE so the AUX transfer fails instead.
> This does not serialize the two threads.
>
> Cc: stable@vger.kernel.org
> Assisted-by: Cursor:xai-grok-4.7
> Signed-off-by: Arthur Liberman <arthur_liberman@hotmail.com>
> ---
>   drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
> index 95f8b7c7d657..a3145cd82d8b 100644
> --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
> +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
> @@ -577,6 +577,17 @@ enum gpio_result dal_ddc_open(
>               goto failure;
>       }
>
> +     /*
> +      * dal_gpio_service_open() installs gpio->pin, and close clears it.
> +      * MST bring-up can AUX-read for DSC while HPD teardown closes the
> +      * same DDC. pin is NULL again in that window; do not dereference it.
> +      */
> +     if (!ddc->pin_data->pin || !ddc->pin_clock->pin) {
> +             dal_gpio_close(ddc->pin_clock);
> +             dal_gpio_close(ddc->pin_data);
> +             return GPIO_RESULT_NULL_HANDLE;
> +     }
> +
>       /* DDC clock and data pins should belong
>        * to the same DDC block id,
>        * we use the data pin to set the pad mode. */
>
> base-commit: 41505ac433cc4c5179deee8860d197b04f6d1c1d