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

[Okash Khawaja]

Hi John,

Yes it is the same. But it is something we need otherwise the code will
have a path which causes deadlock. My suggestion is we include this patch
in speakup code and deal with incorrect cursor position when control key is
hit, as a separate bug.

Previous email explains the deadlock.

Thanks,
Okash


On 27 Aug 2017 5:46 am, "John Covici" <covici at ccs.covici.com> wrote:

> hmmm, new patch is the same as the old one and so will not work any
> better.
>
> On Sat, 26 Aug 2017 13:15:13 -0400,
> Okash Khawaja wrote:
> >
> > 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>
> >
> > ---
> >  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 volatile 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;
> >  }
> >
>
> --
> Your life is like a penny.  You're going to lose it.  The question is:
> How do
> you spend it?
>
>          John Covici
>          covici at ccs.covici.com
>