linux-kernel - [PATCH v2] Input: matrix_keypad - fix keypad does not response

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:   Sat,  3 Feb 2018 23:58:16 +0800
From:   Zhang Bo <zbsdta@....com>
To:     dmitry.torokhov@...il.com
Cc:     robh@...nel.org, DRivshin@...worx.com, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org, zhang.bo19@....com.cn,
        andy.shevchenko@...il.com, zbsdta@....com
Subject: [PATCH v2] Input: matrix_keypad - fix keypad does not response

If matrix_keypad_stop() is calling and the keypad interrupt is triggered,
disable_row_irqs() is called by both matrix_keypad_interrupt() and
matrix_keypad_stop() at the same time. then disable_row_irqs() is called
twice, and the device enter suspend state before keypad->work is executed.
At this condition the device will start keypad and enable irq once after
resume. and then irqs are disabled yet because irqs are disabled twice and
only enable once.

Take lock around keypad->stopped and queue delayed work in
matrix_keypad_start() and matrix_keypad_stop() to ensure irqs operation and
scheduling scan work are in atomic operation.

Signed-off-by: Zhang Bo <zbsdta@....com>
---
Changes in v2:
  - Change commit message and full name in the signed-off-by tag.

 drivers/input/keyboard/matrix_keypad.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 1f316d66e6f7..13fe51824637 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
 	/* Enable IRQs again */
 	spin_lock_irq(&keypad->lock);
 	keypad->scan_pending = false;
-	enable_row_irqs(keypad);
+	if (keypad->stopped == false)
+		enable_row_irqs(keypad);
 	spin_unlock_irq(&keypad->lock);
 }
 
@@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
 {
 	struct matrix_keypad *keypad = input_get_drvdata(dev);
 
+	spin_lock_irq(&keypad->lock);
 	keypad->stopped = false;
-	mb();
 
 	/*
 	 * Schedule an immediate key scan to capture current key state;
 	 * columns will be activated and IRQs be enabled after the scan.
 	 */
-	schedule_delayed_work(&keypad->work, 0);
+	if (keypad->scan_pending == false)
+		schedule_delayed_work(&keypad->work, 0);
+	spin_unlock_irq(&keypad->lock);
 
 	return 0;
 }
@@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
 {
 	struct matrix_keypad *keypad = input_get_drvdata(dev);
 
+	spin_lock_irq(&keypad->lock);
 	keypad->stopped = true;
-	mb();
-	flush_work(&keypad->work.work);
 	/*
 	 * matrix_keypad_scan() will leave IRQs enabled;
 	 * we should disable them now.
 	 */
-	disable_row_irqs(keypad);
+	if (keypad->scan_pending == false)
+		disable_row_irqs(keypad);
+	spin_unlock_irq(&keypad->lock);
+
+	flush_work(&keypad->work.work);
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.14.3