Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Myungho Jung <mhjungk@gmail.com>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: hci_uart: Switch pty driver to slave side in tty_set_termios()
Date: Wed, 30 Jan 2019 11:07:38 +0100
Message-ID: <20190130100738.GQ3691@localhost> (raw)
In-Reply-To: <20190128065259.GA2976@myunghoj-Precision-5530>

On Sun, Jan 27, 2019 at 10:53:02PM -0800, Myungho Jung wrote:
> tty_set_termios() should be called with slave side of pty driver. So, If
> tty driver is pty master, it needs to be switched to ->link.

I'm not sure that's the right solution. PTYs are virtual devices used
for IPC and neither end (master or slave) have support for modem
control or baud rates.

> Reported-by: syzbot+a950165cbb86bdd023a4@syzkaller.appspotmail.com
> Signed-off-by: Myungho Jung <mhjungk@gmail.com>
> ---
>  drivers/bluetooth/hci_ldisc.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index fbf7b4df23ab..90c5ea8c399b 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -299,10 +299,18 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>  	return 0;
>  }
>  
> +/* If driver is pty master, return slave side */
> +static struct tty_struct *hci_uart_get_real_tty(struct tty_struct *tty)
> +{
> +	return  (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
> +		 tty->driver->subtype == PTY_TYPE_MASTER) ? tty->link : tty;
> +}
> +
>  /* Flow control or un-flow control the device */
>  void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>  {
>  	struct tty_struct *tty = hu->tty;
> +	struct tty_struct *real_tty;
>  	struct ktermios ktermios;
>  	int status;
>  	unsigned int set = 0;
> @@ -314,11 +322,12 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>  		return;
>  	}
>  
> +	real_tty = hci_uart_get_real_tty(tty);
>  	if (enable) {
>  		/* Disable hardware flow control */
> -		ktermios = tty->termios;
> +		ktermios = real_tty->termios;
>  		ktermios.c_cflag &= ~CRTSCTS;
> -		status = tty_set_termios(tty, &ktermios);
> +		status = tty_set_termios(real_tty, &ktermios);
>  		BT_DBG("Disabling hardware flow control: %s",
>  		       status ? "failed" : "success");

So instead of these pointless calls to set the slave termios and
modem-control state, you might as well bail out early above (and
similarly in set_baudrate()).

Using n_hci for a master pty really makes no sense at all, so we could
even bail out at ldisc open, but perhaps that can be discussed and
addressed later.

Johan

  parent reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-28  6:53 Myungho Jung
2019-01-28 12:20 ` Marcel Holtmann
2019-01-30 10:07 ` Johan Hovold [this message]
2019-01-31  5:13   ` Myungho Jung
2019-01-31 15:43     ` Johan Hovold

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190130100738.GQ3691@localhost \
    --to=johan@kernel.org \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=mhjungk@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org linux-bluetooth@archiver.kernel.org
	public-inbox-index linux-bluetooth


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/ public-inbox