[patch v3] fix async usb removal

[Samuel Thibault]

Hello,

Okash Khawaja, on sam. 12 août 2017 08:49:02 +0100, wrote:
> When an external USB synth is unplugged while the module is loaded, we
> get a null pointer deref. This is because the tty disappears while
> speakup tries to use to to communicate with the synth. This patch fixes
> it by checking tty for null before using it. Since tty can become null
> between the check and its usage, a mutex is introduced. tty usage is
> now surrounded by the mutex, as is the code in speakup_ldisc_close which
> sets the tty to null. The mutex also serialises calls to tty from
> speakup code. 
> 
> In case of tty being null, this sets synth->alive to zero and starts
> ttys.

Perhaps rather say "restarts ttys in case they were stopped by speakup".

> Signed-off-by: Okash Khawaja <okash.khawaja at gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault at ens-lyon.org>

> ---
>  drivers/staging/speakup/spk_ttyio.c |   50 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> --- a/drivers/staging/speakup/spk_ttyio.c
> +++ b/drivers/staging/speakup/spk_ttyio.c
> @@ -15,6 +15,11 @@ struct spk_ldisc_data {
>  
>  static struct spk_synth *spk_ttyio_synth;
>  static struct tty_struct *speakup_tty;
> +/* mutex to protect against speakup_tty disappearing from underneath us while
> + * we are using it. this can happen when the device physically unplugged,
> + * while in use. it also serialises access to speakup_tty.
> + */
> +static DEFINE_MUTEX(speakup_tty_mutex);
>  
>  static int ser_to_dev(int ser, dev_t *dev_no)
>  {
> @@ -60,8 +65,10 @@ static int spk_ttyio_ldisc_open(struct t
>  
>  static void spk_ttyio_ldisc_close(struct tty_struct *tty)
>  {
> +	mutex_lock(&speakup_tty_mutex);
>  	kfree(speakup_tty->disc_data);
>  	speakup_tty = NULL;
> +	mutex_unlock(&speakup_tty_mutex);
>  }
>  
>  static int spk_ttyio_receive_buf2(struct tty_struct *tty,
> @@ -189,9 +196,11 @@ void spk_ttyio_unregister_ldisc(void)
>  
>  static int spk_ttyio_out(struct spk_synth *in_synth, const char ch)
>  {
> +	mutex_lock(&speakup_tty_mutex);
>  	if (in_synth->alive && speakup_tty && speakup_tty->ops->write) {
>  		int ret = speakup_tty->ops->write(speakup_tty, &ch, 1);
>  
> +		mutex_unlock(&speakup_tty_mutex);
>  		if (ret == 0)
>  			/* No room */
>  			return 0;
> @@ -207,17 +216,50 @@ static int spk_ttyio_out(struct spk_synt
>  		}
>  		return 1;
>  	}
> +
> +	mutex_unlock(&speakup_tty_mutex);
> +	return 0;
> +}
> +
> +static int check_tty(struct tty_struct *tty)
> +{
> +	if (!tty) {
> +		pr_warn("%s: I/O error, deactivating speakup\n",
> +			spk_ttyio_synth->long_name);
> +		/* No synth any more, so nobody will restart TTYs, and we thus
> +		 * need to do it ourselves.  Now that there is no synth we can
> +		 * let application flood anyway
> +		 */
> +		spk_ttyio_synth->alive = 0;
> +		speakup_start_ttys();
> +		return 1;
> +	}
> +
>  	return 0;
>  }
>  
>  static void spk_ttyio_send_xchar(char ch)
>  {
> +	mutex_lock(&speakup_tty_mutex);
> +	if (check_tty(speakup_tty)) {
> +		mutex_unlock(&speakup_tty_mutex);
> +		return;
> +	}
> +
>  	speakup_tty->ops->send_xchar(speakup_tty, ch);
> +	mutex_unlock(&speakup_tty_mutex);
>  }
>  
>  static void spk_ttyio_tiocmset(unsigned int set, unsigned int clear)
>  {
> +	mutex_lock(&speakup_tty_mutex);
> +	if (check_tty(speakup_tty)) {
> +		mutex_unlock(&speakup_tty_mutex);
> +		return;
> +	}
> +
>  	speakup_tty->ops->tiocmset(speakup_tty, set, clear);
> +	mutex_unlock(&speakup_tty_mutex);
>  }
>  
>  static unsigned char ttyio_in(int timeout)
> @@ -257,8 +299,16 @@ static unsigned char spk_ttyio_in_nowait
>  
>  static void spk_ttyio_flush_buffer(void)
>  {
> +	mutex_lock(&speakup_tty_mutex);
> +	if (check_tty(speakup_tty)) {
> +		mutex_unlock(&speakup_tty_mutex);
> +		return;
> +	}
> +
>  	if (speakup_tty->ops->flush_buffer)
>  		speakup_tty->ops->flush_buffer(speakup_tty);
> +
> +	mutex_unlock(&speakup_tty_mutex);
>  }
>  
>  int spk_ttyio_synth_probe(struct spk_synth *synth)
> 

-- 
Samuel
As usual, this being a 1.3.x release, I haven't even compiled this
kernel yet.  So if it works, you should be doubly impressed.
(Linus Torvalds, announcing kernel 1.3.3 on the linux-kernel mailing list.)