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
_______________________________________________
Speakup mailing list
Speakup@xxxxxxxxxxxxxxxxx
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
