[patch 0/1] fix async usb removal

Samuel Thibault samuel.thibault at ens-lyon.org
Mon Aug 7 17:18:44 EDT 2017


Hello,

Okash Khawaja, on sam. 05 août 2017 18:56:00 +0100, wrote:
> I have tested this patch with apollo and it correctly deactivates the
> module when usb is asynchronously unplugged. On unplugging, the tty
> becomes null so using it caused null pointer deref. When this happens
> in the context of catch_up kthread, it leads to kernel lock up a little
> while later.

> +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)
>  {
> +	if (check_tty(speakup_tty))
> +		return;
> +
>  	speakup_tty->ops->send_xchar(speakup_tty, ch);
>  }

This is still unsafe: the unplug might come just between testing for
speakup_tty becoming NULL and using it. Yes, that's very unlikely, but
that's still not acceptable :)

You should rather find out through which path the speakup_tty variable
becomes NULL, and make speakup non-alive there *before* setting it to
NULL.

Samuel


More information about the Speakup mailing list