bluetooth: Fix WARNING in tty_set_termios()

PATCH Superseded Shuah Khan 2 participants 3 messages Feb 3, 2019 Bluetooth drivers

lore

Share & tools

lore

Shuah Khan

author

2019-02-03 16:48 UTC #0 lore raw

PATCH v3 1/1 bluetooth: Fix WARNING in tty_set_termios()

tty_set_termios() has the following WARN_ON which can be triggered with a syscall to invoke TIOCSETD __NR_ioctl.

WARN_ON(tty->driver->type == TTY_DRIVER_TYPE_PTY &&

                tty->driver->subtype == PTY_TYPE_MASTER);

Reference: https://syzkaller.appspot.com/bug?id=2410d22f1d8e5984217329dd0884b01d99e3e48d

Johan Hovold said: "The problemm started with
commit 7721383f4199 ("Bluetooth: hci_uart: Support operational speed during setup") which introduced a new way for how tty_set_termios() could end up being called for a master pty."

Fix it by by preventing setting the HCI line discipline for PTYs in hci_uart_tty_open(). Looked into keying off of tty and ldisc ops, and couldn't find any that would be conclusive. Checking tty as such clearly tags the reason for rejecting the request to set ldisc.

Reported-by [email protected]

Cc Johan Hovold <[email protected]>

Cc Marcel Holtmann <[email protected]>

Cc Al Viro <[email protected]>

Signed-off-by Shuah Khan <[email protected]>

---
 drivers/bluetooth/hci_ldisc.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/bluetooth/hci_ldisc.c +5 -0

index fbf7b4df23ab..a3d313fcc0f2 100644
@@ -480,6 +480,11 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	if (tty->ops->write == NULL)
 		return -EOPNOTSUPP;
 
+	/* don't set HCI line discipline on PTYs */
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
+	    tty->driver->subtype == PTY_TYPE_MASTER)
+		return -EINVAL;
+
 	hu = kzalloc(sizeof(struct hci_uart), GFP_KERNEL);
 	if (!hu) {
 		BT_ERR("Can't allocate control structure");

Signature · 1 line

2.17.1

Marcel Holtmann

maintainer

2019-02-03 17:31 UTC #1 lore raw

Hi Shuah,

Quoted 31 lines

tty_set_termios() has the following WARN_ON which can be triggered with a syscall to invoke TIOCSETD __NR_ioctl.
WARN_ON(tty->driver->type == TTY_DRIVER_TYPE_PTY &&
               tty->driver->subtype == PTY_TYPE_MASTER);
Reference: https://syzkaller.appspot.com/bug?id=2410d22f1d8e5984217329dd0884b01d99e3e48d
Johan Hovold said: "The problemm started with
commit 7721383f4199 ("Bluetooth: hci_uart: Support operational speed during setup") which introduced a new way for how tty_set_termios() could end up being called for a master pty."
Fix it by by preventing setting the HCI line discipline for PTYs in hci_uart_tty_open(). Looked into keying off of tty and ldisc ops, and couldn't find any that would be conclusive. Checking tty as such clearly tags the reason for rejecting the request to set ldisc.
Reported-by [email protected]
Cc Johan Hovold <[email protected]>
Cc Marcel Holtmann <[email protected]>
Cc Al Viro <[email protected]>
Signed-off-by Shuah Khan <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 5 +++++
1 file changed, 5 insertions(+)
drivers/bluetooth/hci_ldisc.c +0 -0
index fbf7b4df23ab..a3d313fcc0f2 100644

drivers/bluetooth/hci_ldisc.c static int hci_uart_tty_open(struct tty_struct *tty) · line 487

Quoted 8 lines

drivers/bluetooth/hci_ldisc.c +5 -0

@@ -480,6 +480,11 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	if (tty->ops->write == NULL)
 		return -EOPNOTSUPP;
 
+	/* don't set HCI line discipline on PTYs */
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
+	    tty->driver->subtype == PTY_TYPE_MASTER)
+		return -EINVAL;
+

this is turning in circles. What is wrong with checking !tty->ops->set_termios here?

Regards

Marcel

shuah

author

2019-02-03 19:36 UTC #2 lore raw

Quoted 36 lines · Marcel Holtmann

Hi Shuah,
Quoted 34 lines
tty_set_termios() has the following WARN_ON which can be triggered with a syscall to invoke TIOCSETD __NR_ioctl.
WARN_ON(tty->driver->type == TTY_DRIVER_TYPE_PTY &&
                tty->driver->subtype == PTY_TYPE_MASTER);
Reference: https://syzkaller.appspot.com/bug?id=2410d22f1d8e5984217329dd0884b01d99e3e48d
Johan Hovold said: "The problemm started with
commit 7721383f4199 ("Bluetooth: hci_uart: Support operational speed during setup") which introduced a new way for how tty_set_termios() could end up being called for a master pty."
Fix it by by preventing setting the HCI line discipline for PTYs in hci_uart_tty_open(). Looked into keying off of tty and ldisc ops, and couldn't find any that would be conclusive. Checking tty as such clearly tags the reason for rejecting the request to set ldisc.
Reported-by [email protected]
Cc Johan Hovold <[email protected]>
Cc Marcel Holtmann <[email protected]>
Cc Al Viro <[email protected]>
Signed-off-by Shuah Khan <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 5 +++++
1 file changed, 5 insertions(+)
drivers/bluetooth/hci_ldisc.c +0 -0
index fbf7b4df23ab..a3d313fcc0f2 100644
@@ -480,6 +480,11 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	if (tty->ops->write == NULL)
 		return -EOPNOTSUPP;
 

Quoted 7 lines

drivers/bluetooth/hci_ldisc.c static int hci_uart_tty_open(struct tty_struct *tty) · line 487
Quoted 5 lines
drivers/bluetooth/hci_ldisc.c +5 -0
+	/* don't set HCI line discipline on PTYs */
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
+	    tty->driver->subtype == PTY_TYPE_MASTER)
+		return -EINVAL;
+
this is turning in circles. What is wrong with checking !tty->ops->set_termios here?

Yeah. I looked into set_termios and thought that it is set in this path. My bad. okay v4 is on its way. Sorry it took so long to get on the same page with you. :(

thanks,
-- Shuah