matrix_keypad_stop() now serialises with matrix_keypad_interrupt()
using spin_lock_irq() while setting the "stopped" flag, but it drops
the lock before disabling the interrupt sources. If an interrupt is
asserted at this point, matrix_keypad_interrupt() will ignore it
because stopped is true. The interrupt will remain asserted and it
may be called repeatedly, potentially leading to a hard lockup on a UP
system.
Instead, matrix_keypad_stop() should disable the interrupt sources
(asynchronously) while still holding the lock. However, we have to
take care because at this point they may already be disabled due to a
pending scan. To avoid mismatched enable/disable calls, we should
maintain the invariant that they are enabled if and only if
!(scan_pending || stopped). Therefore:
- Set the scan_pending flag in matrix_keypad_start(), since it does
schedule a scan and doesn't enable interrupts
- Only disable interrupts in matrix_keypad_stop() if scan_pending is
false
- Only enable interrupts in matrix_keypad_scan() if stopped is false
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx>
---
This is untested and completely based on code review. It's possible
that the problem I identified can't really happen for reasons that
aren't obvious to me.
Ben.
drivers/input/keyboard/matrix_keypad.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 41614c185918..e765950d3491 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)
+ enable_row_irqs(keypad);
spin_unlock_irq(&keypad->lock);
}
@@ -203,6 +204,7 @@ static int matrix_keypad_start(struct input_dev *dev)
struct matrix_keypad *keypad = input_get_drvdata(dev);
keypad->stopped = false;
+ keypad->scan_pending = true;
mb();
/*
@@ -220,14 +222,16 @@ static void matrix_keypad_stop(struct input_dev *dev)
spin_lock_irq(&keypad->lock);
keypad->stopped = true;
+ if (!keypad->scan_pending) {
+ /*
+ * matrix_keypad_scan() will leave IRQs enabled;
+ * we should disable them now.
+ */
+ disable_row_irqs(keypad);
+ }
spin_unlock_irq(&keypad->lock);
flush_work(&keypad->work.work);
- /*
- * matrix_keypad_scan() will leave IRQs enabled;
- * we should disable them now.
- */
- disable_row_irqs(keypad);
}
#ifdef CONFIG_PM_SLEEP
--
2.15.0.rc0
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
