[PATCH] Bluetooth: hci_ldisc: check for missing tty operations

[PATCH] Bluetooth: hci_ldisc: check for missing tty operations

[Vladis Dronov]

Certain ttys lack operations (eg: pty_unix98_ops) which still can be
called by certain hci uart proto (eg: mrvl, ath). Currently this leads
to execution at address 0x0. Fix this by adding checks for missing tty
operations.

Link: https://syzkaller.appspot.com/bug?id=1b42faa2848963564a5b1b7f8c837ea7b55ffa50
Reported-by: syzbot+79337b501d6aa974d0f6@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org # v2.6.39+
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
 drivers/bluetooth/hci_ath.c   |  7 ++++-
 drivers/bluetooth/hci_ldisc.c | 58 ++++++++++++++++++++---------------
 2 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
index a55be205b91a..92d9bded5880 100644
--- a/drivers/bluetooth/hci_ath.c
+++ b/drivers/bluetooth/hci_ath.c
@@ -49,7 +49,12 @@ struct ath_vendor_cmd {
 
 static int ath_wakeup_ar3k(struct tty_struct *tty)
 {
-	int status = tty->driver->ops->tiocmget(tty);
+	int status;
+
+	if (!(tty->driver->ops->tiocmget && tty->driver->ops->tiocmset))
+		return TIOCM_CTS;
+
+	status = tty->driver->ops->tiocmget(tty);
 
 	if (status & TIOCM_CTS)
 		return status;
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index c84f985f348d..29b4970f9bca 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -307,32 +307,40 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
 		BT_DBG("Disabling hardware flow control: %s",
 		       status ? "failed" : "success");
 
-		/* Clear RTS to prevent the device from sending */
-		/* Most UARTs need OUT2 to enable interrupts */
-		status = tty->driver->ops->tiocmget(tty);
-		BT_DBG("Current tiocm 0x%x", status);
-
-		set &= ~(TIOCM_OUT2 | TIOCM_RTS);
-		clear = ~set;
-		set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
-		       TIOCM_OUT2 | TIOCM_LOOP;
-		clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
-			 TIOCM_OUT2 | TIOCM_LOOP;
-		status = tty->driver->ops->tiocmset(tty, set, clear);
-		BT_DBG("Clearing RTS: %s", status ? "failed" : "success");
+		if (tty->driver->ops->tiocmget && tty->driver->ops->tiocmset) {
+			/* Clear RTS to prevent the device from sending */
+			/* Most UARTs need OUT2 to enable interrupts */
+			status = tty->driver->ops->tiocmget(tty);
+			BT_DBG("Current tiocm 0x%x", status);
+
+			set &= ~(TIOCM_OUT2 | TIOCM_RTS);
+			clear = ~set;
+			set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+			       TIOCM_OUT2 | TIOCM_LOOP;
+			clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+			         TIOCM_OUT2 | TIOCM_LOOP;
+			status = tty->driver->ops->tiocmset(tty, set, clear);
+			BT_DBG("Clearing RTS: %s",
+			       status ? "failed" : "success");
+		} else
+			BT_DBG("Terminal RTS control is not present");
 	} else {
-		/* Set RTS to allow the device to send again */
-		status = tty->driver->ops->tiocmget(tty);
-		BT_DBG("Current tiocm 0x%x", status);
-
-		set |= (TIOCM_OUT2 | TIOCM_RTS);
-		clear = ~set;
-		set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
-		       TIOCM_OUT2 | TIOCM_LOOP;
-		clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
-			 TIOCM_OUT2 | TIOCM_LOOP;
-		status = tty->driver->ops->tiocmset(tty, set, clear);
-		BT_DBG("Setting RTS: %s", status ? "failed" : "success");
+		if (tty->driver->ops->tiocmget && tty->driver->ops->tiocmset) {
+			/* Set RTS to allow the device to send again */
+			status = tty->driver->ops->tiocmget(tty);
+			BT_DBG("Current tiocm 0x%x", status);
+
+			set |= (TIOCM_OUT2 | TIOCM_RTS);
+			clear = ~set;
+			set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+			       TIOCM_OUT2 | TIOCM_LOOP;
+			clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+			         TIOCM_OUT2 | TIOCM_LOOP;
+			status = tty->driver->ops->tiocmset(tty, set, clear);
+			BT_DBG("Setting RTS: %s",
+			       status ? "failed" : "success");
+		} else
+			BT_DBG("Terminal RTS control is not present");
 
 		/* Re-enable hardware flow control */
 		ktermios = tty->termios;
-- 
2.20.1

Re: [PATCH] Bluetooth: hci_ldisc: check for missing tty operations

[Marcel Holtmann]

Hi Vladis,

> Certain ttys lack operations (eg: pty_unix98_ops) which still can be
> called by certain hci uart proto (eg: mrvl, ath). Currently this leads
> to execution at address 0x0. Fix this by adding checks for missing tty
> operations.

so I really prefer that we just fail setting the line discipline. These drivers need to check that the underlying transport has all the operations available they need. We can not just ignore them.

Regards

Marcel

Re: [PATCH] Bluetooth: hci_ldisc: check for missing tty operations

[Vladis Dronov]

Hello, Marcel,

I totally agree, the same came to my mind some time after sending the patch.
Let me send a v2 in a while with drivers checking for needed tty operations
presence.

Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer

----- Original Message -----
> From: "Marcel Holtmann" <marcel@holtmann.org>
> To: "Vladis Dronov" <vdronov@redhat.com>
> Cc: "Johan Hedberg" <johan.hedberg@gmail.com>, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
> syzbot+79337b501d6aa974d0f6@syzkaller.appspotmail.com, stable@vger.kernel.org, "Loic Poulain"
> <loic.poulain@intel.com>, "Ilya Faenson" <ifaenson@broadcom.com>
> Sent: Saturday, July 6, 2019 12:35:39 PM
> Subject: Re: [PATCH] Bluetooth: hci_ldisc: check for missing tty operations
> 
> Hi Vladis,
> 
> > Certain ttys lack operations (eg: pty_unix98_ops) which still can be
> > called by certain hci uart proto (eg: mrvl, ath). Currently this leads
> > to execution at address 0x0. Fix this by adding checks for missing tty
> > operations.
> 
> so I really prefer that we just fail setting the line discipline. These
> drivers need to check that the underlying transport has all the operations
> available they need. We can not just ignore them.
> 
> Regards
> 
> Marcel