linux-kernel - [patch] staging: speakup: fix async usb removal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20170812080547.GA2018@sanghar>
Date:   Sat, 12 Aug 2017 09:05:47 +0100
From:   Okash Khawaja <okash.khawaja@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Samuel Thibault <samuel.thibault@...-lyon.org>,
        linux-kernel@...r.kernel.org
Cc:     William Hubbs <w.d.hubbs@...il.com>,
        Chris Brannon <chris@...-brannons.com>,
        Kirk Reiser <kirk@...sers.ca>, speakup@...ux-speakup.org,
        devel@...verdev.osuosl.org
Subject: [patch] staging: speakup: fix async usb removal

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 restarts
ttys in case they were stopped by speakup.

Signed-off-by: Okash Khawaja <okash.khawaja@...il.com>
Reviewed-by: Samuel Thibault <samuel.thibault@...-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)