Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ 3/3] unit: Fix memory leaks in test-gdbus-client
From: Anderson Lizardo @ 2014-02-10 17:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1392052498-9229-1-git-send-email-anderson.lizardo@openbossa.org>

---
 unit/test-gdbus-client.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/unit/test-gdbus-client.c b/unit/test-gdbus-client.c
index 685729a..ee8a760 100644
--- a/unit/test-gdbus-client.c
+++ b/unit/test-gdbus-client.c
@@ -101,6 +101,7 @@ static void destroy_context(struct context *context)
 
 	dbus_connection_flush(context->dbus_conn);
 	dbus_connection_close(context->dbus_conn);
+	dbus_connection_unref(context->dbus_conn);
 
 	g_main_loop_unref(context->main_loop);
 
@@ -956,6 +957,10 @@ static void client_force_disconnect(void)
 
 	g_main_loop_run(context->main_loop);
 
+	g_dbus_unregister_interface(conn, SERVICE_PATH, SERVICE_NAME1);
+	g_dbus_detach_object_manager(conn);
+	dbus_connection_unref(conn);
+
 	destroy_context(context);
 }
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH BlueZ 2/3] gdbus: Fix incorrect DBusConnection reference counting
From: Anderson Lizardo @ 2014-02-10 17:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1392052498-9229-1-git-send-email-anderson.lizardo@openbossa.org>

Commit abfc2b0dd5c3e33abfdf1a815b16d492c1751c06 attempted to fix a crash
related to improper reference counting, but the main issue was that the
reference was taken only during the function call (which is usually
unnecessary for single thread), but still passed a pointer to
DBusConnection to a function that is called by the mainloop. This left a
window where the DBusConnection can be destroyed.
---
 gdbus/mainloop.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/gdbus/mainloop.c b/gdbus/mainloop.c
index 099b67f..ec52554 100644
--- a/gdbus/mainloop.c
+++ b/gdbus/mainloop.c
@@ -70,8 +70,6 @@ static gboolean message_dispatch(void *data)
 {
 	DBusConnection *conn = data;
 
-	dbus_connection_ref(conn);
-
 	/* Dispatch messages */
 	while (dbus_connection_dispatch(conn) == DBUS_DISPATCH_DATA_REMAINS);
 
@@ -84,7 +82,8 @@ static inline void queue_dispatch(DBusConnection *conn,
 						DBusDispatchStatus status)
 {
 	if (status == DBUS_DISPATCH_DATA_REMAINS)
-		g_timeout_add(DISPATCH_TIMEOUT, message_dispatch, conn);
+		g_timeout_add(DISPATCH_TIMEOUT, message_dispatch,
+						dbus_connection_ref(conn));
 }
 
 static gboolean watch_func(GIOChannel *chan, GIOCondition cond, gpointer data)
@@ -92,9 +91,6 @@ static gboolean watch_func(GIOChannel *chan, GIOCondition cond, gpointer data)
 	struct watch_info *info = data;
 	unsigned int flags = 0;
 	DBusDispatchStatus status;
-	DBusConnection *conn;
-
-	conn = dbus_connection_ref(info->conn);
 
 	if (cond & G_IO_IN)  flags |= DBUS_WATCH_READABLE;
 	if (cond & G_IO_OUT) flags |= DBUS_WATCH_WRITABLE;
@@ -103,10 +99,8 @@ static gboolean watch_func(GIOChannel *chan, GIOCondition cond, gpointer data)
 
 	dbus_watch_handle(info->watch, flags);
 
-	status = dbus_connection_get_dispatch_status(conn);
-	queue_dispatch(conn, status);
-
-	dbus_connection_unref(conn);
+	status = dbus_connection_get_dispatch_status(info->conn);
+	queue_dispatch(info->conn, status);
 
 	return TRUE;
 }
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH BlueZ 1/3] gdbus: Fix memory leak
From: Anderson Lizardo @ 2014-02-10 17:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

data->conn and data->path must be destroyed before freeing "data".
---
 gdbus/object.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index b248cbb..13cf9a9 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1253,6 +1253,8 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 
 	if (!dbus_connection_register_object_path(connection, path,
 						&generic_table, data)) {
+		dbus_connection_unref(data->conn);
+		g_free(data->path);
 		g_free(data->introspect);
 		g_free(data);
 		return NULL;
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] android/README: Update with implementation status summary
From: Szymon Janc @ 2014-02-10 14:53 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <CAF3PWx0fcUUQtdvm2c4m985vDNMdtOQKjKaFeg9L-LOZ+-dcxA@mail.gmail.com>

Hi Andrzej,

On Monday 10 of February 2014 15:35:15 Andrzej Kaczmarek wrote:
> Hi Szymon,
> 
> On 10 February 2014 12:50, Szymon Janc <szymon.janc@tieto.com> wrote:
> > This will give a better overview of implemented features.
> > ---
> > 
> >  android/README | 37 ++++++++++++++++++++++++++++++++-----
> >  1 file changed, 32 insertions(+), 5 deletions(-)
> > 
> > diff --git a/android/README b/android/README
> > index e3c314f..49536e2 100644
> > --- a/android/README
> > +++ b/android/README
> > @@ -132,15 +132,42 @@ automatically on HAL library initialization. To
> > deinitialize HAL library and> 
> >  stop daemon type 'bluetooth cleanup'. Type 'help' for more information.
> >  Tab
> >  completion is also supported.
> > 
> > +=====================
> > +Implementation status
> > +=====================
> > +
> > +Summary of HALs implementation status.
> > +
> > +complete    - implementation is feature complete and Android Framework is
> > able +              to use it normally
> > +partial     - implementation is in progress and not all required features
> > are +              present, Android Framework is able to use some of
> > features +initial     - only initial implementations is present, Android
> > Framework is +              able to initialize but most likely not able
> > to use it +not started - no implementation, Android Framework is not able
> > to initialize it +
> > +profile ID    HAL header         status
> > +core          bluetooth.h        complete
> > +a2dp          bt_av.h            complete
> > +gatt          bt_gatt.h          not started
> > +              bt_gatt_client.h   not started
> > +              bt_gatt_server.h   not started
> > +handsfree     bt_hf.h            initial
> > +hidhost       bt_hh.h            complete
> > +health        bt_hl.h            not started
> > +pan           bt_pan.h           complete
> > +avrcp         bt_rc.h            partial
> > +socket        bt_sock.h          complete
> 
> This should be 'partial' since you application cannot register server
> on custom UUID with channel being assigned dynamically
> (BluetoothAdapter::listenUsingRfcommWithServiceRecord). I have patches
> which add this support but need to fix other code first in order to
> make them work properly.

Right, I'll send V2 later (unless you manage to socket send fixes before me:))

-- 
BR
Szymon Janc

^ permalink raw reply

* (no subject)
From: Viswanatham, RaviTeja @ 2014-02-10 14:35 UTC (permalink / raw)
  To: bluez mailin list (linux-bluetooth@vger.kernel.org)

Hello,

I am working on Ubuntu 12.04 with a Bluetooth 3.0 +HS + wifi combo USB dongle. 

I want to reach a data transfer speed of up to 24 Mbit/s. 

My Questions:
Does Bluez support high speed data transfer rates up to 24 Mbit/s (Bluetooth 3.0+HS) ? 

If it does, is there any user configuration involved to achieve that? What other requirements need to be met?
Does, Bluetooth enables AMP function to communicate 802.11n channel to support high speed or it has to be configure with any other drivers? 

I am new to Linux a detail explanation would be really appreciated. Thank you in advance for your support.

Best Regards,

Ravi Teja  

^ permalink raw reply

* Re: [PATCH] android/README: Update with implementation status summary
From: Andrzej Kaczmarek @ 2014-02-10 14:35 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1392033059-3346-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On 10 February 2014 12:50, Szymon Janc <szymon.janc@tieto.com> wrote:
>
> This will give a better overview of implemented features.
> ---
>  android/README | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/android/README b/android/README
> index e3c314f..49536e2 100644
> --- a/android/README
> +++ b/android/README
> @@ -132,15 +132,42 @@ automatically on HAL library initialization. To deinitialize HAL library and
>  stop daemon type 'bluetooth cleanup'. Type 'help' for more information. Tab
>  completion is also supported.
>
> +=====================
> +Implementation status
> +=====================
> +
> +Summary of HALs implementation status.
> +
> +complete    - implementation is feature complete and Android Framework is able
> +              to use it normally
> +partial     - implementation is in progress and not all required features are
> +              present, Android Framework is able to use some of features
> +initial     - only initial implementations is present, Android Framework is
> +              able to initialize but most likely not able to use it
> +not started - no implementation, Android Framework is not able to initialize it
> +
> +profile ID    HAL header         status
> +core          bluetooth.h        complete
> +a2dp          bt_av.h            complete
> +gatt          bt_gatt.h          not started
> +              bt_gatt_client.h   not started
> +              bt_gatt_server.h   not started
> +handsfree     bt_hf.h            initial
> +hidhost       bt_hh.h            complete
> +health        bt_hl.h            not started
> +pan           bt_pan.h           complete
> +avrcp         bt_rc.h            partial
> +socket        bt_sock.h          complete

This should be 'partial' since you application cannot register server
on custom UUID with channel being assigned dynamically
(BluetoothAdapter::listenUsingRfcommWithServiceRecord). I have patches
which add this support but need to fix other code first in order to
make them work properly.

BR,
Andrzej

^ permalink raw reply

* Re: Some patches applied on Fedora that maybe should be considered for being applied upstream
From: Bastien Nocera @ 2014-02-10 13:40 UTC (permalink / raw)
  To: Pacho Ramos; +Cc: BlueZ development
In-Reply-To: <1391935987.4666.5.camel@belkin5>

On Sun, 2014-02-09 at 09:53 +0100, Pacho Ramos wrote:
> Hello
> 
> I was looking at bluez package and found some patches that maybe could
> be upstreamed. Also, I would like to know the reasons for not accepting
> them to ensure they are safe to be applied downstream by us too :)
> 
> http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0001-Allow-using-obexd-without-systemd-in-the-user-sessio.patch -> Does this cause any issues with systemd --user setups?

Giovanni already posted this patch earlier. There's no distribution
using systemd sessions, so this doesn't work yet.

> http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0001-obex-Use-GLib-helper-function-to-manipulate-paths.patch
> http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0002-autopair-Don-t-handle-the-iCade.patch
> http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0004-agent-Assert-possible-infinite-loop.patch
> -> Any reason for not applying it upstream too?

I've posted those patches to the list as well.

> http://pkgs.fedoraproject.org/cgit/bluez.git/tree/0001-work-around-Logitech-diNovo-Edge-keyboard-firmware-i.patch
> -> Taking care this looks to be a really old issue, maybe using the
> workaround would be the only option for now :/

I have no hardware to test this on, I snatched it from Ubuntu's bluez 4
package.

Cheers


^ permalink raw reply

* Re: LE Connection Update Disallowed
From: Sandy Chapman @ 2014-02-10 12:30 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAPm3yA0m0AUyQd04J4H+mQ3NL7XjvkLX3M8R7cKhc8QGGG25hA@mail.gmail.com>

Hi All,

On Thu, Feb 6, 2014 at 12:21 PM, Sandy Chapman <schapman@lixar.com> wrote:
> Hi Anderson,
>
> On Wed, Feb 5, 2014 at 11:23 AM, Anderson Lizardo
> <anderson.lizardo@openbossa.org> wrote:
>> HI Sandy,
>>
>> On Wed, Feb 5, 2014 at 10:51 AM, Sandy Chapman <schapman@lixar.com> wrote:
>>>>> How do I initiate a Connection Update from the peripheral?
>>>>
>>>> I never tried this procedure myself, but my guess it that you are
>>>> using the incorrect mechanism on the slave role. Take a look at the
>>>> "Connection Parameters Update Request" on Vol 3, Part A, Section 4.20.
>>>> I believe this is the correct way to request from the slave (from what
>>>> I understand while reading the Linux kernel implementation of the
>>>> master side).
>>>>
>>>> Note that when Linux is the master, this command is issued
>>>> automatically by the kernel when requested by the slave.
>>>
>>> I've taken a look at that section and it appears that this is what is
>>> used to trigger the Connection Update. It states that the command
>>> shall only be issued from the slave to the master. I can confirm that
>>> my device is in the slave role using 'hcitool con'.
>>
>> I think you didn't notice that the section I mentioned is about a
>> L2CAP signalling packet, not an HCI command (which in this case is to
>> be used on the master side). I suggest you read a bit more on the
>> L2CAP section to understand how these signalling packets work. Then
>> you can try building such packet with "hcitool cmd" (unless there is
>> some support for it on the current kernel, which I didn't check)
>>
>
> Yes, you right, I missed that part. I've built out what my packet
> should look like, but I'm having troubles sending it to the
> controller. I really am stuck on issuing this packet. It appears that
> I need to send an HCI ACL Data Packet which holds a Signalling
> Command. This signalling command then holds the connection parameter
> update request as it's payload. It looks like 'hcitool cmd' can't send
> ACL packets though as the command requires an OGF and OCF which are
> part of the HCI Command Packet, not the HCI ACL Data Packet. From what
> I can tell, the best chance I'd have is to send it via the l2test tool
> over L2CAP, but attempting to use le_public address type doesn't work
> (which I believe will send the message over CID 0x0005 - the fixed LE
> channel). It fails on getsockopt/setsockopt for the SOL_BLUETOOTH
> type. From what I can tell, it requires a kernel with CoC (not sure
> what it means or if I have it). I'm really hoping I'm not going to
> have to compile the bluetooth kernel module myself to send this
> connection update packet.
>
>>>> You may want to take a look at the "GAP Peripheral Preferred
>>>> Connection Parameters" characteristic (see Vol 3, Part C, Section
>>>> 12.3). If iPhones reads this characteristic and honours the
>>>> parameters, it may help.
>>>
>>> Unfortunately, it appears Apple explicitly ignores this parameter
>>> (section 3.6 in this document
>>> https://developer.apple.com/hardwaredrivers/BluetoothDesignGuidelines.pdf).
>>
>> This is unfortunate. It would be the easiest way to pass custom
>> connection parameters IMHO.
>>
>>> I believe that 'hcitool lecup' is exactly supposed to initiate this
>>> process. I've also tried to use 'hcitool cmd' to issue direct commands
>>> to the controller (using Vol 3, Part A, Section 4.20 as a guide), but
>>> I am having no luck. It's stating that the command is disallowed (not
>>> that the parameters are invalid), so I'm guessing there's something
>>> else wrong. Since this is directly communicating with the controller,
>>> where would the problem be? In the kernel, itself? Could it be a
>>> problem with the Broadcom chipset in my MacBook?
>>
>> "hcitool lecup" is just a helper, it uses the same mechanism that
>> "hcitool cmd" uses (and both are just "raw" interfaces to the
>> Bluetooth controller). If you are getting an "Invalid Parameters" on
>> any of them, is either because you built the packet incorrectly on
>> "hcitool cmd" or given invalid values as per the spec.
>>
>> By the way, I suggest using "btmon" instead of "hcidump", as the
>> former has nicer output and is more up-to-date (although I'm not sure
>> it supports parsing "LE Connection Update" parameters).
>>
>
> You're right btmon is much nicer and does support recognition of the
> LE commands.
>
>> Best Regards,
>> --
>> Anderson Lizardo
>> http://www.indt.org/?lang=en
>> INdT - Manaus - Brazil
>
> I know what I'm doing might not be typical, but I really appreciate
> your help. If there's any direction you could point me in, I'd be
> really thankful. I don't really know what to try next.
>
> Thanks again,
>
> Sandy

Just wanted to post that I've got this working. However, I've had to
write code that will format the signalling packet properly and relay
it to the controller via hci (in a similar manner to how hcitool
current sends HCI commands). The approach I suggested in my previous
email worked (HCI ACL Data Packet containing a Signalling Command of
the type Connection Parameter Update Request). This required changes
to hci.c (to format and write the new packet to the hci device).

See Vol. 2, Part E, 4.1 for Host to Controller HCI flow.
See Vol. 2, Part E, 5.4.2 for HCI ACL Data Packet format.
See Vol. 3, Part A, 4 for Signalling Packet format.
See Vol. 3, Part A, 4.20 for Connection Parameter Update Request format.

Since I've got this working, I'm wondering if there is interest from
the bluez devs in supporting this going forward? If so, I can likely
clean up my code in such a way that it'll allow adding the other
signalling packet formats easily. Currently, I've got my specific
signalling packet exposed through hcitool (as a new command) as it was
convenient to piggyback on it. Since the ACL Data Packets are being
sent via HCI, this may be a decent place for this new functionality to
stay.

Anyway, I'm just wondering if there is any interest as I can likely do
some work to add this functionality and I don't think I'm going to be
the only one who wishes to be able to request the slave device to
renegotiate the connection parameters while a connection is open from
the command line. This would be useful in a case where a low energy
connection would require a burst of information to be sent (transfer
of a large log file, or an image from a sensor for examples).

Sandy

-- 


^ permalink raw reply

* Re: [PATCH 1/4] android/haltest: Remove unneeded assignment
From: Luiz Augusto von Dentz @ 2014-02-10 12:16 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1391775078-25010-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Feb 7, 2014 at 2:11 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
>  android/client/if-audio.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/android/client/if-audio.c b/android/client/if-audio.c
> index 8c640a1..5ab11a6 100644
> --- a/android/client/if-audio.c
> +++ b/android/client/if-audio.c
> @@ -225,10 +225,8 @@ static void *playback_thread(void *data)
>                 pthread_mutex_unlock(&outstream_mutex);
>         } while (len && w_len > 0);
>
> -       if (in) {
> +       if (in)
>                 fclose(in);
> -               in = NULL;
> -       }
>
>         pthread_cleanup_pop(1);
>         return NULL;
> --
> 1.8.3.2

Pushed, note that I did move the changes from audio to android since
the audio code will be dropped as it is not unit tested.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH] android/README: Update with implementation status summary
From: Szymon Janc @ 2014-02-10 11:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This will give a better overview of implemented features.
---
 android/README | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/android/README b/android/README
index e3c314f..49536e2 100644
--- a/android/README
+++ b/android/README
@@ -132,15 +132,42 @@ automatically on HAL library initialization. To deinitialize HAL library and
 stop daemon type 'bluetooth cleanup'. Type 'help' for more information. Tab
 completion is also supported.
 
+=====================
+Implementation status
+=====================
+
+Summary of HALs implementation status.
+
+complete    - implementation is feature complete and Android Framework is able
+              to use it normally
+partial     - implementation is in progress and not all required features are
+              present, Android Framework is able to use some of features
+initial     - only initial implementations is present, Android Framework is
+              able to initialize but most likely not able to use it
+not started - no implementation, Android Framework is not able to initialize it
+
+profile ID    HAL header         status
+core          bluetooth.h        complete
+a2dp          bt_av.h            complete
+gatt          bt_gatt.h          not started
+              bt_gatt_client.h   not started
+              bt_gatt_server.h   not started
+handsfree     bt_hf.h            initial
+hidhost       bt_hh.h            complete
+health        bt_hl.h            not started
+pan           bt_pan.h           complete
+avrcp         bt_rc.h            partial
+socket        bt_sock.h          complete
+
 ===========================
 Implementation shortcomings
 ===========================
 
-It is possible that some of HAL functionality is missing implementation due to
-reasons like feature feasibility or necessity for latest Android Framework.
-This sections provides list of such deficiencies. Note that HAL library is
-always expected to fully implement HAL API so missing implementation might
-happen only in daemon.
+It is possible that some of HAL functionality (although being marked as
+complete) is missing implementation due to reasons like feature feasibility or
+necessity for latest Android Framework. This sections provides list of such
+deficiencies. Note that HAL library is always expected to fully implement HAL
+API so missing implementation might happen only in daemon.
 
 HAL Bluetooth
 =============
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH v2 1/6] android/a2dp: Shutdown AVDTP gracefully
From: Luiz Augusto von Dentz @ 2014-02-10 11:36 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1392029145-7954-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Mon, Feb 10, 2014 at 12:45 PM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> 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

Applied, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [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


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