[PATCH v3 1/6] rfcomm: Take proper tty_struct references

[PATCH v3 1/6] rfcomm: Take proper tty_struct references

[Gianluca Anzolin]

In net/bluetooth/rfcomm/tty.c the struct tty_struct is used without taking
references. This may lead to a use-after-free of the rfcomm tty.

Fix this by taking references properly, using the tty_port_* helpers when
possible.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b6e44ad..331d207 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -333,10 +333,9 @@ static inline unsigned int rfcomm_room(struct rfcomm_dlc *dlc)
 static void rfcomm_wfree(struct sk_buff *skb)
 {
 	struct rfcomm_dev *dev = (void *) skb->sk;
-	struct tty_struct *tty = dev->port.tty;
 	atomic_sub(skb->truesize, &dev->wmem_alloc);
-	if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags) && tty)
-		tty_wakeup(tty);
+	if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags))
+		tty_port_tty_wakeup(&dev->port);
 	tty_port_put(&dev->port);
 }
 
@@ -410,6 +409,7 @@ static int rfcomm_release_dev(void __user *arg)
 {
 	struct rfcomm_dev_req req;
 	struct rfcomm_dev *dev;
+	struct tty_struct *tty;
 
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
@@ -429,8 +429,11 @@ static int rfcomm_release_dev(void __user *arg)
 		rfcomm_dlc_close(dev->dlc, 0);
 
 	/* Shut down TTY synchronously before freeing rfcomm_dev */
-	if (dev->port.tty)
-		tty_vhangup(dev->port.tty);
+	tty = tty_port_tty_get(&dev->port);
+	if (tty) {
+		tty_vhangup(tty);
+		tty_kref_put(tty);
+	}
 
 	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
 		rfcomm_dev_del(dev);
@@ -563,6 +566,7 @@ static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc, struct sk_buff *skb)
 static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 {
 	struct rfcomm_dev *dev = dlc->owner;
+	struct tty_struct *tty;
 	if (!dev)
 		return;
 
@@ -572,7 +576,8 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 	wake_up_interruptible(&dev->wait);
 
 	if (dlc->state == BT_CLOSED) {
-		if (!dev->port.tty) {
+		tty = tty_port_tty_get(&dev->port);
+		if (!tty) {
 			if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
 				/* Drop DLC lock here to avoid deadlock
 				 * 1. rfcomm_dev_get will take rfcomm_dev_lock
@@ -591,8 +596,10 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 				tty_port_put(&dev->port);
 				rfcomm_dlc_lock(dlc);
 			}
-		} else
-			tty_hangup(dev->port.tty);
+		} else {
+			tty_hangup(tty);
+			tty_kref_put(tty);
+		}
 	}
 }
 
@@ -604,10 +611,8 @@ static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig)
 
 	BT_DBG("dlc %p dev %p v24_sig 0x%02x", dlc, dev, v24_sig);
 
-	if ((dev->modem_status & TIOCM_CD) && !(v24_sig & RFCOMM_V24_DV)) {
-		if (dev->port.tty && !C_CLOCAL(dev->port.tty))
-			tty_hangup(dev->port.tty);
-	}
+	if ((dev->modem_status & TIOCM_CD) && !(v24_sig & RFCOMM_V24_DV))
+		tty_port_tty_hangup(&dev->port, true);
 
 	dev->modem_status =
 		((v24_sig & RFCOMM_V24_RTC) ? (TIOCM_DSR | TIOCM_DTR) : 0) |
@@ -674,7 +679,7 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
 
 	rfcomm_dlc_lock(dlc);
 	tty->driver_data = dev;
-	dev->port.tty = tty;
+	tty_port_tty_set(&dev->port, tty);
 	rfcomm_dlc_unlock(dlc);
 	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
 
@@ -742,7 +747,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
 
 		rfcomm_dlc_lock(dev->dlc);
 		tty->driver_data = NULL;
-		dev->port.tty = NULL;
+		tty_port_tty_set(&dev->port, NULL);
 		rfcomm_dlc_unlock(dev->dlc);
 
 		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags)) {
-- 
1.8.3.3

[PATCH v3 2/6] rfcomm: Remove the device from the list in the destructor

[Gianluca Anzolin]

The current code removes the device from the device list in several
places. Instead of that do it only in the destructor and in the error path of
rfcomm_add_dev() if the device couldn't be initialized.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 331d207..f5c2a0b 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -76,13 +76,6 @@ static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig);
 
 /* ---- Device functions ---- */
 
-/*
- * The reason this isn't actually a race, as you no doubt have a little voice
- * screaming at you in your head, is that the refcount should never actually
- * reach zero unless the device has already been taken off the list, in
- * rfcomm_dev_del(). And if that's not true, we'll hit the BUG() in
- * rfcomm_dev_destruct() anyway.
- */
 static void rfcomm_dev_destruct(struct tty_port *port)
 {
 	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
@@ -90,10 +83,9 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	BT_DBG("dev %p dlc %p", dev, dlc);
 
-	/* Refcount should only hit zero when called from rfcomm_dev_del()
-	   which will have taken us off the list. Everything else are
-	   refcounting bugs. */
-	BUG_ON(!list_empty(&dev->list));
+	spin_lock(&rfcomm_dev_lock);
+	list_del(&dev->list);
+	spin_unlock(&rfcomm_dev_lock);
 
 	rfcomm_dlc_lock(dlc);
 	/* Detach DLC if it's owned by this dev */
@@ -282,7 +274,9 @@ out:
 			dev->id, NULL);
 	if (IS_ERR(dev->tty_dev)) {
 		err = PTR_ERR(dev->tty_dev);
+		spin_lock(&rfcomm_dev_lock);
 		list_del(&dev->list);
+		spin_unlock(&rfcomm_dev_lock);
 		goto free;
 	}
 
@@ -315,10 +309,6 @@ static void rfcomm_dev_del(struct rfcomm_dev *dev)
 	}
 	spin_unlock_irqrestore(&dev->port.lock, flags);
 
-	spin_lock(&rfcomm_dev_lock);
-	list_del_init(&dev->list);
-	spin_unlock(&rfcomm_dev_lock);
-
 	tty_port_put(&dev->port);
 }
 
@@ -750,13 +740,8 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
 		tty_port_tty_set(&dev->port, NULL);
 		rfcomm_dlc_unlock(dev->dlc);
 
-		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags)) {
-			spin_lock(&rfcomm_dev_lock);
-			list_del_init(&dev->list);
-			spin_unlock(&rfcomm_dev_lock);
-
+		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
 			tty_port_put(&dev->port);
-		}
 	} else
 		spin_unlock_irqrestore(&dev->port.lock, flags);
 
-- 
1.8.3.3

[PATCH v3 3/6] rfcomm: Move the tty initialization and cleanup out of open/close

[Gianluca Anzolin]

Move the tty_struct initialization from rfcomm_tty_open to rfcomm_tty_install
and do the same for the cleanup moving the code from rfcomm_tty_close to
rfcomm_tty_cleanup.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 108 ++++++++++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 40 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index f5c2a0b..cd37f9b 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -633,49 +633,62 @@ static void rfcomm_tty_copy_pending(struct rfcomm_dev *dev)
 		tty_flip_buffer_push(&dev->port);
 }
 
-static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
+/* do the reverse of install, clearing the tty fields and releasing the
+ * reference to tty_port
+ */
+static void rfcomm_tty_cleanup(struct tty_struct *tty)
+{
+	struct rfcomm_dev *dev = tty->driver_data;
+
+	if (dev->tty_dev->parent)
+		device_move(dev->tty_dev, NULL, DPM_ORDER_DEV_LAST);
+
+	/* Close DLC and dettach TTY */
+	rfcomm_dlc_close(dev->dlc, 0);
+
+	clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
+
+	rfcomm_dlc_lock(dev->dlc);
+	tty->driver_data = NULL;
+	rfcomm_dlc_unlock(dev->dlc);
+
+	tty_port_put(&dev->port);
+}
+
+/* we acquire the tty_port reference since it's here the tty is first used
+ * by setting the termios. We also populate the driver_data field and install
+ * the tty port
+ */
+static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 {
 	DECLARE_WAITQUEUE(wait, current);
 	struct rfcomm_dev *dev;
 	struct rfcomm_dlc *dlc;
-	unsigned long flags;
-	int err, id;
-
-	id = tty->index;
+	int err;
 
-	BT_DBG("tty %p id %d", tty, id);
-
-	/* We don't leak this refcount. For reasons which are not entirely
-	   clear, the TTY layer will call our ->close() method even if the
-	   open fails. We decrease the refcount there, and decreasing it
-	   here too would cause breakage. */
-	dev = rfcomm_dev_get(id);
+	dev = rfcomm_dev_get(tty->index);
 	if (!dev)
 		return -ENODEV;
 
 	BT_DBG("dev %p dst %pMR channel %d opened %d", dev, &dev->dst,
 	       dev->channel, dev->port.count);
 
-	spin_lock_irqsave(&dev->port.lock, flags);
-	if (++dev->port.count > 1) {
-		spin_unlock_irqrestore(&dev->port.lock, flags);
-		return 0;
-	}
-	spin_unlock_irqrestore(&dev->port.lock, flags);
-
 	dlc = dev->dlc;
 
 	/* Attach TTY and open DLC */
-
 	rfcomm_dlc_lock(dlc);
 	tty->driver_data = dev;
-	tty_port_tty_set(&dev->port, tty);
 	rfcomm_dlc_unlock(dlc);
 	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
 
+	/* install the tty_port */
+	err = tty_port_install(&dev->port, driver, tty);
+	if (err < 0)
+		goto error_no_dlc;
+
 	err = rfcomm_dlc_open(dlc, &dev->src, &dev->dst, dev->channel);
 	if (err < 0)
-		return err;
+		goto error_no_dlc;
 
 	/* Wait for DLC to connect */
 	add_wait_queue(&dev->wait, &wait);
@@ -702,15 +715,42 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
 	set_current_state(TASK_RUNNING);
 	remove_wait_queue(&dev->wait, &wait);
 
-	if (err == 0)
-		device_move(dev->tty_dev, rfcomm_get_device(dev),
-			    DPM_ORDER_DEV_AFTER_PARENT);
+	if (err < 0)
+		goto error_no_connection;
+
+	device_move(dev->tty_dev, rfcomm_get_device(dev),
+		    DPM_ORDER_DEV_AFTER_PARENT);
+	return 0;
+
+error_no_connection:
+	rfcomm_dlc_close(dlc, err);
+error_no_dlc:
+	clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
+	tty_port_put(&dev->port);
+	return err;
+}
+
+static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
+{
+	struct rfcomm_dev *dev = tty->driver_data;
+	unsigned long flags;
+
+	BT_DBG("tty %p id %d", tty, tty->index);
+
+	spin_lock_irqsave(&dev->port.lock, flags);
+	dev->port.count++;
+	spin_unlock_irqrestore(&dev->port.lock, flags);
 
+	/*
+	 * FIXME: rfcomm should use proper flow control for
+	 * received data. This hack will be unnecessary and can
+	 * be removed when that's implemented
+	 */
 	rfcomm_tty_copy_pending(dev);
 
 	rfcomm_dlc_unthrottle(dev->dlc);
 
-	return err;
+	return 0;
 }
 
 static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
@@ -727,25 +767,11 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
 	spin_lock_irqsave(&dev->port.lock, flags);
 	if (!--dev->port.count) {
 		spin_unlock_irqrestore(&dev->port.lock, flags);
-		if (dev->tty_dev->parent)
-			device_move(dev->tty_dev, NULL, DPM_ORDER_DEV_LAST);
-
-		/* Close DLC and dettach TTY */
-		rfcomm_dlc_close(dev->dlc, 0);
-
-		clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
-
-		rfcomm_dlc_lock(dev->dlc);
-		tty->driver_data = NULL;
-		tty_port_tty_set(&dev->port, NULL);
-		rfcomm_dlc_unlock(dev->dlc);
 
 		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
 			tty_port_put(&dev->port);
 	} else
 		spin_unlock_irqrestore(&dev->port.lock, flags);
-
-	tty_port_put(&dev->port);
 }
 
 static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
@@ -1118,6 +1144,8 @@ static const struct tty_operations rfcomm_ops = {
 	.wait_until_sent	= rfcomm_tty_wait_until_sent,
 	.tiocmget		= rfcomm_tty_tiocmget,
 	.tiocmset		= rfcomm_tty_tiocmset,
+	.install                = rfcomm_tty_install,
+	.cleanup                = rfcomm_tty_cleanup,
 };
 
 int __init rfcomm_init_ttys(void)
-- 
1.8.3.3

[PATCH v3 4/6] rfcomm: Implement .activate, .shutdown and .carrier_raised methods

[Gianluca Anzolin]

Implement .activate, .shutdown and .carrier_raised methods of tty_port to
manage the dlc, moving the code from rfcomm_tty_install() and
rfcomm_tty_cleanup() functions.

At the same time the tty_open close and hangup functions are changed to use the
tty_port helpers that properly call the aforementioned tty_port methods.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 115 ++++++++++++++++++---------------------------
 1 file changed, 47 insertions(+), 68 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index cd37f9b..24a7c8c 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -58,7 +58,6 @@ struct rfcomm_dev {
 	uint			modem_status;
 
 	struct rfcomm_dlc	*dlc;
-	wait_queue_head_t       wait;
 
 	struct device		*tty_dev;
 
@@ -104,8 +103,39 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 	module_put(THIS_MODULE);
 }
 
+/* device-specific initialization: open the dlc */
+static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
+{
+	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+
+	return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+}
+
+/* we block the open until the dlc->state becomes BT_CONNECTED */
+static int rfcomm_dev_carrier_raised(struct tty_port *port)
+{
+	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+
+	return (dev->dlc->state == BT_CONNECTED);
+}
+
+/* device-specific cleanup: close the dlc */
+static void rfcomm_dev_shutdown(struct tty_port *port)
+{
+	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+
+	if (dev->tty_dev->parent)
+		device_move(dev->tty_dev, NULL, DPM_ORDER_DEV_LAST);
+
+	/* close the dlc */
+	rfcomm_dlc_close(dev->dlc, 0);
+}
+
 static const struct tty_port_operations rfcomm_port_ops = {
 	.destruct = rfcomm_dev_destruct,
+	.activate = rfcomm_dev_activate,
+	.shutdown = rfcomm_dev_shutdown,
+	.carrier_raised = rfcomm_dev_carrier_raised,
 };
 
 static struct rfcomm_dev *__rfcomm_dev_get(int id)
@@ -228,7 +258,6 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 
 	tty_port_init(&dev->port);
 	dev->port.ops = &rfcomm_port_ops;
-	init_waitqueue_head(&dev->wait);
 
 	skb_queue_head_init(&dev->pending);
 
@@ -563,9 +592,12 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 	BT_DBG("dlc %p dev %p err %d", dlc, dev, err);
 
 	dev->err = err;
-	wake_up_interruptible(&dev->wait);
+	if (dlc->state == BT_CONNECTED) {
+		device_move(dev->tty_dev, rfcomm_get_device(dev),
+			    DPM_ORDER_DEV_AFTER_PARENT);
 
-	if (dlc->state == BT_CLOSED) {
+		wake_up_interruptible(&dev->port.open_wait);
+	} else if (dlc->state == BT_CLOSED) {
 		tty = tty_port_tty_get(&dev->port);
 		if (!tty) {
 			if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
@@ -640,12 +672,6 @@ static void rfcomm_tty_cleanup(struct tty_struct *tty)
 {
 	struct rfcomm_dev *dev = tty->driver_data;
 
-	if (dev->tty_dev->parent)
-		device_move(dev->tty_dev, NULL, DPM_ORDER_DEV_LAST);
-
-	/* Close DLC and dettach TTY */
-	rfcomm_dlc_close(dev->dlc, 0);
-
 	clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
 
 	rfcomm_dlc_lock(dev->dlc);
@@ -661,7 +687,6 @@ static void rfcomm_tty_cleanup(struct tty_struct *tty)
  */
 static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 {
-	DECLARE_WAITQUEUE(wait, current);
 	struct rfcomm_dev *dev;
 	struct rfcomm_dlc *dlc;
 	int err;
@@ -683,63 +708,22 @@ static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 
 	/* install the tty_port */
 	err = tty_port_install(&dev->port, driver, tty);
-	if (err < 0)
-		goto error_no_dlc;
-
-	err = rfcomm_dlc_open(dlc, &dev->src, &dev->dst, dev->channel);
-	if (err < 0)
-		goto error_no_dlc;
+	if (err)
+		rfcomm_tty_cleanup(tty);
 
-	/* Wait for DLC to connect */
-	add_wait_queue(&dev->wait, &wait);
-	while (1) {
-		set_current_state(TASK_INTERRUPTIBLE);
-
-		if (dlc->state == BT_CLOSED) {
-			err = -dev->err;
-			break;
-		}
-
-		if (dlc->state == BT_CONNECTED)
-			break;
-
-		if (signal_pending(current)) {
-			err = -EINTR;
-			break;
-		}
-
-		tty_unlock(tty);
-		schedule();
-		tty_lock(tty);
-	}
-	set_current_state(TASK_RUNNING);
-	remove_wait_queue(&dev->wait, &wait);
-
-	if (err < 0)
-		goto error_no_connection;
-
-	device_move(dev->tty_dev, rfcomm_get_device(dev),
-		    DPM_ORDER_DEV_AFTER_PARENT);
-	return 0;
-
-error_no_connection:
-	rfcomm_dlc_close(dlc, err);
-error_no_dlc:
-	clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
-	tty_port_put(&dev->port);
 	return err;
 }
 
 static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct rfcomm_dev *dev = tty->driver_data;
-	unsigned long flags;
+	int err;
 
 	BT_DBG("tty %p id %d", tty, tty->index);
 
-	spin_lock_irqsave(&dev->port.lock, flags);
-	dev->port.count++;
-	spin_unlock_irqrestore(&dev->port.lock, flags);
+	err = tty_port_open(&dev->port, tty, filp);
+	if (err)
+		return err;
 
 	/*
 	 * FIXME: rfcomm should use proper flow control for
@@ -756,7 +740,6 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
 static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
 {
 	struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
-	unsigned long flags;
 
 	if (!dev)
 		return;
@@ -764,14 +747,10 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
 	BT_DBG("tty %p dev %p dlc %p opened %d", tty, dev, dev->dlc,
 						dev->port.count);
 
-	spin_lock_irqsave(&dev->port.lock, flags);
-	if (!--dev->port.count) {
-		spin_unlock_irqrestore(&dev->port.lock, flags);
+	tty_port_close(&dev->port, tty, filp);
 
-		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
-			tty_port_put(&dev->port);
-	} else
-		spin_unlock_irqrestore(&dev->port.lock, flags);
+	if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+		tty_port_put(&dev->port);
 }
 
 static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
@@ -1074,7 +1053,7 @@ static void rfcomm_tty_hangup(struct tty_struct *tty)
 	if (!dev)
 		return;
 
-	rfcomm_tty_flush_buffer(tty);
+	tty_port_hangup(&dev->port);
 
 	if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
 		if (rfcomm_dev_get(dev->id) == NULL)
@@ -1164,7 +1143,7 @@ int __init rfcomm_init_ttys(void)
 	rfcomm_tty_driver->subtype	= SERIAL_TYPE_NORMAL;
 	rfcomm_tty_driver->flags	= TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
 	rfcomm_tty_driver->init_termios	= tty_std_termios;
-	rfcomm_tty_driver->init_termios.c_cflag	= B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+	rfcomm_tty_driver->init_termios.c_cflag	= B9600 | CS8 | CREAD | HUPCL;
 	rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
 	tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
 
-- 
1.8.3.3

[PATCH v3 5/6] rfcomm: Fix the reference counting of tty_port

[Gianluca Anzolin]

The tty_port can be released in two cases: when we get a HUP in the functions
rfcomm_tty_hangup() and rfcomm_dev_state_change(). Or when the user releases
the device in rfcomm_release_dev().

In these cases we set the flag RFCOMM_TTY_RELEASED so that no other function can
get a reference to the tty_port.

The rfcomm_dev_del function is removed becase it isn't used anymore.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 42 ++++++++++--------------------------------
 1 file changed, 10 insertions(+), 32 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 24a7c8c..9def67a 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -324,23 +324,6 @@ free:
 	return err;
 }
 
-static void rfcomm_dev_del(struct rfcomm_dev *dev)
-{
-	unsigned long flags;
-	BT_DBG("dev %p", dev);
-
-	BUG_ON(test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags));
-
-	spin_lock_irqsave(&dev->port.lock, flags);
-	if (dev->port.count > 0) {
-		spin_unlock_irqrestore(&dev->port.lock, flags);
-		return;
-	}
-	spin_unlock_irqrestore(&dev->port.lock, flags);
-
-	tty_port_put(&dev->port);
-}
-
 /* ---- Send buffer ---- */
 static inline unsigned int rfcomm_room(struct rfcomm_dlc *dlc)
 {
@@ -454,8 +437,9 @@ static int rfcomm_release_dev(void __user *arg)
 		tty_kref_put(tty);
 	}
 
-	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
-		rfcomm_dev_del(dev);
+	if (!test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+		tty_port_put(&dev->port);
+
 	tty_port_put(&dev->port);
 	return 0;
 }
@@ -607,6 +591,9 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 				 *    rfcomm_dev_lock -> dlc lock
 				 * 2. tty_port_put will deadlock if it's
 				 *    the last reference
+				 *
+				 * FIXME: when we release the lock anything
+				 * could happen to dev, even its destruction
 				 */
 				rfcomm_dlc_unlock(dlc);
 				if (rfcomm_dev_get(dev->id) == NULL) {
@@ -614,7 +601,9 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 					return;
 				}
 
-				rfcomm_dev_del(dev);
+				set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
+				tty_port_put(&dev->port);
+
 				tty_port_put(&dev->port);
 				rfcomm_dlc_lock(dlc);
 			}
@@ -741,16 +730,10 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
 {
 	struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
 
-	if (!dev)
-		return;
-
 	BT_DBG("tty %p dev %p dlc %p opened %d", tty, dev, dev->dlc,
 						dev->port.count);
 
 	tty_port_close(&dev->port, tty, filp);
-
-	if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
-		tty_port_put(&dev->port);
 }
 
 static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
@@ -1050,15 +1033,10 @@ static void rfcomm_tty_hangup(struct tty_struct *tty)
 
 	BT_DBG("tty %p dev %p", tty, dev);
 
-	if (!dev)
-		return;
-
 	tty_port_hangup(&dev->port);
 
 	if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
-		if (rfcomm_dev_get(dev->id) == NULL)
-			return;
-		rfcomm_dev_del(dev);
+		set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
 		tty_port_put(&dev->port);
 	}
 }
-- 
1.8.3.3

[PATCH v3 6/6] rfcomm: Purge the dlc->tx_queue to avoid circular dependency

[Gianluca Anzolin]

In rfcomm_tty_cleanup we purge the dlc->tx_queue which may contain socket
buffers referencing the tty_port and thus preventing the tty_port destruction.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 9def67a..1c6bd90 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -667,6 +667,12 @@ static void rfcomm_tty_cleanup(struct tty_struct *tty)
 	tty->driver_data = NULL;
 	rfcomm_dlc_unlock(dev->dlc);
 
+	/*
+	 * purge the dlc->tx_queue to avoid circular dependencies
+	 * between dev and dlc
+	 */
+	skb_queue_purge(&dev->dlc->tx_queue);
+
 	tty_port_put(&dev->port);
 }
 
-- 
1.8.3.3

Re: [PATCH v3 1/6] rfcomm: Take proper tty_struct references

[Peter Hurley]

On 07/25/2013 02:32 PM, Gianluca Anzolin wrote:
> In net/bluetooth/rfcomm/tty.c the struct tty_struct is used without taking
> references. This may lead to a use-after-free of the rfcomm tty.
>
> Fix this by taking references properly, using the tty_port_* helpers when
> possible.
>
> Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
> ---
>   net/bluetooth/rfcomm/tty.c | 33 +++++++++++++++++++--------------
>   1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
> index b6e44ad..331d207 100644
> --- a/net/bluetooth/rfcomm/tty.c
> +++ b/net/bluetooth/rfcomm/tty.c
> @@ -333,10 +333,9 @@ static inline unsigned int rfcomm_room(struct rfcomm_dlc *dlc)
>   static void rfcomm_wfree(struct sk_buff *skb)
>   {
>   	struct rfcomm_dev *dev = (void *) skb->sk;
> -	struct tty_struct *tty = dev->port.tty;
>   	atomic_sub(skb->truesize, &dev->wmem_alloc);
> -	if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags) && tty)
> -		tty_wakeup(tty);
> +	if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags))
> +		tty_port_tty_wakeup(&dev->port);
>   	tty_port_put(&dev->port);
>   }
>
> @@ -410,6 +409,7 @@ static int rfcomm_release_dev(void __user *arg)
>   {
>   	struct rfcomm_dev_req req;
>   	struct rfcomm_dev *dev;
> +	struct tty_struct *tty;
>
>   	if (copy_from_user(&req, arg, sizeof(req)))
>   		return -EFAULT;
> @@ -429,8 +429,11 @@ static int rfcomm_release_dev(void __user *arg)
>   		rfcomm_dlc_close(dev->dlc, 0);
>
>   	/* Shut down TTY synchronously before freeing rfcomm_dev */
> -	if (dev->port.tty)
> -		tty_vhangup(dev->port.tty);
> +	tty = tty_port_tty_get(&dev->port);
> +	if (tty) {
> +		tty_vhangup(tty);
> +		tty_kref_put(tty);
> +	}
>
>   	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
>   		rfcomm_dev_del(dev);
> @@ -563,6 +566,7 @@ static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc, struct sk_buff *skb)
>   static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
>   {
>   	struct rfcomm_dev *dev = dlc->owner;
> +	struct tty_struct *tty;
>   	if (!dev)
>   		return;
>
> @@ -572,7 +576,8 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
>   	wake_up_interruptible(&dev->wait);
>
>   	if (dlc->state == BT_CLOSED) {
> -		if (!dev->port.tty) {
> +		tty = tty_port_tty_get(&dev->port);
> +		if (!tty) {
>   			if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
>   				/* Drop DLC lock here to avoid deadlock
>   				 * 1. rfcomm_dev_get will take rfcomm_dev_lock
> @@ -591,8 +596,10 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
>   				tty_port_put(&dev->port);
>   				rfcomm_dlc_lock(dlc);
>   			}
> -		} else
> -			tty_hangup(dev->port.tty);
> +		} else {
> +			tty_hangup(tty);
> +			tty_kref_put(tty);
> +		}
>   	}
>   }
>
> @@ -604,10 +611,8 @@ static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig)
>
>   	BT_DBG("dlc %p dev %p v24_sig 0x%02x", dlc, dev, v24_sig);
>
> -	if ((dev->modem_status & TIOCM_CD) && !(v24_sig & RFCOMM_V24_DV)) {
> -		if (dev->port.tty && !C_CLOCAL(dev->port.tty))
> -			tty_hangup(dev->port.tty);
> -	}
> +	if ((dev->modem_status & TIOCM_CD) && !(v24_sig & RFCOMM_V24_DV))
> +		tty_port_tty_hangup(&dev->port, true);
>
>   	dev->modem_status =
>   		((v24_sig & RFCOMM_V24_RTC) ? (TIOCM_DSR | TIOCM_DTR) : 0) |
> @@ -674,7 +679,7 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
>
>   	rfcomm_dlc_lock(dlc);
>   	tty->driver_data = dev;
> -	dev->port.tty = tty;
> +	tty_port_tty_set(&dev->port, tty);

Although strictly speaking, this is correct, I would drop this change
because its functionality is replaced in 4/6 with the call to tty_port_open().
If you want, you could note in the commit message that the
raw assignments in rfcomm_tty_open/close are addressed in commit
'rfcomm: Implement .activate, .shutdown and .carrier_raised methods'.

[ BTW, you remove this line in 3/6 but it's needed until 4/6]

>   	rfcomm_dlc_unlock(dlc);
>   	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
>
> @@ -742,7 +747,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
>
>   		rfcomm_dlc_lock(dev->dlc);
>   		tty->driver_data = NULL;
> -		dev->port.tty = NULL;
> +		tty_port_tty_set(&dev->port, NULL);

Similarly, the call to tty_port_close() in 4/6 replaces this functionality.

Regards,
Peter Hurley

Re: [PATCH v3 3/6] rfcomm: Move the tty initialization and cleanup out of open/close

[Peter Hurley]

On 07/25/2013 02:32 PM, Gianluca Anzolin wrote:
> Move the tty_struct initialization from rfcomm_tty_open to rfcomm_tty_install
> and do the same for the cleanup moving the code from rfcomm_tty_close to
> rfcomm_tty_cleanup.

I would add here a comment regarding the necessity for the extra error
handling required in rfcomm_tty_install because, unlike .open()/.close(),
.cleanup() is not called if .install() fails.

> Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
> ---
>   net/bluetooth/rfcomm/tty.c | 108 ++++++++++++++++++++++++++++-----------------
>   1 file changed, 68 insertions(+), 40 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
> index f5c2a0b..cd37f9b 100644
> --- a/net/bluetooth/rfcomm/tty.c
> +++ b/net/bluetooth/rfcomm/tty.c
> @@ -633,49 +633,62 @@ static void rfcomm_tty_copy_pending(struct rfcomm_dev *dev)
>   		tty_flip_buffer_push(&dev->port);
>   }
>
> -static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
> +/* do the reverse of install, clearing the tty fields and releasing the
> + * reference to tty_port
> + */
> +static void rfcomm_tty_cleanup(struct tty_struct *tty)
> +{
> +	struct rfcomm_dev *dev = tty->driver_data;
> +
> +	if (dev->tty_dev->parent)
> +		device_move(dev->tty_dev, NULL, DPM_ORDER_DEV_LAST);
> +
> +	/* Close DLC and dettach TTY */
> +	rfcomm_dlc_close(dev->dlc, 0);
> +
> +	clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> +
> +	rfcomm_dlc_lock(dev->dlc);
> +	tty->driver_data = NULL;
> +	rfcomm_dlc_unlock(dev->dlc);
> +
> +	tty_port_put(&dev->port);
> +}
> +
> +/* we acquire the tty_port reference since it's here the tty is first used
> + * by setting the termios. We also populate the driver_data field and install
> + * the tty port
> + */
> +static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>   {
>   	DECLARE_WAITQUEUE(wait, current);
>   	struct rfcomm_dev *dev;
>   	struct rfcomm_dlc *dlc;
> -	unsigned long flags;
> -	int err, id;
> -
> -	id = tty->index;
> +	int err;
>
> -	BT_DBG("tty %p id %d", tty, id);
> -
> -	/* We don't leak this refcount. For reasons which are not entirely
> -	   clear, the TTY layer will call our ->close() method even if the
> -	   open fails. We decrease the refcount there, and decreasing it
> -	   here too would cause breakage. */
> -	dev = rfcomm_dev_get(id);
> +	dev = rfcomm_dev_get(tty->index);
>   	if (!dev)
>   		return -ENODEV;
>
>   	BT_DBG("dev %p dst %pMR channel %d opened %d", dev, &dev->dst,
>   	       dev->channel, dev->port.count);
>
> -	spin_lock_irqsave(&dev->port.lock, flags);
> -	if (++dev->port.count > 1) {
> -		spin_unlock_irqrestore(&dev->port.lock, flags);
> -		return 0;
> -	}
> -	spin_unlock_irqrestore(&dev->port.lock, flags);
> -
>   	dlc = dev->dlc;
>
>   	/* Attach TTY and open DLC */
> -
>   	rfcomm_dlc_lock(dlc);
>   	tty->driver_data = dev;
> -	tty_port_tty_set(&dev->port, tty);

As noted in my comment to 1/6, this line (or its equivalent) needs
to be preserved until its functionality is replaced by the call to
tty_port_open()/close() in 4/6


>   	rfcomm_dlc_unlock(dlc);
>   	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
>
> +	/* install the tty_port */
> +	err = tty_port_install(&dev->port, driver, tty);
> +	if (err < 0)
> +		goto error_no_dlc;
> +
>   	err = rfcomm_dlc_open(dlc, &dev->src, &dev->dst, dev->channel);
>   	if (err < 0)
> -		return err;
> +		goto error_no_dlc;
>
>   	/* Wait for DLC to connect */
>   	add_wait_queue(&dev->wait, &wait);
> @@ -702,15 +715,42 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
>   	set_current_state(TASK_RUNNING);
>   	remove_wait_queue(&dev->wait, &wait);
>
> -	if (err == 0)
> -		device_move(dev->tty_dev, rfcomm_get_device(dev),
> -			    DPM_ORDER_DEV_AFTER_PARENT);
> +	if (err < 0)
> +		goto error_no_connection;
> +
> +	device_move(dev->tty_dev, rfcomm_get_device(dev),
> +		    DPM_ORDER_DEV_AFTER_PARENT);
> +	return 0;
> +
> +error_no_connection:
> +	rfcomm_dlc_close(dlc, err);
> +error_no_dlc:
> +	clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> +	tty_port_put(&dev->port);
> +	return err;
> +}
> +
> +static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
> +{
> +	struct rfcomm_dev *dev = tty->driver_data;
> +	unsigned long flags;
> +
> +	BT_DBG("tty %p id %d", tty, tty->index);
> +
> +	spin_lock_irqsave(&dev->port.lock, flags);
> +	dev->port.count++;
> +	spin_unlock_irqrestore(&dev->port.lock, flags);
>
> +	/*
> +	 * FIXME: rfcomm should use proper flow control for
> +	 * received data. This hack will be unnecessary and can
> +	 * be removed when that's implemented
> +	 */
>   	rfcomm_tty_copy_pending(dev);
>
>   	rfcomm_dlc_unthrottle(dev->dlc);
>
> -	return err;
> +	return 0;
>   }
>
>   static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
> @@ -727,25 +767,11 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
>   	spin_lock_irqsave(&dev->port.lock, flags);
>   	if (!--dev->port.count) {
>   		spin_unlock_irqrestore(&dev->port.lock, flags);
> -		if (dev->tty_dev->parent)
> -			device_move(dev->tty_dev, NULL, DPM_ORDER_DEV_LAST);
> -
> -		/* Close DLC and dettach TTY */
> -		rfcomm_dlc_close(dev->dlc, 0);
> -
> -		clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> -
> -		rfcomm_dlc_lock(dev->dlc);
> -		tty->driver_data = NULL;

> -		tty_port_tty_set(&dev->port, NULL);

Similarly with this line.

> -		rfcomm_dlc_unlock(dev->dlc);
>
>   		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
>   			tty_port_put(&dev->port);
>   	} else
>   		spin_unlock_irqrestore(&dev->port.lock, flags);
> -
> -	tty_port_put(&dev->port);
>   }
>
>   static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, int count)
> @@ -1118,6 +1144,8 @@ static const struct tty_operations rfcomm_ops = {
>   	.wait_until_sent	= rfcomm_tty_wait_until_sent,
>   	.tiocmget		= rfcomm_tty_tiocmget,
>   	.tiocmset		= rfcomm_tty_tiocmset,
> +	.install                = rfcomm_tty_install,
> +	.cleanup                = rfcomm_tty_cleanup,
>   };
>
>   int __init rfcomm_init_ttys(void)
>

Re: [PATCH v3 1/6] rfcomm: Take proper tty_struct references

[Gianluca Anzolin]

On Fri, Jul 26, 2013 at 08:07:30AM -0400, Peter Hurley wrote:
> On 07/25/2013 02:32 PM, Gianluca Anzolin wrote:
> >
> >  	rfcomm_dlc_lock(dlc);
> >  	tty->driver_data = dev;
> >-	dev->port.tty = tty;
> >+	tty_port_tty_set(&dev->port, tty);
> 
> Although strictly speaking, this is correct, I would drop this change
> because its functionality is replaced in 4/6 with the call to tty_port_open().
> If you want, you could note in the commit message that the
> raw assignments in rfcomm_tty_open/close are addressed in commit
> 'rfcomm: Implement .activate, .shutdown and .carrier_raised methods'.

Ok I will do that.

> 
> [ BTW, you remove this line in 3/6 but it's needed until 4/6]

Oh, I overlooked that "detail" when I changed the order of the patches...

> 
> >  	rfcomm_dlc_unlock(dlc);
> >  	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
> >
> >@@ -742,7 +747,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
> >
> >  		rfcomm_dlc_lock(dev->dlc);
> >  		tty->driver_data = NULL;
> >-		dev->port.tty = NULL;
> >+		tty_port_tty_set(&dev->port, NULL);
> 
> Similarly, the call to tty_port_close() in 4/6 replaces this functionality.
> 
> Regards,
> Peter Hurley
> 

Ok there is a need for a v4 after all.

Btw did you look at the RFC patch about the dev_add nested locks?
Do you think it's acceptable? I tried to add the device to the list at the end
of the function but the fact is that the dlc callbacks need dev->id so I had to
allocate it or set it to something not valid, like -1.

I didn't also get any reply about the skb_queue_purge patch, I hope it's ok

Thank you,
Gianluca

Re: [PATCH v3 1/6] rfcomm: Take proper tty_struct references

[Peter Hurley]

On 07/26/2013 08:50 AM, Gianluca Anzolin wrote:
> On Fri, Jul 26, 2013 at 08:07:30AM -0400, Peter Hurley wrote:
>> On 07/25/2013 02:32 PM, Gianluca Anzolin wrote:
>>>
>>>   	rfcomm_dlc_lock(dlc);
>>>   	tty->driver_data = dev;
>>> -	dev->port.tty = tty;
>>> +	tty_port_tty_set(&dev->port, tty);
>>
>> Although strictly speaking, this is correct, I would drop this change
>> because its functionality is replaced in 4/6 with the call to tty_port_open().
>> If you want, you could note in the commit message that the
>> raw assignments in rfcomm_tty_open/close are addressed in commit
>> 'rfcomm: Implement .activate, .shutdown and .carrier_raised methods'.
>
> Ok I will do that.
>
>>
>> [ BTW, you remove this line in 3/6 but it's needed until 4/6]
>
> Oh, I overlooked that "detail" when I changed the order of the patches...

Same thing just happened to me with a series of mine.

>>
>>>   	rfcomm_dlc_unlock(dlc);
>>>   	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
>>>
>>> @@ -742,7 +747,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
>>>
>>>   		rfcomm_dlc_lock(dev->dlc);
>>>   		tty->driver_data = NULL;
>>> -		dev->port.tty = NULL;
>>> +		tty_port_tty_set(&dev->port, NULL);
>>
>> Similarly, the call to tty_port_close() in 4/6 replaces this functionality.
>>
>> Regards,
>> Peter Hurley
>>
>
> Ok there is a need for a v4 after all.
>
> Btw did you look at the RFC patch about the dev_add nested locks?
> Do you think it's acceptable? I tried to add the device to the list at the end
> of the function but the fact is that the dlc callbacks need dev->id so I had to
> allocate it or set it to something not valid, like -1.
 >
 > I didn't also get any reply about the skb_queue_purge patch, I hope it's ok

I'm testing the whole patch series now. Assuming nothing breaks and v4 changes
are minimal, I'll give the whole series Reviewed-by: and Tested-by: tags.

When I'm done with that, I'll review the RFC patch.

For v4, you should direct To: Gustavo Padovan, cc: me instead, and add
cc's for Greg Kroah-Hartman <gregkh@linuxfoundation.org> and Jiri Slaby
<jslaby@suse.cz>

Regards,
Peter Hurley


For future reference, for a series like this you might want to consider
adding a cover letter 0/6 (with git format-patch --cover-letter) with an
overview of the whole series.

Cover letters give you an opportunity to explain the motivation
(rfcomm crashes on device disconnect because port reference counting
was broken), explain your solution (implement rfcomm as a proper tty_port),
and allows reviewers to comment on the series as whole (Good work :).