linux-kernel - [PATCH AUTOSEL 5.0 188/262] drm/vkms: Bugfix racing hrtimer vblank handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190327180158.10245-188-sashal@kernel.org>
Date:   Wed, 27 Mar 2019 14:00:43 -0400
From:   Sasha Levin <sashal@...nel.org>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc:     Shayenne Moura <shayenneluzmoura@...il.com>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
        Sasha Levin <sashal@...nel.org>,
        dri-devel@...ts.freedesktop.org
Subject: [PATCH AUTOSEL 5.0 188/262] drm/vkms: Bugfix racing hrtimer vblank handle

From: Shayenne Moura <shayenneluzmoura@...il.com>

[ Upstream commit ba420afab565bdc7b028ddd4f222260f2de7a1db ]

When the vblank irq happens, kernel time subsystem executes
`vkms_vblank_simulate`. In parallel or not, it prepares all stuff
necessary to the next vblank with arm, and it must flush these stuff
before the next vblank irq. However, vblank counter is ahead when arm is
executed in parallel with handle vblank.

CPU 0:					CPU 1:
 |					 |
atomic_commit_tail is ongoing		 |
 |					 |
 |					hrtimer: vkms_vblank_simulate()
 |					 |
 |					drm_crtc_handle_vblank()
 |					 |
drm_crtc_arm_vblank()			 |
 |					 |
->get_vblank_timestamp()		 |
 |					 |
 |					hrtimer_forward_now()

Then, we should guarantee that the vblank interval time is correct (not
changed) before finish the vblank handle.

Fix the bug including the call to `hrtimer_forward_now()` in the same
lock of `drm_crtc_handle_vblank()` to ensure that the timestamp update
is correct when finish the vblank handle.

Signed-off-by: Shayenne Moura <shayenneluzmoura@...il.com>
Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>
Link: https://patchwork.freedesktop.org/patch/msgid/e2e4b8f3a5cab7b2dba75bf1930f86b0a4ee08c9.1548856186.git.shayenneluzmoura@gmail.com
Signed-off-by: Sasha Levin <sashal@...nel.org>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index eb56ee893761..53ab49408a98 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -4,13 +4,17 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 
-static void _vblank_handle(struct vkms_output *output)
+static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 {
+	struct vkms_output *output = container_of(timer, struct vkms_output,
+						  vblank_hrtimer);
 	struct drm_crtc *crtc = &output->crtc;
 	struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state);
+	int ret_overrun;
 	bool ret;
 
 	spin_lock(&output->lock);
+
 	ret = drm_crtc_handle_vblank(crtc);
 	if (!ret)
 		DRM_ERROR("vkms failure on handling vblank");
@@ -31,19 +35,9 @@ static void _vblank_handle(struct vkms_output *output)
 			DRM_WARN("failed to queue vkms_crc_work_handle");
 	}
 
-	spin_unlock(&output->lock);
-}
-
-static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
-{
-	struct vkms_output *output = container_of(timer, struct vkms_output,
-						  vblank_hrtimer);
-	int ret_overrun;
-
-	_vblank_handle(output);
-
 	ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
 					  output->period_ns);
+	spin_unlock(&output->lock);
 
 	return HRTIMER_RESTART;
 }
-- 
2.19.1