linux-kernel - [PATCH] Input: fix atomicity violation in gameport_run_poll

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>] [day] [month] [year] [list]

Date: Fri, 12 Jan 2024 15:38:55 +0800
From: Gui-Dong Han <2045gemini@...il.com>
To: dmitry.torokhov@...il.com,
	arnd@...nel.org,
	schnelle@...ux.ibm.com
Cc: linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	baijiaju1990@...look.com,
	Gui-Dong Han <2045gemini@...il.com>,
	stable@...r.kernel.org
Subject: [PATCH] Input: fix atomicity violation in gameport_run_poll_handler

In gameport_run_poll_handler():
    ...
    if (gameport->poll_cnt)
        mod_timer(&gameport->poll_timer, jiffies + ...));

In gameport_stop_polling():
    spin_lock(&gameport->timer_lock);
    if (!--gameport->poll_cnt)
        del_timer(&gameport->poll_timer);
    spin_unlock(&gameport->timer_lock);

An atomicity violation occurs due to the concurrent execution of
gameport_run_poll_handler() and gameport_stop_polling(). The current check
for gameport->poll_cnt in gameport_run_poll_handler() is not effective
because poll_cnt can be decremented to 0 and del_timer can be called in
gameport_stop_polling() before mod_timer is called in
gameport_run_poll_handler(). This situation leads to the risk of calling
mod_timer for a timer that has already been deleted in
gameport_stop_polling(). Since calling mod_timer on a deleted timer
reactivates it, this atomicity violation could result in the timer being
activated while the poll_cnt value is 0.

This possible bug is found by an experimental static analysis tool
developed by our team, BassCheck[1]. This tool analyzes the locking APIs
to extract function pairs that can be concurrently executed, and then
analyzes the instructions in the paired functions to identify possible
concurrency bugs including data races and atomicity violations. The above
possible bug is reported when our tool analyzes the source code of
Linux 5.17.

To resolve this issue, it is suggested to add a spinlock pair in
gameport_run_poll_handler() to ensure atomicity. With this patch applied,
our tool no longer reports the bug, with the kernel configuration
allyesconfig for x86_64. Due to the absence of the requisite hardware, we
are unable to conduct runtime testing of the patch. Therefore, our
verification is solely based on code logic analysis.

[1] https://sites.google.com/view/basscheck/

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@...r.kernel.org
Signed-off-by: Gui-Dong Han <2045gemini@...il.com>
---
 drivers/input/gameport/gameport.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c
index 34f416a3ebcb..12af46d3c059 100644
--- a/drivers/input/gameport/gameport.c
+++ b/drivers/input/gameport/gameport.c
@@ -202,8 +202,13 @@ static void gameport_run_poll_handler(struct timer_list *t)
 	struct gameport *gameport = from_timer(gameport, t, poll_timer);

 	gameport->poll_handler(gameport);
+
+	spin_lock(&gameport->timer_lock);
+
 	if (gameport->poll_cnt)
 		mod_timer(&gameport->poll_timer, jiffies + msecs_to_jiffies(gameport->poll_interval));
+
+	spin_unlock(&gameport->timer_lock);
 }

 /*
-- 
2.34.1