* Re: [PATCH] Fix closing stream of locked sep when reconfiguring
From: Johan Hedberg @ 2010-09-08 16:48 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1283958823-4811-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Sep 08, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>
> Stream should only be closed if sep is unlocked otherwise there is a
> possibility that sep reconfiguration is triggered although the sep is in
> use by the user application.
> ---
> audio/a2dp.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/audio/a2dp.c b/audio/a2dp.c
> index b062aed..ef0df17 100644
> --- a/audio/a2dp.c
> +++ b/audio/a2dp.c
> @@ -1393,6 +1393,8 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> }
>
> if (l != NULL) {
> + if (a2dp_sep_get_lock(tmp))
> + goto failed;
> setup->reconfigure = TRUE;
> if (avdtp_close(session, tmp->stream, FALSE) < 0) {
> error("avdtp_close failed");
Thanks for the patch! It's now in the upstream tree.
Johan
^ permalink raw reply
* [PATCH v2] Bluetooth: Add socket option definitions for AMP
From: Mat Martineau @ 2010-09-08 16:46 UTC (permalink / raw)
To: linux-bluetooth
Cc: marcel, rshaffer, haijun.liu, linux-arm-msm, Mat Martineau
This adds a new BT_AMP socket option to control the use of AMP channels.
It is for use with the SOL_BLUETOOTH option level on L2CAP sockets.
Available option values are defined as:
BT_AMP_REQUIRE_BR_EDR
* Default
* AMP controllers cannot be used
* Channel move requests from the remote device are denied
* If the L2CAP channel is currently using AMP, move the channel to
BR/EDR
BT_AMP_PREFER_AMP
* Allow use of AMP controllers
* If the L2CAP channel is currently on BR/EDR and AMP controller
resources are available, initiate a channel move to AMP
* Channel move requests from the remote device are allowed
* If the L2CAP socket has not been connected yet, try to create
and configure the channel directly on an AMP controller rather
than BR/EDR
BT_AMP_PREFER_BR_EDR
* Allow use of AMP controllers
* If the L2CAP channel is currently on AMP, move it to BR/EDR
* Channel move requests from the remote device are allowed
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/bluetooth.h | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..5e8b6c3 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -64,6 +64,31 @@ struct bt_security {
#define BT_DEFER_SETUP 7
+
+#define BT_AMP_POLICY 8
+/* Require BR/EDR (default policy)
+ * AMP controllers cannot be used
+ * Channel move requests from the remote device are denied
+ * If the L2CAP channel is currently using AMP, move the channel to BR/EDR
+ */
+#define BT_AMP_POLICY_REQUIRE_BR_EDR 0
+/* Prefer AMP
+ * Allow use of AMP controllers
+ * If the L2CAP channel is currently on BR/EDR and AMP controller
+ * resources are available, initiate a channel move to AMP
+ * Channel move requests from the remote device are allowed
+ * If the L2CAP socket has not been connected yet, try to create
+ * and configure the channel directly on an AMP controller rather
+ * than BR/EDR
+ */
+#define BT_AMP_POLICY_PREFER_AMP 1
+/* Prefer BR/EDR
+ * Allow use of AMP controllers
+ * If the L2CAP channel is currently on AMP, move it to BR/EDR
+ * Channel move requests from the remote device are allowed
+ */
+#define BT_AMP_POLICY_PREFER_BR_EDR 2
+
#define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
#define BT_ERR(fmt, arg...) printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
#define BT_DBG(fmt, arg...) pr_debug("%s: " fmt "\n" , __func__ , ## arg)
--
1.7.1
^ permalink raw reply related
* [PATCH] Fix closing stream of locked sep when reconfiguring
From: Luiz Augusto von Dentz @ 2010-09-08 15:13 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Stream should only be closed if sep is unlocked otherwise there is a
possibility that sep reconfiguration is triggered although the sep is in
use by the user application.
---
audio/a2dp.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index b062aed..ef0df17 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1393,6 +1393,8 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
}
if (l != NULL) {
+ if (a2dp_sep_get_lock(tmp))
+ goto failed;
setup->reconfigure = TRUE;
if (avdtp_close(session, tmp->stream, FALSE) < 0) {
error("avdtp_close failed");
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Bluetooth: remove extra newline from debug output
From: Emeltchenko Andrei @ 2010-09-08 13:26 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/rfcomm/core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 16b79f3..a7e4f2d 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1698,7 +1698,7 @@ static int rfcomm_recv_frame(struct rfcomm_session *s, struct sk_buff *skb)
break;
default:
- BT_ERR("Unknown packet type 0x%02x\n", type);
+ BT_ERR("Unknown packet type 0x%02x", type);
break;
}
kfree_skb(skb);
--
1.7.0.4
^ permalink raw reply related
* Compiling Issues
From: steven bluez @ 2010-09-08 11:59 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Marcel Holtmann, Gustavo F. Padovan"
[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]
Hi all ,
While I compile Bluez4.70 in my lucidlynx (ubuntu 10.04 &10.10). I
am facing issues please do suggest on how to compile the same & i have
attached the logs
CC src/manager.o
> CC src/adapter.o
> CC src/device.o
> CC src/dbus-common.o
> CC src/dbus-hci.o
> GEN audio/telephony.c
> CC audio/telephony.o
> GEN src/bluetooth.exp
> GEN src/bluetooth.ver
> CCLD src/bluetoothd
> src/attrib-server.o: In function `send_notification':
> attrib-server.c:(.text+0x21d): undefined reference to `enc_notification'
> attrib-server.c:(.text+0x272): undefined reference to `g_attrib_send'
> src/attrib-server.o: In function `connect_event':
> attrib-server.c:(.text+0x9bc): undefined reference to `g_attrib_new'
> attrib-server.c:(.text+0x9e7): undefined reference to `g_attrib_register'
> src/attrib-server.o: In function `channel_destroy':
> attrib-server.c:(.text+0xa31): undefined reference to
> `g_attrib_unregister_all'
> attrib-server.c:(.text+0xa3c): undefined reference to `g_attrib_unref'
> src/attrib-server.o: In function `.L59':
> attrib-server.c:(.text+0xaf4): undefined reference to `dec_read_by_grp_req'
> attrib-server.c:(.text+0xb2e): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0xb6d): undefined reference to `g_attrib_send'
> src/attrib-server.o: In function `.L58':
> attrib-server.c:(.text+0xb9e): undefined reference to `dec_read_req'
> attrib-server.c:(.text+0xc05): undefined reference to `enc_read_resp'
> src/attrib-server.o: In function `.L57':
> attrib-server.c:(.text+0xc3c): undefined reference to
> `dec_read_by_type_req'
> attrib-server.c:(.text+0xe09): undefined reference to
> `enc_read_by_type_resp'
> attrib-server.c:(.text+0xe1d): undefined reference to `att_data_list_free'
> src/attrib-server.o: In function `.L56':
> attrib-server.c:(.text+0xe5f): undefined reference to `dec_find_info_req'
> attrib-server.c:(.text+0x102a): undefined reference to `enc_find_info_resp'
> attrib-server.c:(.text+0x1038): undefined reference to `att_data_list_free'
> attrib-server.c:(.text+0x12da): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x1314): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x134c): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x1382): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x13b5): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x13dc): undefined reference to
> `enc_read_by_grp_resp'
> attrib-server.c:(.text+0x13f0): undefined reference to `att_data_list_free'
> attrib-server.c:(.text+0x145a): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x148f): undefined reference to `enc_error_resp'
> collect2: ld returned 1 exit status
> make[1]: *** [src/bluetoothd] Error 1
> make: *** [all] Error 2
>
Regards,
Steven
[-- Attachment #2: Type: text/html, Size: 3304 bytes --]
^ permalink raw reply
* Bluez 4.70 Compiling issues
From: steven bluez @ 2010-09-08 10:33 UTC (permalink / raw)
To: Gustavo F. Padovan", marcel, linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 2869 bytes --]
Hi all ,
While I compile Bluez4.70 in my lucidlynx (ubuntu 10.04 &10.10). I
am facing issues please do suggest on how to compile the same & i have
attached the logs
> CC src/manager.o
> CC src/adapter.o
> CC src/device.o
> CC src/dbus-common.o
> CC src/dbus-hci.o
> GEN audio/telephony.c
> CC audio/telephony.o
> GEN src/bluetooth.exp
> GEN src/bluetooth.ver
> CCLD src/bluetoothd
> src/attrib-server.o: In function `send_notification':
> attrib-server.c:(.text+0x21d): undefined reference to `enc_notification'
> attrib-server.c:(.text+0x272): undefined reference to `g_attrib_send'
> src/attrib-server.o: In function `connect_event':
> attrib-server.c:(.text+0x9bc): undefined reference to `g_attrib_new'
> attrib-server.c:(.text+0x9e7): undefined reference to `g_attrib_register'
> src/attrib-server.o: In function `channel_destroy':
> attrib-server.c:(.text+0xa31): undefined reference to
> `g_attrib_unregister_all'
> attrib-server.c:(.text+0xa3c): undefined reference to `g_attrib_unref'
> src/attrib-server.o: In function `.L59':
> attrib-server.c:(.text+0xaf4): undefined reference to `dec_read_by_grp_req'
> attrib-server.c:(.text+0xb2e): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0xb6d): undefined reference to `g_attrib_send'
> src/attrib-server.o: In function `.L58':
> attrib-server.c:(.text+0xb9e): undefined reference to `dec_read_req'
> attrib-server.c:(.text+0xc05): undefined reference to `enc_read_resp'
> src/attrib-server.o: In function `.L57':
> attrib-server.c:(.text+0xc3c): undefined reference to
> `dec_read_by_type_req'
> attrib-server.c:(.text+0xe09): undefined reference to
> `enc_read_by_type_resp'
> attrib-server.c:(.text+0xe1d): undefined reference to `att_data_list_free'
> src/attrib-server.o: In function `.L56':
> attrib-server.c:(.text+0xe5f): undefined reference to `dec_find_info_req'
> attrib-server.c:(.text+0x102a): undefined reference to `enc_find_info_resp'
> attrib-server.c:(.text+0x1038): undefined reference to `att_data_list_free'
> attrib-server.c:(.text+0x12da): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x1314): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x134c): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x1382): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x13b5): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x13dc): undefined reference to
> `enc_read_by_grp_resp'
> attrib-server.c:(.text+0x13f0): undefined reference to `att_data_list_free'
> attrib-server.c:(.text+0x145a): undefined reference to `enc_error_resp'
> attrib-server.c:(.text+0x148f): undefined reference to `enc_error_resp'
> collect2: ld returned 1 exit status
> make[1]: *** [src/bluetoothd] Error 1
> make: *** [all] Error 2
>
Regards,
Steven
[-- Attachment #2: Type: text/html, Size: 3249 bytes --]
^ permalink raw reply
* Re: [PATCH] Bluetooth: check L2CAP length in first ACL fragment
From: Andrei Emeltchenko @ 2010-09-08 8:17 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel
In-Reply-To: <alpine.DEB.2.00.1009071040450.3317@linux-sea-02>
Hi, Mat,
Thanks for good comments.
On Tue, Sep 7, 2010 at 10:12 PM, Mat Martineau <mathewm@codeaurora.org> wro=
te:
>>> On Fri, Sep 3, 2010 at 7:26 PM, Mat Martineau <mathewm@codeaurora.org>
>>> wrote:
>>>>> [ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020
>>>>> [ 1712.804809] [<c0031870>] (unwind_backtrace+0x0/0xdc) from
>>>>> [<c00a1f70>]
>>>>> (__alloc_pages_nodemask+0x4)
>>>>> [ 1712.814666] [<c00a1f70>] (__alloc_pages_nodemask+0x47c/0x4d4) from
>>>>> [<c00a1fd8>] (__get_free_pages+)
>>>>> [ 1712.824645] [<c00a1fd8>] (__get_free_pages+0x10/0x3c) from
>>>>> [<c026eb5c>]
>>>>> (__alloc_skb+0x4c/0xfc)
>>>>> [ 1712.833465] [<c026eb5c>] (__alloc_skb+0x4c/0xfc) from [<bf28c738>]
>>>>> (l2cap_recv_acldata+0xf0/0x1f8 )
>>>>> [ 1712.843322] [<bf28c738>] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap])
>>>>> from
>>>>> [<bf0094ac>] (hci_rx_task+0x)
>>>> What is logged right before this allocation failure? =A0There should b=
e a
>>>> "Start: total len %d, frag len %d" line. =A0Actually, all log messages
>>>> from
>>>> l2cap_recv_acldata leading up to the failure would be helpful.
>>>
>>> When I enable logs system behave differently because of huge traffic
>>> to syslog :-)
>
> Even more reason to suspect that the main bug is something different :)
The bug happens randomly after executing stress tests to BT
(Codenomicon test tools shall do the job). We cannot reproduce the bug
with the single test case.
I think the reason is that memory gets fragmented and kmalloc may fail
and this may be not a bug. There is even special option which suppress
page allocation warnings. For the reference check discussion below:
http://kerneltrap.org/mailarchive/linux-kernel/2008/4/1/1321084
>>>>> After applying patch dmesg looks like:
>>>>> ...
>>>>> l2cap_recv_acldata: Frame exceeding recv MTU (len 64182, MTU 1013)
>>>>> l2cap_recv_acldata: Unexpected continuation frame (len 34)
>>>>> ...
>>>>>
>>>>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>>>> ---
>>>>> net/bluetooth/l2cap.c | =A0 11 +++++++++++
>>>>> 1 files changed, 11 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>>>> index 9fad312..21824d7 100644
>>>>> --- a/net/bluetooth/l2cap.c
>>>>> +++ b/net/bluetooth/l2cap.c
>>>>> @@ -4654,6 +4654,8 @@ static int l2cap_recv_acldata(struct hci_conn
>>>>> *hcon,
>>>>> struct sk_buff *skb, u16 fl
>>>>>
>>>>> =A0 =A0 =A0 =A0if (flags & ACL_START) {
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct l2cap_hdr *hdr;
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct sock *sk;
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u16 cid;
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int len;
>>>>>
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (conn->rx_len) {
>>>>> @@ -4672,6 +4674,7 @@ static int l2cap_recv_acldata(struct hci_conn
>>>>> *hcon,
>>>>> struct sk_buff *skb, u16 fl
>>>>> d
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hdr =3D (struct l2cap_hdr *) skb->data=
;
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0len =3D __le16_to_cpu(hdr->len) + L2CA=
P_HDR_SIZE;
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cid =3D __le16_to_cpu(hdr->cid);
>>>>
>>>> Some stress tools also send L2CAP data one byte at a time - the start
>>>> fragment may not have all four header bytes. =A0l2cap_recv_acldata()
>>>> currently
>>>> allows short start fragments with two or three bytes, which would not
>>>> contain a valid CID.
>>>
>>> Yes currently we allow 2 bytes in start fragment and check hdr->len.
>>> What about following change (require 4 bytes in start fragment)
>>>
>>> ---------------------------------------------
>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (skb->len < 2) {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (skb->len < L2CAP_HDR_SIZE) {
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BT_ERR("Frame is too sho=
rt (len %d)", skb->len);
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0l2cap_conn_unreliable(co=
nn, ECOMM);
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto drop;
>>> ---------------------------------------------
>
> Yes, this check is critical to avoid crashing when receiving a short star=
t
> fragment. =A0I'm just not sure it's a good idea to reject all start fragm=
ents
> less than 4 bytes.
>
>> I have found in "BLUETOOTH SPECIFICATION Version 4.0 [Vol 3] page 36"
>>
>> "Note: Start Fragments always begin with the Basic L2CAP header of a
>> PDU." So the statement above is correct.
>
> I'm still not sure this language guarantees that start frames will contai=
n
> both the length and CID, it doesn't use the word "shall" or say it's a
> complete header. =A0This may be a theoretical point - basebands will not
> usually send such short start fragments,
I have never seen such a small fragments. This is theoretical _&&_
not-recommended case. Moreover I think "always" means all packet shall
have basic L2CAP header.
> but I've seen stress tools that do.
This is one more reason we shall drop such a small packets.
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (len =3D=3D skb->len) { =A0 =A0 =A0=
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/*
>>>>> Complete frame received */ @@ -4688,6 +4691,14 @@ static int
>>>>> l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto drop; =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0}
>>>>>
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sk =3D l2cap_get_chan_by_scid(&conn->ch=
an_list, cid);
>>>>
>>>> There are several issues here.
>>>>
>>>> l2cap_get_chan_by_scid() might return NULL, which will definitely happ=
en
>>>> with signaling or connectionless data frames.
>>>>
>>>> l2cap_get_chan_by_scid() also calls bh_lock_sock() if a channel is
>>>> found,
>>>> and I don't see that you added a matching call to bh_unlock_sock() if
>>>> l2cap_get_chan_by_scid() returns a non-NULL value.
>>>
>>> My mistake here. What about following check:
>>>
>>> ---------------------------------------
>>> @@ -3916,6 +3919,19 @@ static int l2cap_recv_acldata(struct hci_conn
>>> *hcon, struct sk_buff *skb,
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto drop;
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>>>
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sk =3D l2cap_get_chan_by_scid(&conn->chan=
_list, cid);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!sk)
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto drop;
>>> +
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (l2cap_pi(sk)->imtu < len) {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BT_ERR("Frame exceeding r=
ecv MTU (len %d, MTU
>>> %d)",
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 len, l2cap_pi(sk)->imtu);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->rx_len =3D 0; /* ne=
eded? */
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bh_unlock_sock(sk);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto drop;
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bh_unlock_sock(sk);
>>> +
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Allocate skb for the complete frame (=
with header) */
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0conn->rx_skb =3D bt_skb_alloc(len, GFP_A=
TOMIC);
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!conn->rx_skb)
>>>
>>> --------------------------------------
>
> That fixes the locking issue, but sk is expected to be NULL for valid
> signaling or connectionless data frames and you don't want to drop those.
This is valid point, I have not tested those packet fragmented.
I will modify the patch so that NULL sk goes through and I am thinking
about adding
"l2cap_conn_unreliable(conn, ECOMM)" just before "goto drop"
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (l2cap_pi(sk)->imtu < len) {
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BT_ERR("Frame exceeding=
recv MTU (len %d, MTU
>>>>> %d)",
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 len, l2cap_pi(sk)->imtu);
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->rx_len =3D 0; /* =
needed? */
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto drop;
>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>>>>> +
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Allocate skb for the complete frame=
(with header) */
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0conn->rx_skb =3D bt_skb_alloc(len, GFP=
_ATOMIC);
>>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!conn->rx_skb)
>>>>
>>>> In general, this approach adds an extra channel lookup and an extra
>>>> lock/unlock on every ACL start frame.
>>>
>>> Yes this adds extra lock/unlock but only if the frame is not complete.
>>> There shouldn't be many fragmented L2CAP packets.
>
> Fragmentation of incoming ACL data will be very dependent on the baseband
> firmware, the packet type used over the-air, and L2CAP MTU/MPS. =A0Lockin=
g is
> relatively expensive, so I still have reservations.
I think if we get L2CAP packets fragmented we are already screwed up
and one extra lock does not make a difference.
>>>> Even if the MTU is known to be exceeded on with the start frame, no mo=
re
>>>> than 64k is ever allocated (which shouldn't cause problems in itself).
>>>
>>> I think that for the mobile device this can be a problem since this
>>> shall be contiguous
>>> memory.
>
> But it's the same allocation and size constraint used for good L2CAP data=
.
> =A0There should be no problem if the allocated skb is reliably freed when=
the
> bad frame is dropped.
>
>>>> The root of the problem may be heap corruption due to a buffer overrun=
,
>>>> which seems possible due to the crash deep in the memory allocator.
Regards,
Andrei
^ permalink raw reply
* To support APM 6658
From: Zaahir Khan @ 2010-09-08 7:12 UTC (permalink / raw)
To: linux-bluetooth
Hi,
We are going to use APM 6658 combo WiFi and BT chip.
Our requirement is to Print through BT over UART.
We are using freescale iMX257 PDK board and
linux kenel 2.6.31 which got the support for Bluez-libs-2.25,
Bluez-utils-2.25 and kernel modules.
But your forum suggest Bluez-libs-2.25 and Bluez-utils-2.25 are obsolete.
And latest packages are as follows
bluez-4.70.tar.gz
bluez-firmware-1.2.tar.gz
bluez-hcidump-1.4.2.tar.gz
obexd-0.30.tar.gz
For our requirement, do we need go for above all four packages or else
please suggest??
As we understand bluez-4.70.tar.gz will be containing Bluez-libs-2.25
and Bluez-utils-2.25.
and obexd-0.30.tar.gz for object exchange.
Do we need bluez-firmware-1.2.tar.gz and bluez-hcidump-1.4.2.tar.gz also ??
Any help highly appretiaed
Thanks & regards,
Zaahir Khan
^ permalink raw reply
* Re: Why is l2cap_connect re-implemented in gattool instead of using btio wrappers?
From: Arun Kumar @ 2010-09-08 6:09 UTC (permalink / raw)
To: Vinicius Gomes; +Cc: linux-bluetooth
In-Reply-To: <AANLkTim9LjKjVgw-aSbjTrH0wZsvzHV=W3p4gadau4t3@mail.gmail.com>
Hello Vinicius
Thanks for the reply. However I am referring to latest Bluez 4.70 file
gatttool.c which implements its own version l2cap_
connect(void) rather than an old version of gatttool.c.
Do you mean to say that this l2cap_connect() would get deprecated and
btio routines be used instead in later bluez versions?
--
Best Regards,
Arun Kumar Singh
www.crazydaks.com
On Wed, Sep 8, 2010 at 5:43 AM, Vinicius Gomes
<vinicius.gomes@openbossa.org> wrote:
> Hi Arun,
>
> On Tue, Sep 7, 2010 at 3:06 PM, Arun Kumar <arunkat@gmail.com> wrote:
>> Hi,
>>
>> Just trying to understand the need for a separate l2cap_connect
>> re-implemention in gatttool.c instead of using bt_io_connect( ) wrapper
>> from
>> btio.c? Since current GATT implementation tries to work on existing BR/EDR,
>> cant btio wrapper suffice for this initial step...
>>
>
> An initial version of gatttool used to have support for ATT/GATT over
> Unix sockets, we did not use btio because it allowed us to reuse most
> of the connection code.
>
> Now that the Unix socket support was removed, there's no real reason
> not use the btio functions.
>
>> Best regards,
>> Arun
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
> Cheers,
> --
> Vinicius
>
^ permalink raw reply
* RE: MP3 over A2DP issue
From: 박찬열 @ 2010-09-08 2:38 UTC (permalink / raw)
To: 'pl bossart', linux-bluetooth
In-Reply-To: <AANLkTi=Q=E2uUBXuyBuOdrxiyWU+meiFLkaef3xvwQ1=@mail.gmail.com>
Hi
Could you send me "bluetoothd" log?
Thanks
Chanyeol,Park
-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
owner@vger.kernel.org] On Behalf Of pl bossart
Sent: Wednesday, September 08, 2010 10:35 AM
To: linux-bluetooth@vger.kernel.org
Subject: MP3 over A2DP issue
Hi,
I have been doing some experiments to extend PulseAudio to support
compressed data, so that the audio routing is simplified a great deal.
I've already completed the work for AC3 passthrough over SPDIF/HDMI,
works fine and the concept holds water.
Now I am trying to send MP3 over A2DP through PulseAudio. I removed
SBC encoding, detect mp3 frames and handle timings, now I am almost
done and of course I am stuck with a stupid issue.
I can't seem to open an MP3 connection to my Nokia BH-103 headset, for
some reason the BT_OPEN request fails. Here's the log I am getting.
D: module-bluetooth-device.c: Connected to the bluetooth audio service
D: module-bluetooth-device.c: Sending BT_REQUEST -> BT_GET_CAPABILITIES
D: module-bluetooth-device.c: Trying to receive message from audio
service...
D: module-bluetooth-device.c: Received BT_RESPONSE <- BT_GET_CAPABILITIES
D: module-bluetooth-device.c: Payload size is 26 6
E: module-bluetooth-device.c: BT Codec: seid 1 transport 0 type 1
length 13 configured 1 lock 0 data 15
E: module-bluetooth-device.c: BT Codec: seid 2 transport 0 type 3
length 13 configured 0 lock 0 data 15
E: module-bluetooth-device.c: MPEG caps detected
E: module-bluetooth-device.c: channel_mode 15 crc 1 layer 1 frequency
7 mpf 0 bitrate 65279
E: module-bluetooth-device.c: mpeg seid 2
E: module-bluetooth-device.c: SBC caps detected
E: module-bluetooth-device.c: channel_mode 15 frequency 15
allocation_method 3 subbands 3 block_length 15 min_bitpool 2
max_bitpool 51
E: module-bluetooth-device.c: sbc seid 1
D: module-bluetooth-device.c: Got device capabilities
D: module-bluetooth-device.c: Sending BT_REQUEST -> BT_OPEN
D: module-bluetooth-device.c: Trying to receive message from audio
service...
D: module-bluetooth-device.c: Received BT_ERROR <- BT_OPEN
E: module-bluetooth-device.c: Received error condition: Invalid argument
E: module-bluetooth-device.c: BT_OPEN expect failed
I: card.c: Changed profile of card 1 "bluez_card.00_0B_
And here's the log using the gstreamer ad2p sink, it shows the same
type of error (but this time this isn't my code...):
[ume@localhost bluez]$ gst-launch filesrc
location=~/AURAL/Audio/maxwork.mp3 ! mp3parse ! a2dpsink
device=00:0B:E4:94:31:9D
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
0:00:00.021572200 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:1328:gst_avdtp_sink_audioservice_recv:<avdtpsink>
BT_OPEN failed : Invalid argument(22)
0:00:00.021609480 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:1085:gst_avdtp_sink_configure:<avdtpsink> Error
while receiving device confirmation
WARNING: from element
/GstPipeline:pipeline0/GstA2dpSink:a2dpsink0/GstAvdtpSink:avdtpsink:
Internal data flow problem.
Additional debug info:
gstbasesink.c(3436): gst_base_sink_chain_unlocked ():
/GstPipeline:pipeline0/GstA2dpSink:a2dpsink0/GstAvdtpSink:avdtpsink:
Received buffer without a new-segment. Assuming timestamps start from 0.
0:00:00.021884764 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:1328:gst_avdtp_sink_audioservice_recv:<avdtpsink>
BT_START_STREAM failed : Success(0)
0:00:00.021904018 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:937:gst_avdtp_sink_stream_start:<avdtpsink> Error
while stream start confirmation
ERROR: from element /GstPipeline:pipeline0/GstFileSrc:filesrc0:
Internal data flow error.
SBC connections work fine with gst-launch filesrc
location=~/AURAL/Audio/viol-1mn.wav ! wavparse ! sbcenc ! a2dpsink
device=00:0B:E4:94:31:9D
So is there something wrong with this specific headset, am I doing
anything wrong in the configuration? And what can I do in terms of
instrumentation to find the issue?
Or is it an issue with a bad seid reported by the device? When I use
the SBC seid in the BT_OPEN request, the open works fine. As soon as I
use the mpeg seid, things go south.
Bear with me, I am far from my audio comfort zone here. Any pointers
would be appreciated.
Thanks,
-Pierre
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* MP3 over A2DP issue
From: pl bossart @ 2010-09-08 1:35 UTC (permalink / raw)
To: linux-bluetooth
Hi,
I have been doing some experiments to extend PulseAudio to support
compressed data, so that the audio routing is simplified a great deal.
I've already completed the work for AC3 passthrough over SPDIF/HDMI,
works fine and the concept holds water.
Now I am trying to send MP3 over A2DP through PulseAudio. I removed
SBC encoding, detect mp3 frames and handle timings, now I am almost
done and of course I am stuck with a stupid issue.
I can't seem to open an MP3 connection to my Nokia BH-103 headset, for
some reason the BT_OPEN request fails. Here's the log I am getting.
D: module-bluetooth-device.c: Connected to the bluetooth audio service
D: module-bluetooth-device.c: Sending BT_REQUEST -> BT_GET_CAPABILITIES
D: module-bluetooth-device.c: Trying to receive message from audio service...
D: module-bluetooth-device.c: Received BT_RESPONSE <- BT_GET_CAPABILITIES
D: module-bluetooth-device.c: Payload size is 26 6
E: module-bluetooth-device.c: BT Codec: seid 1 transport 0 type 1
length 13 configured 1 lock 0 data 15
E: module-bluetooth-device.c: BT Codec: seid 2 transport 0 type 3
length 13 configured 0 lock 0 data 15
E: module-bluetooth-device.c: MPEG caps detected
E: module-bluetooth-device.c: channel_mode 15 crc 1 layer 1 frequency
7 mpf 0 bitrate 65279
E: module-bluetooth-device.c: mpeg seid 2
E: module-bluetooth-device.c: SBC caps detected
E: module-bluetooth-device.c: channel_mode 15 frequency 15
allocation_method 3 subbands 3 block_length 15 min_bitpool 2
max_bitpool 51
E: module-bluetooth-device.c: sbc seid 1
D: module-bluetooth-device.c: Got device capabilities
D: module-bluetooth-device.c: Sending BT_REQUEST -> BT_OPEN
D: module-bluetooth-device.c: Trying to receive message from audio service...
D: module-bluetooth-device.c: Received BT_ERROR <- BT_OPEN
E: module-bluetooth-device.c: Received error condition: Invalid argument
E: module-bluetooth-device.c: BT_OPEN expect failed
I: card.c: Changed profile of card 1 "bluez_card.00_0B_
And here's the log using the gstreamer ad2p sink, it shows the same
type of error (but this time this isn't my code...):
[ume@localhost bluez]$ gst-launch filesrc
location=~/AURAL/Audio/maxwork.mp3 ! mp3parse ! a2dpsink
device=00:0B:E4:94:31:9D
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
0:00:00.021572200 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:1328:gst_avdtp_sink_audioservice_recv:<avdtpsink>
BT_OPEN failed : Invalid argument(22)
0:00:00.021609480 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:1085:gst_avdtp_sink_configure:<avdtpsink> Error
while receiving device confirmation
WARNING: from element
/GstPipeline:pipeline0/GstA2dpSink:a2dpsink0/GstAvdtpSink:avdtpsink:
Internal data flow problem.
Additional debug info:
gstbasesink.c(3436): gst_base_sink_chain_unlocked ():
/GstPipeline:pipeline0/GstA2dpSink:a2dpsink0/GstAvdtpSink:avdtpsink:
Received buffer without a new-segment. Assuming timestamps start from 0.
0:00:00.021884764 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:1328:gst_avdtp_sink_audioservice_recv:<avdtpsink>
BT_START_STREAM failed : Success(0)
0:00:00.021904018 4939 0x95df668 ERROR avdtpsink
audio/gstavdtpsink.c:937:gst_avdtp_sink_stream_start:<avdtpsink> Error
while stream start confirmation
ERROR: from element /GstPipeline:pipeline0/GstFileSrc:filesrc0:
Internal data flow error.
SBC connections work fine with gst-launch filesrc
location=~/AURAL/Audio/viol-1mn.wav ! wavparse ! sbcenc ! a2dpsink
device=00:0B:E4:94:31:9D
So is there something wrong with this specific headset, am I doing
anything wrong in the configuration? And what can I do in terms of
instrumentation to find the issue?
Or is it an issue with a bad seid reported by the device? When I use
the SBC seid in the BT_OPEN request, the open works fine. As soon as I
use the mpeg seid, things go south.
Bear with me, I am far from my audio comfort zone here. Any pointers
would be appreciated.
Thanks,
-Pierre
^ permalink raw reply
* Re: Why is l2cap_connect re-implemented in gattool instead of using btio wrappers?
From: Vinicius Gomes @ 2010-09-08 0:13 UTC (permalink / raw)
To: Arun Kumar; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikfGdrt2gL0gVnjA6gPYw1i+PXpbyWw0kFEf6gz@mail.gmail.com>
Hi Arun,
On Tue, Sep 7, 2010 at 3:06 PM, Arun Kumar <arunkat@gmail.com> wrote:
> Hi,
>
> Just trying to understand the need for a separate l2cap_connect
> re-implemention in gatttool.c instead of using bt_io_connect( ) wrapper
> from
> btio.c? Since current GATT implementation tries to work on existing BR/EDR,
> cant btio wrapper suffice for this initial step...
>
An initial version of gatttool used to have support for ATT/GATT over
Unix sockets, we did not use btio because it allowed us to reuse most
of the connection code.
Now that the Unix socket support was removed, there's no real reason
not use the btio functions.
> Best regards,
> Arun
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Cheers,
--
Vinicius
^ permalink raw reply
* Re: BD address changed but pairing failed
From: Sebastien Cayetanot @ 2010-09-07 22:25 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Luiz Augusto von Dentz, linux-bluetooth
In-Reply-To: <1283889495.6640.27.camel@localhost.localdomain>
Le 9/7/2010 9:58 PM, Marcel Holtmann a écrit :
> Hi Sebastien,
>
>>>> I'm currently facing an issue regarding the pairing of BT devices after
>>>> having change the BD_Addr.
>>>>
>>>>
>>>> I use a TI chip and also use hci command or bdaddr script to modify the
>>>> bd_addr
>>>>
>>>> I'm also using the bluez-4.69
>>>>
>>>> I change the BD_Addr using the bdaddr scripts provided into the bluez src.
>>>>
>>>> When other devices scan me, the new address is seen but hciconfig still show
>>>> the old BD address.
>>>>
>>>> I have sent the hcitool cmd to read the bd_addr and after the hciconfig
>>>> return the one.
>>>>
>>>> I'm discoverable with the new BD address.
>>>>
>>>> But When I try to pair an A2DP headset or BT Keyboard/Mouse it failed since
>>>> BD_Addr change.
>>>>
>>>> I have try to use directly the hcitool cmd using the vendor specific command
>>>> but I got the same result.
>>>>
>>>> I have tried to kill bluetooth daemon and restart it after having change the
>>>> bd_addr. Still failing.
>>>>
>>>> I have tried to kill bluetooth daemon before changing the BD adress and
>>>> start it after the change it still fail.
>>>>
>>>> I have also tried to power on/off interface. still the issue
>>>>
>>>> I think that something is "broken" is bluez when the BD_Addr is changed.
>>>>
>>>> Is someone already seen this issue ?
>>> I don't think you are suppose to change the address like that, it is
>>> supposed to be done once in the chip initialization and then keep it
>>> like that forever, so I guess there is something wrong if the chip is
>>> already initialize with the wrong address. What are you trying achieve
>>> with this?
>>>
>> Surely not but regarding some tests scripts I have seen on bluez, I
>> suppose that's possible. Perhaps, some chips support and other don't.
>>
>> I have tried to change the BD_Address during the BT initialization and
>> prior any HCI command sent to the chip and it works.
> all of them needed a special warm reset. The HCI_Reset was never enough.
> I might work on some Broadcom chips this way, but all others do need
> special reset magic.
>
>> I expected the same behavior than observed on WLAN : I have changed the
>> WLAN MAC address after and everything works fine.
> The WiFi MAC address is different than a BD_ADDR. You can NOT expect any
> similar behavior. That they happen to be both IEEE addresses means
> nothing at all.
>
> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
We succeed to change the BD address and have a working system if you
change the BD address prior to send any hci command during the BT driver
boot.
Regards
^ permalink raw reply
* Re: [PATCH 1/4] Do not automatically remove watches for service names
From: Johan Hedberg @ 2010-09-07 20:50 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1283768782-4283-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
I've pushed all four patches to BlueZ upstream and the three gdbus
specific ones also to obexd upstream.
Johan
^ permalink raw reply
* Re: BD address changed but pairing failed
From: Marcel Holtmann @ 2010-09-07 19:58 UTC (permalink / raw)
To: Sebastien Cayetanot; +Cc: Luiz Augusto von Dentz, linux-bluetooth
In-Reply-To: <4C84BFB2.3000802@linux.intel.com>
Hi Sebastien,
> >> I'm currently facing an issue regarding the pairing of BT devices after
> >> having change the BD_Addr.
> >>
> >>
> >> I use a TI chip and also use hci command or bdaddr script to modify the
> >> bd_addr
> >>
> >> I'm also using the bluez-4.69
> >>
> >> I change the BD_Addr using the bdaddr scripts provided into the bluez src.
> >>
> >> When other devices scan me, the new address is seen but hciconfig still show
> >> the old BD address.
> >>
> >> I have sent the hcitool cmd to read the bd_addr and after the hciconfig
> >> return the one.
> >>
> >> I'm discoverable with the new BD address.
> >>
> >> But When I try to pair an A2DP headset or BT Keyboard/Mouse it failed since
> >> BD_Addr change.
> >>
> >> I have try to use directly the hcitool cmd using the vendor specific command
> >> but I got the same result.
> >>
> >> I have tried to kill bluetooth daemon and restart it after having change the
> >> bd_addr. Still failing.
> >>
> >> I have tried to kill bluetooth daemon before changing the BD adress and
> >> start it after the change it still fail.
> >>
> >> I have also tried to power on/off interface. still the issue
> >>
> >> I think that something is "broken" is bluez when the BD_Addr is changed.
> >>
> >> Is someone already seen this issue ?
> > I don't think you are suppose to change the address like that, it is
> > supposed to be done once in the chip initialization and then keep it
> > like that forever, so I guess there is something wrong if the chip is
> > already initialize with the wrong address. What are you trying achieve
> > with this?
> >
> Surely not but regarding some tests scripts I have seen on bluez, I
> suppose that's possible. Perhaps, some chips support and other don't.
>
> I have tried to change the BD_Address during the BT initialization and
> prior any HCI command sent to the chip and it works.
all of them needed a special warm reset. The HCI_Reset was never enough.
I might work on some Broadcom chips this way, but all others do need
special reset magic.
> I expected the same behavior than observed on WLAN : I have changed the
> WLAN MAC address after and everything works fine.
The WiFi MAC address is different than a BD_ADDR. You can NOT expect any
similar behavior. That they happen to be both IEEE addresses means
nothing at all.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: check L2CAP length in first ACL fragment
From: Mat Martineau @ 2010-09-07 19:12 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth, marcel
In-Reply-To: <AANLkTinZ6AYd1g0URdYxStfA-4-yd3-5ESagzYLpX4xR@mail.gmail.com>
Andrei -
On Mon, 6 Sep 2010, Andrei Emeltchenko wrote:
> Hi,
>
> On Mon, Sep 6, 2010 at 2:32 PM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
>> Hi,
>>
>> On Fri, Sep 3, 2010 at 7:26 PM, Mat Martineau <mathewm@codeaurora.org> wrote:
>>>> Trace below is received when using stress tools sending big
>>>> fragmented L2CAP packets.
>>>> ...
>>>> [ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020
>>>> [ 1712.804809] [<c0031870>] (unwind_backtrace+0x0/0xdc) from [<c00a1f70>]
>>>> (__alloc_pages_nodemask+0x4)
>>>> [ 1712.814666] [<c00a1f70>] (__alloc_pages_nodemask+0x47c/0x4d4) from
>>>> [<c00a1fd8>] (__get_free_pages+)
>>>> [ 1712.824645] [<c00a1fd8>] (__get_free_pages+0x10/0x3c) from [<c026eb5c>]
>>>> (__alloc_skb+0x4c/0xfc)
>>>> [ 1712.833465] [<c026eb5c>] (__alloc_skb+0x4c/0xfc) from [<bf28c738>]
>>>> (l2cap_recv_acldata+0xf0/0x1f8 )
>>>> [ 1712.843322] [<bf28c738>] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap]) from
>>>> [<bf0094ac>] (hci_rx_task+0x)
>>>> ...
>>>
>>> What is logged right before this allocation failure? There should be a
>>> "Start: total len %d, frag len %d" line. Actually, all log messages from
>>> l2cap_recv_acldata leading up to the failure would be helpful.
>>
>> When I enable logs system behave differently because of huge traffic
>> to syslog :-)
Even more reason to suspect that the main bug is something different
:)
>>>> After applying patch dmesg looks like:
>>>> ...
>>>> l2cap_recv_acldata: Frame exceeding recv MTU (len 64182, MTU 1013)
>>>> l2cap_recv_acldata: Unexpected continuation frame (len 34)
>>>> ...
>>>>
>>>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>>> ---
>>>> net/bluetooth/l2cap.c | 11 +++++++++++
>>>> 1 files changed, 11 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>>> index 9fad312..21824d7 100644
>>>> --- a/net/bluetooth/l2cap.c
>>>> +++ b/net/bluetooth/l2cap.c
>>>> @@ -4654,6 +4654,8 @@ static int l2cap_recv_acldata(struct hci_conn *hcon,
>>>> struct sk_buff *skb, u16 fl
>>>>
>>>> if (flags & ACL_START) {
>>>> struct l2cap_hdr *hdr;
>>>> + struct sock *sk;
>>>> + u16 cid;
>>>> int len;
>>>>
>>>> if (conn->rx_len) {
>>>> @@ -4672,6 +4674,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon,
>>>> struct sk_buff *skb, u16 fl
>>>> d
>>>> hdr = (struct l2cap_hdr *) skb->data;
>>>> len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
>>>> + cid = __le16_to_cpu(hdr->cid);
>>>
>>> Some stress tools also send L2CAP data one byte at a time - the start
>>> fragment may not have all four header bytes. l2cap_recv_acldata() currently
>>> allows short start fragments with two or three bytes, which would not
>>> contain a valid CID.
>>
>> Yes currently we allow 2 bytes in start fragment and check hdr->len.
>> What about following change (require 4 bytes in start fragment)
>>
>> ---------------------------------------------
>> - if (skb->len < 2) {
>> + if (skb->len < L2CAP_HDR_SIZE) {
>> BT_ERR("Frame is too short (len %d)", skb->len);
>> l2cap_conn_unreliable(conn, ECOMM);
>> goto drop;
>> ---------------------------------------------
Yes, this check is critical to avoid crashing when receiving a short
start fragment. I'm just not sure it's a good idea to reject all
start fragments less than 4 bytes.
> I have found in "BLUETOOTH SPECIFICATION Version 4.0 [Vol 3] page 36"
>
> "Note: Start Fragments always begin with the Basic L2CAP header of a
> PDU." So the statement above is correct.
I'm still not sure this language guarantees that start frames will
contain both the length and CID, it doesn't use the word "shall" or
say it's a complete header. This may be a theoretical point -
basebands will not usually send such short start fragments, but I've
seen stress tools that do.
>>
>>>> if (len == skb->len) { /*
>>>> Complete frame received */ @@ -4688,6 +4691,14 @@ static int
>>>> l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
>>>> u16 fl goto drop; }
>>>>
>>>> + sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
>>>
>>> There are several issues here.
>>>
>>> l2cap_get_chan_by_scid() might return NULL, which will definitely happen
>>> with signaling or connectionless data frames.
>>>
>>> l2cap_get_chan_by_scid() also calls bh_lock_sock() if a channel is found,
>>> and I don't see that you added a matching call to bh_unlock_sock() if
>>> l2cap_get_chan_by_scid() returns a non-NULL value.
>>
>> My mistake here. What about following check:
>>
>> ---------------------------------------
>> @@ -3916,6 +3919,19 @@ static int l2cap_recv_acldata(struct hci_conn
>> *hcon, struct sk_buff *skb,
>> goto drop;
>> }
>>
>> + sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
>> + if (!sk)
>> + goto drop;
>> +
>> + if (l2cap_pi(sk)->imtu < len) {
>> + BT_ERR("Frame exceeding recv MTU (len %d, MTU %d)",
>> + len, l2cap_pi(sk)->imtu);
>> + conn->rx_len = 0; /* needed? */
>> + bh_unlock_sock(sk);
>> + goto drop;
>> + } else
>> + bh_unlock_sock(sk);
>> +
>> /* Allocate skb for the complete frame (with header) */
>> conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
>> if (!conn->rx_skb)
>>
>> --------------------------------------
That fixes the locking issue, but sk is expected to be NULL for valid
signaling or connectionless data frames and you don't want to drop
those.
>>
>>>
>>>> + if (l2cap_pi(sk)->imtu < len) {
>>>> + BT_ERR("Frame exceeding recv MTU (len %d, MTU
>>>> %d)",
>>>> + len, l2cap_pi(sk)->imtu);
>>>> + conn->rx_len = 0; /* needed? */
>>>> + goto drop;
>>>> + }
>>>> +
>>>> /* Allocate skb for the complete frame (with header) */
>>>> conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
>>>> if (!conn->rx_skb)
>>>
>>> In general, this approach adds an extra channel lookup and an extra
>>> lock/unlock on every ACL start frame.
>>
>> Yes this adds extra lock/unlock but only if the frame is not
>> complete. There shouldn't be many fragmented L2CAP packets.
Fragmentation of incoming ACL data will be very dependent on the
baseband firmware, the packet type used over the-air, and L2CAP
MTU/MPS. Locking is relatively expensive, so I still have
reservations.
>>> Even if the MTU is known to be exceeded on with the start frame,
>>> no more than 64k is ever allocated (which shouldn't cause problems
>>> in itself).
>>
>> I think that for the mobile device this can be a problem since this
>> shall be contiguous
>> memory.
But it's the same allocation and size constraint used for good L2CAP
data. There should be no problem if the allocated skb is reliably
freed when the bad frame is dropped.
>>> The root of the problem may be heap corruption due to a buffer
>>> overrun, which seems possible due to the crash deep in the memory
>>> allocator.
Hope this is helpful!
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Why is l2cap_connect re-implemented in gattool instead of using btio wrappers?
From: Arun Kumar @ 2010-09-07 18:06 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <AANLkTim7WPm4nyKVwU36ASMDTF_5N=eB8AnfutcODMv9@mail.gmail.com>
Hi,
Just trying to understand the need for a separate l2cap_connect
re-implemention in gatttool.c instead of using bt_io_connect( ) wrapper
from
btio.c? Since current GATT implementation tries to work on existing BR/EDR,
cant btio wrapper suffice for this initial step...
Best regards,
Arun
^ permalink raw reply
* Re: [RFC] Sim Access Profile API doc
From: Suraj Sumangala @ 2010-09-07 12:54 UTC (permalink / raw)
To: linux-bluetooth, Jothikumar.Mothilal, joakim.xj.ceder,
claudio.takahasi, Waldemar.Rymarkiewicz, Arkadiusz.Lichwa
In-Reply-To: <20100907101230.GA6309@jh-x301>
Hi Johan,
On 9/7/2010 3:42 PM, Johan Hedberg wrote:
> We're defining an API here, not implementation specific details, so I
> don't quite understand your concern. What's the point of defining an
> agent object if it will not receive any method calls? The RegisterAgent
> might as well be called EnableServer in that case and not contain any
> object path parameter at all.
>
> Having a proper agent interface and methods in it corresponding to SAP
> commands makes much more sense imho than emiting signals for SAP
> commands and receiving method calls for the SAP replies. It also doesn't
> in any way restrict the agent side implementation details. A D-Bus
> method call-method reply pair is a more intuitive mapping to a
> SAP-command-SAP-response pair compared to a D-Bus signal + method call
> which don't have any tight API level association (anybody can catch the
> signal and anybody can send the method call).
>
Got your point, Thanks.
Also, do let me know if you have any other suggestion regarding the
interface.
Regards
Suraj
^ permalink raw reply
* Re: Patch to improve cupsdir detection
From: Bastien Nocera @ 2010-09-07 12:48 UTC (permalink / raw)
To: Pacho Ramos; +Cc: BlueZ development
In-Reply-To: <1283854723.24491.3.camel@localhost.localdomain>
On Tue, 2010-09-07 at 12:18 +0200, Pacho Ramos wrote:
> Hello
>
> Since really a lot of time we are shipping in Gentoo attached patch to
> adapt cupsdir to our cups installations (that uses /usr/libexec/cups
> instead of /usr/lib) :
> diff --git a/Makefile.tools b/Makefile.tools
> index d9a2425..a382e05 100644
> --- a/Makefile.tools
> +++ b/Makefile.tools
> @@ -122,7 +122,7 @@ EXTRA_DIST += tools/dfubabel.1 tools/avctrl.8
>
>
> if CUPS
> -cupsdir = $(libdir)/cups/backend
> +cupsdir = `cups-config --serverbin`/backend
>
> cups_PROGRAMS = cups/bluetooth
>
> I send this to inform you about it and, if possible, get it
> upstreamed :-)
Given that the cups backend doesn't actually require the cups headers or
development packages, you'd need to check for cups-config existing, and
then use a nice default if not present.
This should be done in acinclude.m4, with a new macro, which will get
added to the generated configure.
Cheers
^ permalink raw reply
* RE: [RFC] Sim Access Profile API doc
From: Waldemar.Rymarkiewicz @ 2010-09-07 12:24 UTC (permalink / raw)
To: suraj
Cc: Suraj.Sumangala, linux-bluetooth, Jothikumar.Mothilal,
joakim.xj.ceder, claudio.takahasi, Arkadiusz.Lichwa
In-Reply-To: <4C8605A4.3020300@Atheros.com>
Hi Suraj,
>-----Original Message-----
>From: Suraj Sumangala [mailto:suraj@Atheros.com]
>Sent: Tuesday, September 07, 2010 11:28 AM
>Hi Waldek,
>
>On 9/7/2010 2:06 PM, Waldemar.Rymarkiewicz@tieto.com wrote:
>> Hi Suraj,
>>
>>> From: Suraj Sumangala [mailto:suraj@atheros.com]
>>> Sent: Tuesday, September 07, 2010 8:03 AM
>>
>>> +
>>> + void Disconnect(string type)
>>> +
>>> + This initiates a SAP disconnection from
>>> SAP server.
>>> +
>>> + The type parameter specifies the type
>>> of disconnection.
>>> +
>>> + "Graceful" -> lets the SAP client initiate a
>>> + garceful disconnection.
>>> +
>>> + "Immediate" -> disconnects the connection
>>> + immediately from the server.
>>> +
>>
>> In my view, the disconnection should be initiates by the sap
>server, not by the agent. In a typical use case the agent will
>communicate sim state change (card removed or not accessible)
>to sap server. Based on that, the sap server should make a
>decision whether disconnect or not. What's more the disconnect
>procedure is defined in the SAP spec, and would be nice to
>keep all those procedures in the sap server, not in the agent
>which will be specific for a sim provider. Therefore, I would
>remove this method call from the api. I would add the
>Disconnect signal instead.
>>
>> Signal:
>> Disconnect()
>> This signal will be send when SAP connectionis
>successfully removed.
>>
>> In case of graceful disconnection sap server sends signal
>to the agent REQUEST(disconnect) and wait till SIM free its
>stuff and send back RESPONSE(disconnect). In case of
>immediate disconnection sap server will send Disconnect()
>signal to the agent.
>
>Ok, In which all situations should the SAP server decide that
>the SAP connection has to be disconnected (you have mentioned
>Sim Status change as one candidate)?
>Also, based on what should the server decide to initiate a
>"Graceful" or "Immediate" disconnection?
Well, I thought this over and in case of sim state change sap should propagate this change to sap client, not disconnect. The only place when sap sever uses immediate disconnection imho are problems on the connection with agent (dbus errors), and the graceful disconnection should be used in case when the user want to stop sap server. In this point Enable/Disable methods are missing to let the User chance to stop/run sap server.
/Waldek
^ permalink raw reply
* [PATCH] Avoid possible memory leak in mcap_create_mcl function
From: Santiago Carot-Nemesio @ 2010-09-07 10:36 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
health/mcap.c | 39 ++++++++++++++++++++++++++++++++++-----
1 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/health/mcap.c b/health/mcap.c
index 6f1e565..a9a1190 100644
--- a/health/mcap.c
+++ b/health/mcap.c
@@ -783,6 +783,17 @@ static void mcap_uncache_mcl(struct mcap_mcl *mcl)
mcl->ctrl &= ~MCAP_CTRL_FREE;
}
+static void close_connecting_mcl(struct mcap_mcl *mcl)
+{
+ mcl->ctrl &= ~MCAP_CTRL_CONN;
+
+ if (mcl->ctrl & MCAP_CTRL_FREE) {
+ /* We have closed an MCL marked as releseable */
+ /* while it was in connecting process */
+ mcl->ms->mcl_uncached_cb(mcl, mcl->ms->user_data);
+ }
+}
+
void mcap_close_mcl(struct mcap_mcl *mcl, gboolean cache)
{
if (!mcl)
@@ -792,6 +803,9 @@ void mcap_close_mcl(struct mcap_mcl *mcl, gboolean cache)
g_io_channel_shutdown(mcl->cc, TRUE, NULL);
g_io_channel_unref(mcl->cc);
mcl->cc = NULL;
+
+ if (mcl->ctrl & MCAP_CTRL_CONN)
+ close_connecting_mcl(mcl);
}
mcl->state = MCL_IDLE;
@@ -1674,8 +1688,6 @@ static void mcap_connect_mcl_cb(GIOChannel *chan, GError *conn_err,
gpointer data = con->user_data;
GError *gerr = NULL;
- g_free(con);
-
mcl->ctrl &= ~MCAP_CTRL_CONN;
if (conn_err) {
@@ -1739,6 +1751,21 @@ static void connect_dc_event_cb(GIOChannel *chan, GError *err,
mcl->cb->mdl_connected(mdl, mcl->cb->user_data);
}
+static void mcl_io_destroy(gpointer data)
+{
+ struct connect_mcl *con = data;
+ struct mcap_mcl *mcl = con->mcl;
+
+ g_free(con);
+
+ if ((mcl->state != MCL_IDLE) || (mcl->ctrl & MCAP_CTRL_CACHED))
+ return;
+
+ /* This is a new MCL or another MCL removed */
+ /* from MCAP cache so we have to release it */
+ mcap_mcl_unref(mcl);
+}
+
gboolean mcap_create_mcl(struct mcap_instance *ms,
const bdaddr_t *addr,
uint16_t ccpsm,
@@ -1765,8 +1792,9 @@ gboolean mcap_create_mcl(struct mcap_instance *ms,
set_default_cb(mcl);
mcl->next_mdl = (rand() % MCAP_MDLID_FINAL) + 1;
mcl = mcap_mcl_ref(mcl);
- } else
- mcl->ctrl |= MCAP_CTRL_CONN;
+ }
+
+ mcl->ctrl |= MCAP_CTRL_CONN;
con = g_new0(struct connect_mcl, 1);
con->mcl = mcl;
@@ -1774,13 +1802,14 @@ gboolean mcap_create_mcl(struct mcap_instance *ms,
con->user_data = user_data;
mcl->cc = bt_io_connect(BT_IO_L2CAP, mcap_connect_mcl_cb, con,
- NULL, err,
+ mcl_io_destroy, err,
BT_IO_OPT_SOURCE_BDADDR, &ms->src,
BT_IO_OPT_DEST_BDADDR, addr,
BT_IO_OPT_PSM, ccpsm,
BT_IO_OPT_MTU, MCAP_CC_MTU,
BT_IO_OPT_SEC_LEVEL, ms->sec,
BT_IO_OPT_INVALID);
+
if (!mcl->cc) {
g_free(con);
mcl->ctrl &= ~MCAP_CTRL_CONN;
--
1.7.0.4
^ permalink raw reply related
* Patch to improve cupsdir detection
From: Pacho Ramos @ 2010-09-07 10:18 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 570 bytes --]
Hello
Since really a lot of time we are shipping in Gentoo attached patch to
adapt cupsdir to our cups installations (that uses /usr/libexec/cups
instead of /usr/lib) :
diff --git a/Makefile.tools b/Makefile.tools
index d9a2425..a382e05 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -122,7 +122,7 @@ EXTRA_DIST += tools/dfubabel.1 tools/avctrl.8
if CUPS
-cupsdir = $(libdir)/cups/backend
+cupsdir = `cups-config --serverbin`/backend
cups_PROGRAMS = cups/bluetooth
I send this to inform you about it and, if possible, get it
upstreamed :-)
Thanks a lot
[-- Attachment #2: cups-location.patch --]
[-- Type: text/x-patch, Size: 311 bytes --]
diff --git a/Makefile.tools b/Makefile.tools
index d9a2425..a382e05 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -122,7 +122,7 @@ EXTRA_DIST += tools/dfubabel.1 tools/avctrl.8
if CUPS
-cupsdir = $(libdir)/cups/backend
+cupsdir = `cups-config --serverbin`/backend
cups_PROGRAMS = cups/bluetooth
^ permalink raw reply related
* Re: [RFC] Sim Access Profile API doc
From: Johan Hedberg @ 2010-09-07 10:12 UTC (permalink / raw)
To: Suraj Sumangala
Cc: linux-bluetooth, Jothikumar.Mothilal, joakim.xj.ceder,
claudio.takahasi, Waldemar.Rymarkiewicz, Arkadiusz.Lichwa
In-Reply-To: <4C860CBE.8060808@Atheros.com>
Hi,
On Tue, Sep 07, 2010, Suraj Sumangala wrote:
> The idea was to let the agent implementation be written depending on
> the actual SIM card reader implementation used in the back end. The
> agent can then map its signal/methods to the corresponding SAP
> server calls.
We're defining an API here, not implementation specific details, so I
don't quite understand your concern. What's the point of defining an
agent object if it will not receive any method calls? The RegisterAgent
might as well be called EnableServer in that case and not contain any
object path parameter at all.
Having a proper agent interface and methods in it corresponding to SAP
commands makes much more sense imho than emiting signals for SAP
commands and receiving method calls for the SAP replies. It also doesn't
in any way restrict the agent side implementation details. A D-Bus
method call-method reply pair is a more intuitive mapping to a
SAP-command-SAP-response pair compared to a D-Bus signal + method call
which don't have any tight API level association (anybody can catch the
signal and anybody can send the method call).
Johan
^ permalink raw reply
* Re: [RFC] Sim Access Profile API doc
From: Suraj Sumangala @ 2010-09-07 9:58 UTC (permalink / raw)
To: linux-bluetooth, Jothikumar.Mothilal, joakim.xj.ceder,
claudio.takahasi, Waldemar.Rymarkiewicz, Arkadiusz.Lichwa
In-Reply-To: <20100907092927.GA28640@jh-x301>
Hi Johan,
On 9/7/2010 2:59 PM, Johan Hedberg wrote:
> Hi Suraj,
>
> On Tue, Sep 07, 2010, Suraj Sumangala wrote:
>> +Methods void RegisterAgent(object agent)
>> +
>> + This registers the Sim Access adapter agent.
>> + This also registers the profile with the service
>> + Databse and start waiting for an RFCOMM connection
>> + on the SAP server channel.
>> +
>> + The object path defines the path the of the agent.
>> +
>> + If an application disconnects from the bus all
>> + of its registered agents will be removed.
>> +
>> + Possible errors: org.bluez.Error.InvalidArguments
>> + org.bluez.Error.AlreadyExists
>> + org.bluez.Error.Failed
>> +
>> + void UnregisterAgent(object agent)
>> +
>> + This unregisters the agent that has been previously
>> + registered. The object path parameter must match the
>> + same value that has been used on registration.
>> +
>> + Possible errors: org.bluez.Error.DoesNotExist
>> + org.bluez.Error.InvalidArguments
>> + org.bluez.Error.Failed
>
> So you have these nice agent registration methods, however where's the
> definition of the agent interface? And if you're going to use an agent
> for this (which seems like a good idea) the SAP commands should map to
> D-Bus method calls to the agent and the D-Bus method replies from the
> agent should map to the SAP replies. I.e. please get rid of the D-Bus
> signals and methods you currently have for this.
The idea was to let the agent implementation be written depending on the
actual SIM card reader implementation used in the back end. The agent
can then map its signal/methods to the corresponding SAP server calls.
>
> Btw, I'm not 100% convinced that this is the most sensible architecture
> for implementing SAP, but assuming that it is the above comments apply
> :)
Yep, I understand. There were doubts raised on this methods. That is the
reason why I thought about getting comments from the community.
Your comments are most welcome.
>
> Johan
Regards
Suraj
^ permalink raw reply
* Re: [PATCH] Fix clean-local target
From: Johan Hedberg @ 2010-09-07 9:47 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz; +Cc: u.kleine-koenig, linux-bluetooth
In-Reply-To: <99B09243E1A5DA4898CDD8B700111448097958128E@EXMB04.eu.tieto.com>
Hi Waldek,
On Tue, Sep 07, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
> @Johan: Will you consider to push it to upstream ?
Yes, it's now upstream. I was waiting for a comment from Marcel since he
quite often has something to say about autotools file changes, but since
the patch seems quite trivial I went ahead and pushed it.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox