[patch 0/1] staging: speakup: fix speakup-r empty line lockup

[Okash Khawaja]

Hi,

This patch fixes kernel lockup shown by lockdep log below. Further
details are in patch header.

Samuel, please note that I removed initialisation of the static int
in_keyboard_notifier to zero as that is not required and reported as
error by checkpatch.

Thanks,
Okash


[ 1293.803242] ============================================
[ 1293.803304] WARNING: possible recursive locking detected
[ 1293.803369] 4.12.2-ARCH+ #7 Tainted: G C O
[ 1293.803442] --------------------------------------------
[ 1293.803504] swapper/0/0 is trying to acquire lock:
[ 1293.803562] (&(&dev->event_lock)->rlock){-.-...}, at: [<ffffffff8153b0ae>] input_event+0x3e/0x80
[ 1293.803671]
[ 1293.803671] but task is already holding lock:
[ 1293.803740] (&(&dev->event_lock)->rlock){-.-...}, at: [<ffffffff8153b0ae>] input_event+0x3e/0x80
[ 1293.803843]
[ 1293.803843] other info that might help us debug this:
[ 1293.803916] Possible unsafe locking scenario:
[ 1293.803916]
[ 1293.803984] CPU0
[ 1293.804021] ----
[ 1293.804054] lock(&(&dev->event_lock)->rlock);
[ 1293.804109] lock(&(&dev->event_lock)->rlock);
[ 1293.804164]
[ 1293.804164] *** DEADLOCK ***
[ 1293.804164]
[ 1293.804233] May be due to missing lock nesting notation
[ 1293.804233]
[ 1293.804311] 6 locks held by swapper/0/0:
[ 1293.804359] #0: (&serio->lock){-.-...}, at: [<ffffffffa0007fea>] serio_interrupt+0x2a/0x90 [serio]
[ 1293.804469] #1: (&(&dev->event_lock)->rlock){-.-...}, at: [<ffffffff8153b0ae>] input_event+0x3e/0x80
[ 1293.804577] #2: (rcu_read_lock){......}, at: [<ffffffff8153a6b5>] input_pass_values.part.1+0x5/0x260
[ 1293.804685] #3: (kbd_event_lock){-.-...}, at: [<ffffffff814a091c>] kbd_event+0x3c/0x710
[ 1293.804781] #4: (rcu_read_lock){......}, at: [<ffffffff810b1135>] __atomic_notifier_call_chain+0x5/0x110
[ 1293.804898] #5: (speakup_info.spinlock){-.-...}, at: [<ffffffffa06690ac>] keyboard_notifier_call+0xfc/0xcc0 [speakup]
[ 1293.805028]
[ 1293.805028] stack backtrace:
[ 1293.805084] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G C O 4.12.2-ARCH+ #7
[ 1293.805168] Hardware name: Dell Inc. Inspiron 7559/0H0CC0, BIOS 1.2.2 01/22/2017
[ 1293.805250] Call Trace:
[ 1293.805284] <IRQ>
[ 1293.805320] dump_stack+0x8e/0xcd
[ 1293.805366] __lock_acquire+0x87d/0x1a20
[ 1293.805423] ? native_apic_wait_icr_idle+0x1f/0x30
[ 1293.805482] ? arch_irq_work_raise+0x34/0x40
[ 1293.805537] lock_acquire+0xa5/0x250
[ 1293.805583] ? lock_acquire+0xa5/0x250
[ 1293.805632] ? input_event+0x3e/0x80
[ 1293.805683] _raw_spin_lock_irqsave+0x4d/0x90
[ 1293.805736] ? input_event+0x3e/0x80
[ 1293.805782] input_event+0x3e/0x80
[ 1293.805833] speakup_fake_down_arrow2+0x57/0xd0 [speakup]
[ 1293.805902] read_all_doc+0xd7/0xe0 [speakup]
[ 1293.805960] ? kbd_fakekey+0x40/0x40 [speakup]
[ 1293.806020] keyboard_notifier_call+0xbe8/0xcc0 [speakup]
[ 1293.806085] ? lock_acquire+0xa5/0x250
[ 1293.806136] notifier_call_chain+0x4a/0x70
[ 1293.806191] __atomic_notifier_call_chain+0x72/0x110
[ 1293.806254] atomic_notifier_call_chain+0x16/0x20
[ 1293.806311] kbd_event+0x320/0x710
[ 1293.806361] input_to_handler+0xdb/0xf0
[ 1293.806411] input_pass_values.part.1+0x1b3/0x260
[ 1293.806469] input_handle_event+0xe4/0x150
[ 1293.806608] input_event+0x52/0x80
[ 1293.806660] atkbd_interrupt+0x50a/0x7c0 [atkbd]
[ 1293.806719] serio_interrupt+0x46/0x90 [serio]
[ 1293.806778] i8042_interrupt+0x20d/0x3b0 [i8042]
[ 1293.806839] __handle_irq_event_percpu+0x45/0x390
[ 1293.806899] handle_irq_event_percpu+0x32/0x80
[ 1293.806955] handle_irq_event+0x39/0x60
[ 1293.807007] handle_edge_irq+0x78/0x130
[ 1293.808787] handle_irq+0x1a/0x30
[ 1293.809755] do_IRQ+0x58/0x110
[ 1293.810712] common_interrupt+0x93/0x93
[ 1293.811644] RIP: 0010:cpuidle_enter_state+0x143/0x390
[ 1293.812575] RSP: 0018:ffffffff81c03dc0 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffce
[ 1293.813518] RAX: ffffffff81c18500 RBX: 0000012d3ca4a19b RCX: 0000000000000000
[ 1293.814462] RDX: ffffffff81c18500 RSI: 0000000000000001 RDI: ffffffff81c18500
[ 1293.815398] RBP: ffffffff81c03e00 R08: cccccccccccccccd R09: 0000000000000000
[ 1293.816335] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8804707e3100
[ 1293.817261] R13: 0000000000000000 R14: 0000000000000008 R15: ffffffff81cc7898
[ 1293.818179] </IRQ>
[ 1293.819081] cpuidle_enter+0x17/0x20
[ 1293.819986] call_cpuidle+0x23/0x40
[ 1293.820884] do_idle+0x18f/0x1f0
[ 1293.821776] cpu_startup_entry+0x71/0x80
[ 1293.822662] rest_init+0x131/0x140
[ 1293.823547] start_kernel+0x44d/0x46e
[ 1293.824426] ? early_idt_handler_array+0x120/0x120
[ 1293.825326] x86_64_start_reservations+0x2f/0x31
[ 1293.826186] x86_64_start_kernel+0x141/0x164
[ 1293.827045] secondary_startup_64+0x9f/0x9f

[Okash Khawaja]

When cursor is at beginning of an empty or whitespace-only line and
speakup-r typed, kernel locks up. This happens because deadlock of in
input_event function over dev->event_lock, as demonstrated by lockdep
logs. The reason for that is speakup simulates a down arrow - because
cursor is at an empty line - while inside key press notifier handler
which is ultimately triggered from input_event function. The simulated
key press leads to input_event being called again, this time under its
own context. So the spinlock is dev->event_lock is acquired while still
being held.

This patch ensures that key press is not simulated from inside key press
notifier handler. Instead it delegates to cursor_timer. It starts the
timer and passes RA_DOWN_ARROW as argument. When timer handler runs and
sees RA_DOWN_ARROW, it will then call kbd_fakekey2(RA_DOWN_ARROW) which
will correctly simulate the keypress inside timer context.

When not inside key press notifier callback, the behaviour will remain
the same as before this patch.

Signed-off-by: Okash Khawaja <***@gmail.com>
Reviewed-by: Samuel Thibault <***@ens-lyon.org>

---
drivers/staging/speakup/main.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)

--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1376,6 +1376,8 @@ static void reset_highlight_buffers(stru

static int read_all_key;

+static int in_keyboard_notifier;
+
static void start_read_all_timer(struct vc_data *vc, int command);

enum {
@@ -1408,7 +1410,10 @@ static void read_all_doc(struct vc_data
cursor_track = read_all_mode;
spk_reset_index_count(0);
if (get_sentence_buf(vc, 0) == -1) {
- kbd_fakekey2(vc, RA_DOWN_ARROW);
+ del_timer(&cursor_timer);
+ if (!in_keyboard_notifier)
+ speakup_fake_down_arrow();
+ start_read_all_timer(vc, RA_DOWN_ARROW);
} else {
say_sentence_num(0, 0);
synth_insert_next_index(0);
@@ -2212,8 +2217,10 @@ static int keyboard_notifier_call(struct
int ret = NOTIFY_OK;
static int keycode; /* to hold the current keycode */

+ in_keyboard_notifier = 1;
+
if (vc->vc_mode == KD_GRAPHICS)
- return ret;
+ goto out;

/*
* First, determine whether we are handling a fake keypress on
@@ -2225,7 +2232,7 @@ static int keyboard_notifier_call(struct
*/

if (speakup_fake_key_pressed())
- return ret;
+ goto out;

switch (code) {
case KBD_KEYCODE:
@@ -2266,6 +2273,8 @@ static int keyboard_notifier_call(struct
break;
}
}
+out:
+ in_keyboard_notifier = 0;
return ret;
}