[PATCH 6.18.y] drm/vkms: Fix ABBA deadlock in vblank disable and timer callback

[PATCH 6.18.y] drm/vkms: Fix ABBA deadlock in vblank disable and timer callback

[w15303746062]

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

[Note: This patch addresses a legacy VKMS implementation deadlock specific
to older stable trees (e.g., 6.18.y). Mainline has removed this code during
the generic DRM_CRTC_VBLANK_TIMER_FUNCS refactoring.]

During local fuzzing with Syzkaller, an RCU preempt stall (soft lockup)
was observed. This is caused by an ABBA deadlock between the
drm_vblank_disable_and_save() function and the vkms_vblank_simulate()
hrtimer callback.

The race condition occurs as follows:

Thread A (CPU 3 - DRM_IOCTL_MODE_SETCRTC):
  - drm_vblank_disable_and_save() acquires `&dev->vblank_time_lock`.
  - Calls __disable_vblank() -> vkms_disable_vblank().
  - Calls hrtimer_cancel() to synchronously stop the vblank timer.
  - BLOCK: hrtimer_cancel() spins indefinitely waiting for the timer
    callback to finish executing on CPU 0.

Thread B (CPU 0 - hrtimer interrupt):
  - Executes the hrtimer callback vkms_vblank_simulate().
  - Calls drm_crtc_handle_vblank() -> drm_handle_vblank().
  - BLOCK: drm_handle_vblank() tries to acquire `&dev->vblank_time_lock`
    and spins forever because Thread A is holding it.

This patch fixes the deadlock by replacing hrtimer_cancel() with
hrtimer_try_to_cancel(). If the timer callback is running, try_to_cancel()
will safely return -1 and allow Thread A to proceed and release the lock.

Additionally, vkms_vblank_simulate() is modified to conditionally return
HRTIMER_NORESTART if drm_crtc_handle_vblank() fails (which it will,
because Thread A sets `vblank->enabled = false` immediately after
try_to_cancel). This acts as a self-destruct mechanism, preventing the
timer from blindly re-arming itself and causing an infinite loop of
DRM_ERROR messages.

Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index e60573e0f3e9..a62153b73548 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -57,7 +57,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 
 	dma_fence_end_signalling(fence_cookie);
 
-	return HRTIMER_RESTART;
+	return ret ? HRTIMER_RESTART : HRTIMER_NORESTART;
 }
 
 static int vkms_enable_vblank(struct drm_crtc *crtc)
@@ -77,7 +77,7 @@ static void vkms_disable_vblank(struct drm_crtc *crtc)
 {
 	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
 
-	hrtimer_cancel(&out->vblank_hrtimer);
+	hrtimer_try_to_cancel(&out->vblank_hrtimer);
 }
 
 static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
-- 
2.34.1

Re: [PATCH 6.18.y] drm/vkms: Fix ABBA deadlock in vblank disable and timer callback

[Greg KH]

On Fri, May 15, 2026 at 09:18:26PM +0800, w15303746062@163.com wrote:
> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> 
> [Note: This patch addresses a legacy VKMS implementation deadlock specific
> to older stable trees (e.g., 6.18.y). Mainline has removed this code during
> the generic DRM_CRTC_VBLANK_TIMER_FUNCS refactoring.]

Why not apply those upstream commits here as well?  No need to diverge
from Linus's tree, otherwise we will end up having a mess that nothing
can ever be backported to.

How many commits need to be backported?  Have you tried?

thanks,

greg k-h

Re:Re: [PATCH 6.18.y] drm/vkms: Fix ABBA deadlock in vblank disable and timer callback

[w15303746062]

Hi Greg,

Thanks for the quick response and review.


At 2026-05-15 23:09:46, "Greg KH" <greg@kroah.com> wrote:
>On Fri, May 15, 2026 at 09:18:26PM +0800, w15303746062@163.com wrote:
>> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>> 
>> [Note: This patch addresses a legacy VKMS implementation deadlock specific
>> to older stable trees (e.g., 6.18.y). Mainline has removed this code during
>> the generic DRM_CRTC_VBLANK_TIMER_FUNCS refactoring.]
>
>Why not apply those upstream commits here as well?  No need to diverge
>from Linus's tree, otherwise we will end up having a mess that nothing
>can ever be backported to.
>
>How many commits need to be backported?  Have you tried?

I have looked into the upstream commits. The commit that removed this 
vulnerable legacy code in mainline is:
02e2681ffe1a ("drm/vkms: Convert to DRM's vblank timer")

I tried to apply it to 6.18.y, but it does not apply cleanly. The reason 
is that this upstream commit is not a simple bug fix, but a massive 
refactoring. It completely rips out the custom VKMS hrtimer and ports 
the driver to a newly introduced DRM core infrastructure 
(DRM_CRTC_VBLANK_TIMER_FUNCS and drm_vblank_helper.h).

To backport commit 02e2681ffe1a, we would first need to backport the 
entire DRM generic vblank timer infrastructure to 6.18.y. This seems 
too intrusive and violates the minimal-risk policy for stable trees.

Therefore, since the legacy custom hrtimer still exists in 6.18.y and 
is actively causing ABBA deadlocks (RCU stalls), this minimalistic 
and localized patch (using hrtimer_try_to_cancel) is proposed as the 
safest way to fix the issue specifically for older stable branches 
without pulling in major DRM core refactoring.

Would this localized fix be acceptable for the stable tree?

Thanks,
Mingyu

Re: Re: [PATCH 6.18.y] drm/vkms: Fix ABBA deadlock in vblank disable and timer callback

[Greg KH]

On Sat, May 16, 2026 at 10:43:35AM +0800, w15303746062 wrote:
> 
> Hi Greg,
> 
> Thanks for the quick response and review.
> 
> 
> At 2026-05-15 23:09:46, "Greg KH" <greg@kroah.com> wrote:
> >On Fri, May 15, 2026 at 09:18:26PM +0800, w15303746062@163.com wrote:
> >> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> >> 
> >> [Note: This patch addresses a legacy VKMS implementation deadlock specific
> >> to older stable trees (e.g., 6.18.y). Mainline has removed this code during
> >> the generic DRM_CRTC_VBLANK_TIMER_FUNCS refactoring.]
> >
> >Why not apply those upstream commits here as well?  No need to diverge
> >from Linus's tree, otherwise we will end up having a mess that nothing
> >can ever be backported to.
> >
> >How many commits need to be backported?  Have you tried?
> 
> I have looked into the upstream commits. The commit that removed this 
> vulnerable legacy code in mainline is:
> 02e2681ffe1a ("drm/vkms: Convert to DRM's vblank timer")
> 
> I tried to apply it to 6.18.y, but it does not apply cleanly. The reason 
> is that this upstream commit is not a simple bug fix, but a massive 
> refactoring. It completely rips out the custom VKMS hrtimer and ports 
> the driver to a newly introduced DRM core infrastructure 
> (DRM_CRTC_VBLANK_TIMER_FUNCS and drm_vblank_helper.h).
> 
> To backport commit 02e2681ffe1a, we would first need to backport the 
> entire DRM generic vblank timer infrastructure to 6.18.y. This seems 
> too intrusive and violates the minimal-risk policy for stable trees.

There is no "minimal-risk policy for stable trees".  And if there was,
the least ammount of risk would be to take the reviewed and tested
patches that are already in Linus's tree, and NOT take anything that is
not already there, as 90% of the time that we do that, it comes back to
bite us hard.

So please, just backport all the needed changes here.  Otherwise how are
we going to deal with the merge conflicts for the next 4 years in this
file?

Or, get the maintainers of this file to agree and review this one-off
change that it is acceptable.  As they are going to be the ones getting
the bug reports and not having their patches applied over the years, not
anyone else :)

thanks,

greg k-h