Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] btusb.c add IMC Networks id 13d3:3404 (bcm20702A0)
From: Jurgen Kramer @ 2014-02-10 11:26 UTC (permalink / raw)
  To: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

Attached patch adds support for IMC Networks (13d3:3404) to BCM20702A0
in btusb.c. This device is used on the Asrock Z87E-ITX motherboard.
Tested against 3.12.9.

diff -uNrp
linux-3.12.9-301.jk1.fc20.x86_64.orig/drivers/bluetooth/btusb.c
linux-3.12.9-301.jk1.fc20.x86_64.new/drivers/bluetooth/btusb.c
--- a/drivers/bluetooth/btusb.c	2014-02-10 11:35:08.976975562 +0100
+++ b/drivers/bluetooth/btusb.c	2014-02-10 11:37:03.864921122 +0100
@@ -106,6 +106,7 @@ static struct usb_device_id btusb_table[
 	{ USB_DEVICE(0x04ca, 0x2003) },
 	{ USB_DEVICE(0x0489, 0xe042) },
 	{ USB_DEVICE(0x413c, 0x8197) },
+	{ USB_DEVICE(0x13d3, 0x3404) },
 
 	/* Foxconn - Hon Hai */
 	{ USB_VENDOR_AND_INTERFACE_INFO(0x0489, 0xff, 0x01, 0x01) },

Signed-off-by: Jurgen Kramer <gtm.kramer@xs4all.nl>

Jurgen

[-- Attachment #2: btusb-bcm20702a0-add-imc-networks.patch --]
[-- Type: text/x-patch, Size: 562 bytes --]

diff -uNrp linux-3.12.9-301.jk1.fc20.x86_64.orig/drivers/bluetooth/btusb.c linux-3.12.9-301.jk1.fc20.x86_64.new/drivers/bluetooth/btusb.c
--- a/drivers/bluetooth/btusb.c	2014-02-10 11:35:08.976975562 +0100
+++ b/drivers/bluetooth/btusb.c	2014-02-10 11:37:03.864921122 +0100
@@ -106,6 +106,7 @@ static struct usb_device_id btusb_table[
 	{ USB_DEVICE(0x04ca, 0x2003) },
 	{ USB_DEVICE(0x0489, 0xe042) },
 	{ USB_DEVICE(0x413c, 0x8197) },
+	{ USB_DEVICE(0x13d3, 0x3404) },
 
 	/* Foxconn - Hon Hai */
 	{ USB_VENDOR_AND_INTERFACE_INFO(0x0489, 0xff, 0x01, 0x01) },

^ permalink raw reply

* [PATCH v2 6/6] android/a2dp: Fix audio deregistration
From: Andrzej Kaczmarek @ 2014-02-10 10:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1392029145-7954-1-git-send-email-andrzej.kaczmarek@tieto.com>

Unregistering a SEP can trigger abort_cfm callback if some device is
connected thus we should free setups list after all endpoints are
unregistered to avoid error in abort_cfm due to non-existing setup.
---
 android/a2dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index bef037d..838ade8 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -1481,12 +1481,12 @@ static void bt_audio_unregister(void)
 	if (audio_retry_id > 0)
 		g_source_remove(audio_retry_id);
 
-	g_slist_free_full(setups, setup_free);
-	setups = NULL;
-
 	g_slist_free_full(endpoints, unregister_endpoint);
 	endpoints = NULL;
 
+	g_slist_free_full(setups, setup_free);
+	setups = NULL;
+
 	audio_ipc_cleanup();
 }
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 5/6] android/a2dp: Disconnect headset on IPC failure
From: Andrzej Kaczmarek @ 2014-02-10 10:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1392029145-7954-1-git-send-email-andrzej.kaczmarek@tieto.com>

In case audio IPC is suddenly disconnected (most likely due to crash of
mediaserver process) we should disconnect headset since it is no longer
associated with valid setup and cannot be used properly.
---
 android/a2dp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 4c6ab18..bef037d 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -1513,6 +1513,7 @@ static gboolean audio_retry_register(void *data)
 
 static void audio_disconnected(void *data)
 {
+	GSList *l;
 	bool restart;
 
 	DBG("");
@@ -1524,6 +1525,12 @@ static void audio_disconnected(void *data)
 
 	bt_audio_unregister();
 
+	for (l = devices; l; l = g_slist_next(l)) {
+		struct a2dp_device *dev = l->data;
+
+		avdtp_shutdown(dev->session);
+	}
+
 	if (!restart)
 		return;
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 4/6] android/hal-audio: Write SBC parameters to logcat
From: Andrzej Kaczmarek @ 2014-02-10 10:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1392029145-7954-1-git-send-email-andrzej.kaczmarek@tieto.com>

---
 android/hal-audio.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 766327b..9312659 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -289,6 +289,78 @@ static int sbc_get_presets(struct audio_preset *preset, size_t *len)
 	return i;
 }
 
+static int sbc_freq2int(uint8_t freq)
+{
+	switch (freq) {
+	case SBC_SAMPLING_FREQ_16000:
+		return 16000;
+	case SBC_SAMPLING_FREQ_32000:
+		return 32000;
+	case SBC_SAMPLING_FREQ_44100:
+		return 44100;
+	case SBC_SAMPLING_FREQ_48000:
+		return 48000;
+	default:
+		return 0;
+	}
+}
+
+static const char *sbc_mode2str(uint8_t mode)
+{
+	switch (mode) {
+	case SBC_CHANNEL_MODE_MONO:
+		return "Mono";
+	case SBC_CHANNEL_MODE_DUAL_CHANNEL:
+		return "DualChannel";
+	case SBC_CHANNEL_MODE_STEREO:
+		return "Stereo";
+	case SBC_CHANNEL_MODE_JOINT_STEREO:
+		return "JointStereo";
+	default:
+		return "(unknown)";
+	}
+}
+
+static int sbc_blocks2int(uint8_t blocks)
+{
+	switch (blocks) {
+	case SBC_BLOCK_LENGTH_4:
+		return 4;
+	case SBC_BLOCK_LENGTH_8:
+		return 8;
+	case SBC_BLOCK_LENGTH_12:
+		return 12;
+	case SBC_BLOCK_LENGTH_16:
+		return 16;
+	default:
+		return 0;
+	}
+}
+
+static int sbc_subbands2int(uint8_t subbands)
+{
+	switch (subbands) {
+	case SBC_SUBBANDS_4:
+		return 4;
+	case SBC_SUBBANDS_8:
+		return 8;
+	default:
+		return 0;
+	}
+}
+
+static const char *sbc_allocation2str(uint8_t allocation)
+{
+	switch (allocation) {
+	case SBC_ALLOCATION_SNR:
+		return "SNR";
+	case SBC_ALLOCATION_LOUDNESS:
+		return "Loudness";
+	default:
+		return "(unknown)";
+	}
+}
+
 static void sbc_init_encoder(struct sbc_data *sbc_data)
 {
 	a2dp_sbc_t *in = &sbc_data->sbc;
@@ -298,6 +370,15 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
 
 	out->endian = SBC_LE;
 	out->bitpool = in->max_bitpool;
+
+	DBG("frequency=%d channel_mode=%s block_length=%d subbands=%d "
+			"allocation=%s bitpool=%d-%d",
+			sbc_freq2int(in->frequency),
+			sbc_mode2str(in->channel_mode),
+			sbc_blocks2int(in->block_length),
+			sbc_subbands2int(in->subbands),
+			sbc_allocation2str(in->allocation_method),
+			in->min_bitpool, in->max_bitpool);
 }
 
 static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 3/6] android/hal-audio: Ignore write call when closing
From: Andrzej Kaczmarek @ 2014-02-10 10:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1392029145-7954-1-git-send-email-andrzej.kaczmarek@tieto.com>

We should not try to neither auto-resume nor write when state is set to
NONE as this is case when we're being closed and it's ok do ignore
write request.
---
 android/hal-audio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index efdf823..766327b 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -831,6 +831,10 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 {
 	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
 
+	/* just return in case we're closing */
+	if (out->audio_state == AUDIO_A2DP_STATE_NONE)
+		return -1;
+
 	/* We can auto-start only from standby */
 	if (out->audio_state == AUDIO_A2DP_STATE_STANDBY) {
 		DBG("stream in standby, auto-start");
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 2/6] android/a2dp: Notify audio state on SEP close
From: Andrzej Kaczmarek @ 2014-02-10 10:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1392029145-7954-1-git-send-email-andrzej.kaczmarek@tieto.com>

---
 android/a2dp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 8cff535..4c6ab18 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -990,6 +990,8 @@ static gboolean sep_close_ind(struct avdtp *session,
 		return FALSE;
 	}
 
+	bt_audio_notify_state(setup, HAL_AUDIO_STOPPED);
+
 	setup_remove(setup);
 
 	return TRUE;
@@ -1163,13 +1165,23 @@ static void sep_close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 			void *user_data)
 {
 	struct a2dp_endpoint *endpoint = user_data;
+	struct a2dp_setup *setup;
 
 	DBG("");
 
 	if (err)
 		return;
 
-	setup_remove_by_id(endpoint->id);
+	setup = find_setup(endpoint->id);
+	if (!setup) {
+		error("Unable to find stream setup for %u endpoint",
+								endpoint->id);
+		return;
+	}
+
+	bt_audio_notify_state(setup, HAL_AUDIO_STOPPED);
+
+	setup_remove(setup);
 }
 
 static void sep_abort_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 1/6] android/a2dp: Shutdown AVDTP gracefully
From: Andrzej Kaczmarek @ 2014-02-10 10:45 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek

When shutting down AVDTP connection we first abort and wait for stream
to go to idle state before disconnecting signalling channel.
---
 android/avdtp.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/android/avdtp.c b/android/avdtp.c
index e26d6ec..2629e67 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -398,6 +398,8 @@ struct avdtp {
 	struct pending_req *req;
 
 	GSList *disconnect;
+
+	bool shutdown;
 };
 
 static GSList *lseps = NULL;
@@ -913,6 +915,11 @@ static void avdtp_sep_set_state(struct avdtp *session,
 		session->streams = g_slist_remove(session->streams, stream);
 		stream_free(stream);
 	}
+
+	if (session->io && session->shutdown && session->streams == NULL) {
+		int sock = g_io_channel_unix_get_fd(session->io);
+		shutdown(sock, SHUT_RDWR);
+	}
 }
 
 static void finalize_discovery(struct avdtp *session, int err)
@@ -2141,7 +2148,7 @@ gboolean avdtp_remove_disconnect_cb(struct avdtp *session, unsigned int id)
 void avdtp_shutdown(struct avdtp *session)
 {
 	GSList *l;
-	int sock;
+	bool aborting = false;
 
 	if (!session->io)
 		return;
@@ -2149,12 +2156,18 @@ void avdtp_shutdown(struct avdtp *session)
 	for (l = session->streams; l; l = g_slist_next(l)) {
 		struct avdtp_stream *stream = l->data;
 
-		avdtp_close(session, stream, TRUE);
+		if (stream->abort_int || avdtp_abort(session, stream) == 0)
+			aborting = true;
 	}
 
-	sock = g_io_channel_unix_get_fd(session->io);
+	if (aborting) {
+		/* defer shutdown until all streams are aborted properly */
+		session->shutdown = true;
+	} else {
+		int sock = g_io_channel_unix_get_fd(session->io);
 
-	shutdown(sock, SHUT_RDWR);
+		shutdown(sock, SHUT_RDWR);
+	}
 }
 
 static void queue_request(struct avdtp *session, struct pending_req *req,
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 24/24] Bluetooth: Fix write_room() calculation
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

The skb truesize of a 12-byte payload with a 10-byte head/tail
reserve is 768 bytes. Consequently, even with 40 tx_credits, at
most 6 packets could be queued at any one time:

  40 tx_credits * 127-byte mtu < 768-byte truesize * 7

This error could also cause the tx queue to apparently stall if
credit flow control is disabled (where tx_credits is fixed at 5),
or if the receiver only granted a limited number of tx credits
(eg., less than 7).

Instead, track the outstanding number of queued packets not yet sent
in wmem_alloc and allow for a maximum of 40 queued packets. Report
the space avail for a single write() as the mtu * number of packets
left before reaching the maximum.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 3f44195..403ec09 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -352,20 +352,16 @@ static inline unsigned int rfcomm_room(struct rfcomm_dev *dev)
 {
 	struct rfcomm_dlc *dlc = dev->dlc;
 
-	/* The limit is bogus; the number of packets which can
-	 * currently be sent by the krfcommd thread has no relevance
-	 * to the number of packets which can be queued on the dlc's
-	 * tx queue.
-	 */
-	int limit = dlc->mtu * (dlc->tx_credits?:1);
+	/* Limit the outstanding number of packets not yet sent to 40 */
+	int pending = 40 - atomic_read(&dev->wmem_alloc);
 
-	return max(0, limit - atomic_read(&dev->wmem_alloc));
+	return max(0, pending) * dlc->mtu;
 }
 
 static void rfcomm_wfree(struct sk_buff *skb)
 {
 	struct rfcomm_dev *dev = (void *) skb->sk;
-	atomic_sub(skb->truesize, &dev->wmem_alloc);
+	atomic_dec(&dev->wmem_alloc);
 	if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags))
 		tty_port_tty_wakeup(&dev->port);
 	tty_port_put(&dev->port);
@@ -374,7 +370,7 @@ static void rfcomm_wfree(struct sk_buff *skb)
 static void rfcomm_set_owner_w(struct sk_buff *skb, struct rfcomm_dev *dev)
 {
 	tty_port_get(&dev->port);
-	atomic_add(skb->truesize, &dev->wmem_alloc);
+	atomic_inc(&dev->wmem_alloc);
 	skb->sk = (void *) dev;
 	skb->destructor = rfcomm_wfree;
 }
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 23/24] Bluetooth: Refactor write_room() calculation
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Compute the amount of space available for a single write()
within rfcomm_room(); clamp to 0 for negative values. Note
this patch does not change the result of the computation.

Report the amount of room returned in the debug printk.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index af775f3..3f44195 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -348,11 +348,18 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 }
 
 /* ---- Send buffer ---- */
-static inline unsigned int rfcomm_room(struct rfcomm_dlc *dlc)
+static inline unsigned int rfcomm_room(struct rfcomm_dev *dev)
 {
-	/* We can't let it be zero, because we don't get a callback
-	   when tx_credits becomes nonzero, hence we'd never wake up */
-	return dlc->mtu * (dlc->tx_credits?:1);
+	struct rfcomm_dlc *dlc = dev->dlc;
+
+	/* The limit is bogus; the number of packets which can
+	 * currently be sent by the krfcommd thread has no relevance
+	 * to the number of packets which can be queued on the dlc's
+	 * tx queue.
+	 */
+	int limit = dlc->mtu * (dlc->tx_credits?:1);
+
+	return max(0, limit - atomic_read(&dev->wmem_alloc));
 }
 
 static void rfcomm_wfree(struct sk_buff *skb)
@@ -809,16 +816,12 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 static int rfcomm_tty_write_room(struct tty_struct *tty)
 {
 	struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
-	int room;
+	int room = 0;
 
-	BT_DBG("tty %p", tty);
-
-	if (!dev || !dev->dlc)
-		return 0;
+	if (dev && dev->dlc)
+		room = rfcomm_room(dev);
 
-	room = rfcomm_room(dev->dlc) - atomic_read(&dev->wmem_alloc);
-	if (room < 0)
-		room = 0;
+	BT_DBG("tty %p room %d", tty, room);
 
 	return room;
 }
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 22/24] Bluetooth: Don't fail RFCOMM tty writes
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

The tty driver api design prefers no-fail writes if the driver
write_room() method has previously indicated space is available
to accept writes. Since this is trivially possible for the
RFCOMM tty driver, do so.

Introduce rfcomm_dlc_send_noerror(), which queues but does not
schedule the krfcomm thread if the dlc is not yet connected
(and thus does not error based on the connection state).
The mtu size test is also unnecessary since the caller already
chunks the written data into mtu size.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h |  1 +
 net/bluetooth/rfcomm/core.c    | 14 ++++++++++++++
 net/bluetooth/rfcomm/tty.c     | 23 +++++++----------------
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 79bb913..11aca1e 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -238,6 +238,7 @@ int  rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
 								u8 channel);
 int  rfcomm_dlc_close(struct rfcomm_dlc *d, int reason);
 int  rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb);
+void rfcomm_dlc_send_noerror(struct rfcomm_dlc *d, struct sk_buff *skb);
 int  rfcomm_dlc_set_modem_status(struct rfcomm_dlc *d, u8 v24_sig);
 int  rfcomm_dlc_get_modem_status(struct rfcomm_dlc *d, u8 *v24_sig);
 void rfcomm_dlc_accept(struct rfcomm_dlc *d);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 8e14df6..112749c 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -568,6 +568,20 @@ int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
 	return len;
 }
 
+void rfcomm_dlc_send_noerror(struct rfcomm_dlc *d, struct sk_buff *skb)
+{
+	int len = skb->len;
+
+	BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
+
+	rfcomm_make_uih(skb, d->addr);
+	skb_queue_tail(&d->tx_queue, skb);
+
+	if (d->state == BT_CONNECTED &&
+	    !test_bit(RFCOMM_TX_THROTTLED, &d->flags))
+		rfcomm_schedule();
+}
+
 void __rfcomm_dlc_throttle(struct rfcomm_dlc *d)
 {
 	BT_DBG("dlc %p state %ld", d, d->state);
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index f6b9f0c..af775f3 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -374,14 +374,10 @@ static void rfcomm_set_owner_w(struct sk_buff *skb, struct rfcomm_dev *dev)
 
 static struct sk_buff *rfcomm_wmalloc(struct rfcomm_dev *dev, unsigned long size, gfp_t priority)
 {
-	if (atomic_read(&dev->wmem_alloc) < rfcomm_room(dev->dlc)) {
-		struct sk_buff *skb = alloc_skb(size, priority);
-		if (skb) {
-			rfcomm_set_owner_w(skb, dev);
-			return skb;
-		}
-	}
-	return NULL;
+	struct sk_buff *skb = alloc_skb(size, priority);
+	if (skb)
+		rfcomm_set_owner_w(skb, dev);
+	return skb;
 }
 
 /* ---- Device IOCTLs ---- */
@@ -786,7 +782,7 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 	struct rfcomm_dev *dev = (struct rfcomm_dev *) tty->driver_data;
 	struct rfcomm_dlc *dlc = dev->dlc;
 	struct sk_buff *skb;
-	int err = 0, sent = 0, size;
+	int sent = 0, size;
 
 	BT_DBG("tty %p count %d", tty, count);
 
@@ -794,7 +790,6 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 		size = min_t(uint, count, dlc->mtu);
 
 		skb = rfcomm_wmalloc(dev, size + RFCOMM_SKB_RESERVE, GFP_ATOMIC);
-
 		if (!skb)
 			break;
 
@@ -802,17 +797,13 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 
 		memcpy(skb_put(skb, size), buf + sent, size);
 
-		err = rfcomm_dlc_send(dlc, skb);
-		if (err < 0) {
-			kfree_skb(skb);
-			break;
-		}
+		rfcomm_dlc_send_noerror(dlc, skb);
 
 		sent  += size;
 		count -= size;
 	}
 
-	return sent ? sent : err;
+	return sent;
 }
 
 static int rfcomm_tty_write_room(struct tty_struct *tty)
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 21/24] Bluetooth: Force -EIO from tty read/write if .activate() fails
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

If rfcomm_dlc_open() fails, set tty into error state which returns
-EIO from reads and writes.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 375b60d..f6b9f0c 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -111,8 +111,12 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
 {
 	struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+	int err;
 
-	return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+	err = rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+	if (err)
+		set_bit(TTY_IO_ERROR, &tty->flags);
+	return err;
 }
 
 /* we block the open until the dlc->state becomes BT_CONNECTED */
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 20/24] Bluetooth: Cleanup RFCOMM device registration error handling
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

If RFCOMM tty device registration fails, cleanup by releasing
the tty_port reference to trigger rfcomm_dev destruction
(rather than open-coding it).

The dlc reference release is moved into rfcomm_dev_add(),
which ensures cleanup in both error paths -- ie., if
__rfcomm_dev_add() fails or if tty_port_register_device() fails.

Fixes releasing the module reference if device registration fails.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 0537a05..375b60d 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -93,7 +93,8 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	rfcomm_dlc_put(dlc);
 
-	tty_unregister_device(rfcomm_tty_driver, dev->id);
+	if (dev->tty_dev)
+		tty_unregister_device(rfcomm_tty_driver, dev->id);
 
 	spin_lock(&rfcomm_dev_lock);
 	list_del(&dev->list);
@@ -317,16 +318,15 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 	BT_DBG("id %d channel %d", req->dev_id, req->channel);
 
 	dev = __rfcomm_dev_add(req, dlc);
-	if (IS_ERR(dev))
+	if (IS_ERR(dev)) {
+		rfcomm_dlc_put(dlc);
 		return PTR_ERR(dev);
+	}
 
 	tty = tty_port_register_device(&dev->port, rfcomm_tty_driver,
 			dev->id, NULL);
 	if (IS_ERR(tty)) {
-		spin_lock(&rfcomm_dev_lock);
-		list_del(&dev->list);
-		spin_unlock(&rfcomm_dev_lock);
-		kfree(dev);
+		tty_port_put(&dev->port);
 		return PTR_ERR(tty);
 	}
 
@@ -420,10 +420,8 @@ static int __rfcomm_create_dev(struct sock *sk, void __user *arg)
 	}
 
 	id = rfcomm_dev_add(&req, dlc);
-	if (id < 0) {
-		rfcomm_dlc_put(dlc);
+	if (id < 0)
 		return id;
-	}
 
 	if (req.flags & (1 << RFCOMM_REUSE_DLC)) {
 		/* DLC is now used by device.
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 19/24] Bluetooth: Refactor rfcomm_dev_add()
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Move rfcomm_dev allocation and initialization into new function,
__rfcomm_dev_add(), to simplify resource release in error handling.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index ef27695..0537a05 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -208,17 +208,16 @@ static ssize_t show_channel(struct device *tty_dev, struct device_attribute *att
 static DEVICE_ATTR(address, S_IRUGO, show_address, NULL);
 static DEVICE_ATTR(channel, S_IRUGO, show_channel, NULL);
 
-static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
+static struct rfcomm_dev *__rfcomm_dev_add(struct rfcomm_dev_req *req,
+					   struct rfcomm_dlc *dlc)
 {
 	struct rfcomm_dev *dev, *entry;
 	struct list_head *head = &rfcomm_dev_list;
 	int err = 0;
 
-	BT_DBG("id %d channel %d", req->dev_id, req->channel);
-
 	dev = kzalloc(sizeof(struct rfcomm_dev), GFP_KERNEL);
 	if (!dev)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	spin_lock(&rfcomm_dev_lock);
 
@@ -301,22 +300,37 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
 	   holds reference to this module. */
 	__module_get(THIS_MODULE);
 
+	spin_unlock(&rfcomm_dev_lock);
+	return dev;
+
 out:
 	spin_unlock(&rfcomm_dev_lock);
+	kfree(dev);
+	return ERR_PTR(err);
+}
 
-	if (err < 0)
-		goto free;
+static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
+{
+	struct rfcomm_dev *dev;
+	struct device *tty;
+
+	BT_DBG("id %d channel %d", req->dev_id, req->channel);
 
-	dev->tty_dev = tty_port_register_device(&dev->port, rfcomm_tty_driver,
+	dev = __rfcomm_dev_add(req, dlc);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	tty = tty_port_register_device(&dev->port, rfcomm_tty_driver,
 			dev->id, NULL);
-	if (IS_ERR(dev->tty_dev)) {
-		err = PTR_ERR(dev->tty_dev);
+	if (IS_ERR(tty)) {
 		spin_lock(&rfcomm_dev_lock);
 		list_del(&dev->list);
 		spin_unlock(&rfcomm_dev_lock);
-		goto free;
+		kfree(dev);
+		return PTR_ERR(tty);
 	}
 
+	dev->tty_dev = tty;
 	rfcomm_reparent_device(dev);
 	dev_set_drvdata(dev->tty_dev, dev);
 
@@ -327,10 +341,6 @@ out:
 		BT_ERR("Failed to create channel attribute");
 
 	return dev->id;
-
-free:
-	kfree(dev);
-	return err;
 }
 
 /* ---- Send buffer ---- */
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 18/24] Bluetooth: Serialize RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

At least two different race conditions exist with multiple concurrent
RFCOMMCREATEDEV and RFCOMMRELEASEDEV ioctls:
* Multiple concurrent RFCOMMCREATEDEVs with RFCOMM_REUSE_DLC can
  mistakenly share the same DLC.
* RFCOMMRELEASEDEV can destruct the rfcomm_dev still being
  constructed by RFCOMMCREATEDEV.

Introduce rfcomm_ioctl_mutex to serialize these add/remove operations.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 4a38b54..ef27695 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -40,6 +40,7 @@
 #define RFCOMM_TTY_MAJOR 216		/* device node major id of the usb/bluetooth.c driver */
 #define RFCOMM_TTY_MINOR 0
 
+static DEFINE_MUTEX(rfcomm_ioctl_mutex);
 static struct tty_driver *rfcomm_tty_driver;
 
 struct rfcomm_dev {
@@ -373,7 +374,7 @@ static struct sk_buff *rfcomm_wmalloc(struct rfcomm_dev *dev, unsigned long size
 
 #define NOCAP_FLAGS ((1 << RFCOMM_REUSE_DLC) | (1 << RFCOMM_RELEASE_ONHUP))
 
-static int rfcomm_create_dev(struct sock *sk, void __user *arg)
+static int __rfcomm_create_dev(struct sock *sk, void __user *arg)
 {
 	struct rfcomm_dev_req req;
 	struct rfcomm_dlc *dlc;
@@ -423,7 +424,7 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 	return id;
 }
 
-static int rfcomm_release_dev(void __user *arg)
+static int __rfcomm_release_dev(void __user *arg)
 {
 	struct rfcomm_dev_req req;
 	struct rfcomm_dev *dev;
@@ -466,6 +467,28 @@ static int rfcomm_release_dev(void __user *arg)
 	return 0;
 }
 
+static int rfcomm_create_dev(struct sock *sk, void __user *arg)
+{
+	int ret;
+
+	mutex_lock(&rfcomm_ioctl_mutex);
+	ret = __rfcomm_create_dev(sk, arg);
+	mutex_unlock(&rfcomm_ioctl_mutex);
+
+	return ret;
+}
+
+static int rfcomm_release_dev(void __user *arg)
+{
+	int ret;
+
+	mutex_lock(&rfcomm_ioctl_mutex);
+	ret = __rfcomm_release_dev(arg);
+	mutex_unlock(&rfcomm_ioctl_mutex);
+
+	return ret;
+}
+
 static int rfcomm_get_dev_list(void __user *arg)
 {
 	struct rfcomm_dev *dev;
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 17/24] Bluetooth: Rename __rfcomm_dev_get() to __rfcomm_dev_lookup()
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Functions which search lists for matching id's are more
commonly named *_lookup, which is the convention in the
bluetooth core as well.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index fa1226f..4a38b54 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -140,7 +140,7 @@ static const struct tty_port_operations rfcomm_port_ops = {
 	.carrier_raised = rfcomm_dev_carrier_raised,
 };
 
-static struct rfcomm_dev *__rfcomm_dev_get(int id)
+static struct rfcomm_dev *__rfcomm_dev_lookup(int id)
 {
 	struct rfcomm_dev *dev;
 
@@ -157,7 +157,7 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
 
 	spin_lock(&rfcomm_dev_lock);
 
-	dev = __rfcomm_dev_get(id);
+	dev = __rfcomm_dev_lookup(id);
 
 	if (dev && !tty_port_get(&dev->port))
 		dev = NULL;
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 16/24] Bluetooth: Fix RFCOMM parent device for reused dlc
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

The RFCOMM tty device is parented to the acl link device when
the dlc state_change(BT_CONNECTED) notification is received.
However, if the dlc from the RFCOMM socket is being reused
(RFCOMM_REUSE_DLC is set), then the dlc may already be connected,
and no notification will occur.

Instead, always parent the RFCOMM tty device to the acl link
device at registration time. If the acl link device is not available
(eg, because the dlc is not connected) then the tty will remain
unparented until the BT_CONNECTED notification is received.

Fixes regression with ModemManager when the rfcomm device is
created with the flag RFCOMM_REUSE_DLC.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 2975bc4..fa1226f 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -316,6 +316,7 @@ out:
 		goto free;
 	}
 
+	rfcomm_reparent_device(dev);
 	dev_set_drvdata(dev->tty_dev, dev);
 
 	if (device_create_file(dev->tty_dev, &dev_attr_address) < 0)
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 15/24] Bluetooth: Fix unsafe RFCOMM device parenting
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Accessing the results of hci_conn_hash_lookup_ba() is unsafe without
holding the hci_dev_lock() during the lookup. For example:

CPU 0                             | CPU 1
hci_conn_hash_lookup_ba           | hci_conn_del
  rcu_read_lock                   |   hci_conn_hash_del
  list_for_each_entry_rcu         |     list_del_rcu
    if (.....)                    |       synchronize_rcu
      rcu_read_unlock             |
                                  |   hci_conn_del_sysfs
                                  |   hci_dev_put
                                  |   hci_conn_put
                                  |     put_device (last reference)
                                  |       bt_link_release
                                  |         kfree(conn)
      return p  << just freed     |

Even if a hci_conn reference were taken (via hci_conn_get), would
not guarantee the lifetime of the sysfs device, but only safe
access to the in-memory structure.

Ensure the hci_conn device stays valid while the rfcomm device
is reparented; rename rfcomm_get_device() to rfcomm_reparent_device()
and perform the reparenting within the function while holding the
hci_dev_lock.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a58d693..2975bc4 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -167,20 +167,29 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
 	return dev;
 }
 
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
+static void rfcomm_reparent_device(struct rfcomm_dev *dev)
 {
 	struct hci_dev *hdev;
 	struct hci_conn *conn;
 
 	hdev = hci_get_route(&dev->dst, &dev->src);
 	if (!hdev)
-		return NULL;
+		return;
 
+	/* The lookup results are unsafe to access without the
+	 * hci device lock (FIXME: why is this not documented?)
+	 */
+	hci_dev_lock(hdev);
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
 
-	hci_dev_put(hdev);
+	/* Just because the acl link is in the hash table is no
+	 * guarantee the sysfs device has been added ...
+	 */
+	if (conn && device_is_registered(&conn->dev))
+		device_move(dev->tty_dev, &conn->dev, DPM_ORDER_DEV_AFTER_PARENT);
 
-	return conn ? &conn->dev : NULL;
+	hci_dev_unlock(hdev);
+	hci_dev_put(hdev);
 }
 
 static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
@@ -589,8 +598,7 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
 
 	dev->err = err;
 	if (dlc->state == BT_CONNECTED) {
-		device_move(dev->tty_dev, rfcomm_get_device(dev),
-			    DPM_ORDER_DEV_AFTER_PARENT);
+		rfcomm_reparent_device(dev);
 
 		wake_up_interruptible(&dev->port.open_wait);
 	} else if (dlc->state == BT_CLOSED)
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 14/24] Bluetooth: Directly close dlc for not yet started RFCOMM session
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

If the RFCOMM session has not yet been started (ie., session is
still in BT_BOUND state) when a dlc is closed, directly close and
unlink the dlc rather than sending a DISC frame that is never
sent.

This allows the dlci to be immediately reused rather than waiting
for a 20 second timeout.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 88f0edc..8e14df6 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -467,13 +467,19 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 
 	switch (d->state) {
 	case BT_CONNECT:
-	case BT_CONFIG:
-		/* Fall through */
-
 	case BT_CONNECTED:
 		__rfcomm_dlc_disconn(d);
 		break;
 
+	case BT_CONFIG:
+		if (s->state != BT_BOUND) {
+			__rfcomm_dlc_disconn(d);
+			break;
+		}
+		/* if closing a dlc in a session that hasn't been started,
+		 * just close and unlink the dlc
+		 */
+
 	default:
 		rfcomm_dlc_clear_timer(d);
 
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 13/24] Bluetooth: Refactor dlc disconnect logic in rfcomm_dlc_close()
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Prepare for directly closing dlc if the RFCOMM session has not
yet been started; refactor the dlc disconnect logic into a separate
local function, __rfcomm_dlc_disconn(). Retains functional
equivalence.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 797b7e1..88f0edc 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -430,6 +430,20 @@ int rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, u8 chann
 	return r;
 }
 
+static void __rfcomm_dlc_disconn(struct rfcomm_dlc *d)
+{
+	struct rfcomm_session *s = d->session;
+
+	d->state = BT_DISCONN;
+	if (skb_queue_empty(&d->tx_queue)) {
+		rfcomm_send_disc(s, d->dlci);
+		rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT);
+	} else {
+		rfcomm_queue_disc(d);
+		rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT * 2);
+	}
+}
+
 static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 {
 	struct rfcomm_session *s = d->session;
@@ -457,14 +471,7 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 		/* Fall through */
 
 	case BT_CONNECTED:
-		d->state = BT_DISCONN;
-		if (skb_queue_empty(&d->tx_queue)) {
-			rfcomm_send_disc(s, d->dlci);
-			rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT);
-		} else {
-			rfcomm_queue_disc(d);
-			rfcomm_dlc_set_timer(d, RFCOMM_DISC_TIMEOUT * 2);
-		}
+		__rfcomm_dlc_disconn(d);
 		break;
 
 	default:
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 12/24] Bluetooth: Refactor deferred setup test in rfcomm_dlc_close()
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Prepare for directly closing dlc if the RFCOMM session has not
yet been started; refactor the deferred setup test for only those
dlc states to which the test applies. Retains functional
equivalence.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index b5a3785..797b7e1 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -442,11 +442,18 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 	switch (d->state) {
 	case BT_CONNECT:
 	case BT_CONFIG:
+	case BT_OPEN:
+	case BT_CONNECT2:
 		if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
 			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
 			rfcomm_schedule();
-			break;
+			return 0;
 		}
+	}
+
+	switch (d->state) {
+	case BT_CONNECT:
+	case BT_CONFIG:
 		/* Fall through */
 
 	case BT_CONNECTED:
@@ -460,15 +467,6 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 		}
 		break;
 
-	case BT_OPEN:
-	case BT_CONNECT2:
-		if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
-			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
-			rfcomm_schedule();
-			break;
-		}
-		/* Fall through */
-
 	default:
 		rfcomm_dlc_clear_timer(d);
 
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 11/24] Bluetooth: Simplify RFCOMM session state eval
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Merge conditional test for BT_LISTEN session state into following
switch statement (which is functionally equivalent).

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 646b6ff..b5a3785 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1967,12 +1967,11 @@ static void rfcomm_process_sessions(void)
 			continue;
 		}
 
-		if (s->state == BT_LISTEN) {
+		switch (s->state) {
+		case BT_LISTEN:
 			rfcomm_accept_connection(s);
 			continue;
-		}
 
-		switch (s->state) {
 		case BT_BOUND:
 			s = rfcomm_check_connection(s);
 			break;
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 10/24] Bluetooth: Verify dlci not in use before rfcomm_dev create
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

Only one session/channel combination may be in use at any one
time. However, the failure does not occur until the tty is
opened (in rfcomm_dlc_open()).

Because these settings are actually bound at rfcomm device
creation (via RFCOMMCREATEDEV ioctl), validate and fail before
creating the rfcomm tty device.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h |  1 +
 net/bluetooth/rfcomm/core.c    | 26 +++++++++++++++++++++++++-
 net/bluetooth/rfcomm/tty.c     |  8 ++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 6f3fbc5..79bb913 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -241,6 +241,7 @@ int  rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb);
 int  rfcomm_dlc_set_modem_status(struct rfcomm_dlc *d, u8 v24_sig);
 int  rfcomm_dlc_get_modem_status(struct rfcomm_dlc *d, u8 *v24_sig);
 void rfcomm_dlc_accept(struct rfcomm_dlc *d);
+struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel);
 
 #define rfcomm_dlc_lock(d)     spin_lock(&d->lock)
 #define rfcomm_dlc_unlock(d)   spin_unlock(&d->lock)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index facd8a7..646b6ff 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -359,6 +359,11 @@ static struct rfcomm_dlc *rfcomm_dlc_get(struct rfcomm_session *s, u8 dlci)
 	return NULL;
 }
 
+static int rfcomm_check_channel(u8 channel)
+{
+	return channel < 1 || channel > 30;
+}
+
 static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, u8 channel)
 {
 	struct rfcomm_session *s;
@@ -368,7 +373,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
 	BT_DBG("dlc %p state %ld %pMR -> %pMR channel %d",
 	       d, d->state, src, dst, channel);
 
-	if (channel < 1 || channel > 30)
+	if (rfcomm_check_channel(channel))
 		return -EINVAL;
 
 	if (d->state != BT_OPEN && d->state != BT_CLOSED)
@@ -513,6 +518,25 @@ no_session:
 	return r;
 }
 
+struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
+{
+	struct rfcomm_session *s;
+	struct rfcomm_dlc *dlc = NULL;
+	u8 dlci;
+
+	if (rfcomm_check_channel(channel))
+		return ERR_PTR(-EINVAL);
+
+	rfcomm_lock();
+	s = rfcomm_session_get(src, dst);
+	if (s) {
+		dlci = __dlci(!s->initiator, channel);
+		dlc = rfcomm_dlc_get(s, dlci);
+	}
+	rfcomm_unlock();
+	return dlc;
+}
+
 int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
 {
 	int len = skb->len;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 6ea08b0..a58d693 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -385,6 +385,14 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 		dlc = rfcomm_pi(sk)->dlc;
 		rfcomm_dlc_hold(dlc);
 	} else {
+		/* Validate the channel is unused */
+		dlc = rfcomm_dlc_exists(&req.src, &req.dst, req.channel);
+		if (IS_ERR(dlc))
+			return PTR_ERR(dlc);
+		else if (dlc) {
+			rfcomm_dlc_put(dlc);
+			return -EBUSY;
+		}
 		dlc = rfcomm_dlc_alloc(GFP_KERNEL);
 		if (!dlc)
 			return -ENOMEM;
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 09/24] Bluetooth: Fix RFCOMM tty teardown race
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

RFCOMM tty device teardown can race with new tty device registration
for the same device id:

CPU 0                           | CPU 1
rfcomm_dev_add                  | rfcomm_dev_destruct
                                |   spin_lock
                                |   list_del   <== dev_id no longer used
                                |   spin_unlock
  spin_lock                     |     .
  [search rfcomm_dev_list]      |     .
  [dev_id not in use]           |     .
  [initialize new rfcomm_dev]   |     .
  spin_unlock                   |     .
                                |     .
  tty_port_register_device      |   tty_unregister_device

Don't remove rfcomm_dev from the device list until after tty device
unregistration has completed.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/bluetooth/rfcomm/tty.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index bb570d9..6ea08b0 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -84,10 +84,6 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	BT_DBG("dev %p dlc %p", dev, dlc);
 
-	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 */
 	if (dlc->owner == dev)
@@ -98,6 +94,10 @@ static void rfcomm_dev_destruct(struct tty_port *port)
 
 	tty_unregister_device(rfcomm_tty_driver, dev->id);
 
+	spin_lock(&rfcomm_dev_lock);
+	list_del(&dev->list);
+	spin_unlock(&rfcomm_dev_lock);
+
 	kfree(dev);
 
 	/* It's safe to call module_put() here because socket still
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 08/24] Bluetooth: Fix unreleased rfcomm_dev reference
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

When RFCOMM_RELEASE_ONHUP is set, the rfcomm tty driver 'takes over'
the initial rfcomm_dev reference created by the RFCOMMCREATEDEV ioctl.
The assumption is that the rfcomm tty driver will release the
rfcomm_dev reference when the tty is freed (in rfcomm_tty_cleanup()).
However, if the tty is never opened, the 'take over' never occurs,
so when RFCOMMRELEASEDEV ioctl is called, the reference is not
released.

Track the state of the reference 'take over' so that the release
is guaranteed by either the RFCOMMRELEASEDEV ioctl or the rfcomm tty
driver.

Note that the synchronous hangup in rfcomm_release_dev() ensures
that rfcomm_tty_install() cannot race with the RFCOMMRELEASEDEV ioctl.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h | 1 +
 net/bluetooth/rfcomm/tty.c     | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 29d9727..6f3fbc5 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -332,6 +332,7 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel,
 
 /* rfcomm_dev.status bit definitions */
 #define RFCOMM_DEV_RELEASED   0
+#define RFCOMM_TTY_OWNED      1
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index d9d4bc8..bb570d9 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -441,7 +441,7 @@ static int rfcomm_release_dev(void __user *arg)
 		tty_kref_put(tty);
 	}
 
-	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+	if (!test_bit(RFCOMM_TTY_OWNED, &dev->status))
 		tty_port_put(&dev->port);
 
 	tty_port_put(&dev->port);
@@ -685,8 +685,10 @@ static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 	 * when the last process closes the tty. The behaviour is expected by
 	 * userspace.
 	 */
-	if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+	if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
+		set_bit(RFCOMM_TTY_OWNED, &dev->status);
 		tty_port_put(&dev->port);
+	}
 
 	return 0;
 }
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 07/24] Bluetooth: Release rfcomm_dev only once
From: Peter Hurley @ 2014-02-10  1:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo Padovan, Johan Hedberg, Gianluca Anzolin,
	Alexander Holler, Andrey Vihrov, Sander Eikelenboom,
	linux-bluetooth, linux-kernel, Peter Hurley
In-Reply-To: <1391997564-1805-1-git-send-email-peter@hurleysoftware.com>

No logic prevents an rfcomm_dev from being released multiple
times. For example, if the rfcomm_dev ref count is large due
to pending tx, then multiple RFCOMMRELEASEDEV ioctls may
mistakenly release the rfcomm_dev too many times. Note that
concurrent ioctls are not required to create this condition.

Introduce RFCOMM_DEV_RELEASED status bit which guarantees the
rfcomm_dev can only be released once.

NB: Since the flags are exported to userspace, introduce the status
field to track state for which userspace should not be aware.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/net/bluetooth/rfcomm.h |  6 +++++-
 net/bluetooth/rfcomm/tty.c     | 11 +++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 486213a..29d9727 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -323,11 +323,15 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel,
 #define RFCOMMGETDEVINFO	_IOR('R', 211, int)
 #define RFCOMMSTEALDLC		_IOW('R', 220, int)
 
+/* rfcomm_dev.flags bit definitions */
 #define RFCOMM_REUSE_DLC      0
 #define RFCOMM_RELEASE_ONHUP  1
 #define RFCOMM_HANGUP_NOW     2
 #define RFCOMM_TTY_ATTACHED   3
-#define RFCOMM_TTY_RELEASED   4
+#define RFCOMM_DEFUNCT_BIT4   4	  /* don't reuse this bit - userspace visible */
+
+/* rfcomm_dev.status bit definitions */
+#define RFCOMM_DEV_RELEASED   0
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b385d99..d9d4bc8 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -51,6 +51,8 @@ struct rfcomm_dev {
 	unsigned long		flags;
 	int			err;
 
+	unsigned long		status;		/* don't export to userspace */
+
 	bdaddr_t		src;
 	bdaddr_t		dst;
 	u8			channel;
@@ -423,6 +425,12 @@ static int rfcomm_release_dev(void __user *arg)
 		return -EPERM;
 	}
 
+	/* only release once */
+	if (test_and_set_bit(RFCOMM_DEV_RELEASED, &dev->status)) {
+		tty_port_put(&dev->port);
+		return -EALREADY;
+	}
+
 	if (req.flags & (1 << RFCOMM_HANGUP_NOW))
 		rfcomm_dlc_close(dev->dlc, 0);
 
@@ -433,8 +441,7 @@ static int rfcomm_release_dev(void __user *arg)
 		tty_kref_put(tty);
 	}
 
-	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags) &&
-	    !test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+	if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
 		tty_port_put(&dev->port);
 
 	tty_port_put(&dev->port);
-- 
1.8.1.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox