[patch v3] staging: speakup: fix speakup-r empty line lockup

[Samuel Thibault]

Okash Khawaja, on lun. 28 août 2017 23:13:13 +0100, wrote:
> (Please ignore patch v2 last sent)
> 
> 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 <okash.khawaja at gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault at 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 = 0;
> +
>  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;
>  }
>  
> 

-- 
Samuel
The nice thing about Windows is - It does not just crash, it displays a
dialog box and lets you press 'OK' first.
(Arno Schaefer's .sig)