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=1b42faa2848963564a5b1b7f8c837ea7b55ffa5... 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;
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
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
linux-stable-mirror@lists.linaro.org