Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] Added firmware load patch to crap directory.
From: Marcel Holtmann @ 2010-10-05  7:46 UTC (permalink / raw)
  To: Bala Shanmugam; +Cc: linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <1286118002-2354-1-git-send-email-sbalashanmugam@atheros.com>

Hi Bala,

> This patch in crap directory enables btusb to load firmware 
> to device RAM when it is plugged in.
>  Signed-off-by: Bala Shanmugam <sbalashanmugam@atheros.com>
> ---
>  crap/0003-btusb-Add-fw-load-support.patch |  424 +++++++++++++++++++++++++++++
>  1 files changed, 424 insertions(+), 0 deletions(-)
>  create mode 100644 crap/0003-btusb-Add-fw-load-support.patch
> 
> diff --git a/crap/0003-btusb-Add-fw-load-support.patch b/crap/0003-btusb-Add-fw-load-support.patch
> new file mode 100644
> index 0000000..6642d6b
> --- /dev/null
> +++ b/crap/0003-btusb-Add-fw-load-support.patch
> @@ -0,0 +1,424 @@
> +Reason for not yet publishing: Marcel feels that Atheros sflash based BT device
> +doesn't follow bluetooth H:2 specification and HCI commands should be supported
> +in firmware if it is detected as bluetooth device. Using HCI command, firmware
> +should be loaded.
> +
> +In sflash based device there is not enough memory to support HCI commands in firmware.
> +So load firmware from btusb when the device comes up.

and why are you just not fixing this properly. You can have a custom
firmware loader with a different product ID. Load the firmware and then
on reset it brings it up with proper Bluetooth USB class identifiers.

Nothing demands that the firmware loading happens via HCI. You can
invent your own protocol easily. See bcm203x firmware loader driver that
has been around since the beginning of Bluetooth.

Regards

Marcel



^ permalink raw reply

* [PATCH] Fix use of uninitialised variable on legacy pairing
From: Luiz Augusto von Dentz @ 2010-10-05  7:40 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

Regression caused by e7daece858070d71cecf6ade4f0e3c93272c53ac:

==23899== Use of uninitialised value of size 4
==23899==    at 0x49CD888: _itoa_word (_itoa.c:196)
==23899==    by 0x49D1109: vfprintf (vfprintf.c:1613)
==23899==    by 0x4A7506C: __vsprintf_chk (vsprintf_chk.c:86)
==23899==    by 0x4A74FAC: __sprintf_chk (sprintf_chk.c:33)
==23899==    by 0x4830E08: ba2str (stdio2.h:34)
==23899==    by 0x1496B3: set_pin_length (security.c:514)
==23899==    by 0x168399: pincode_cb (dbus-hci.c:179)
==23899==    by 0x162E0D: pincode_cb (device.c:2135)
==23899==    by 0x15AD55: pincode_reply (agent.c:416)
==23899==    by 0x49467E0: ??? (in /lib/libdbus-1.so.3.5.2)
==23899==    by 0x4934975: ??? (in /lib/libdbus-1.so.3.5.2)
==23899==    by 0x4937B81: dbus_connection_dispatch (in /lib/libdbus-1.so.3.5.2)
==23899==
==23899== Conditional jump or move depends on uninitialised value(s)
==23899==    at 0x49CD893: _itoa_word (_itoa.c:196)
==23899==    by 0x49D1109: vfprintf (vfprintf.c:1613)
==23899==    by 0x4A7506C: __vsprintf_chk (vsprintf_chk.c:86)
==23899==    by 0x4A74FAC: __sprintf_chk (sprintf_chk.c:33)
==23899==    by 0x4830E08: ba2str (stdio2.h:34)
==23899==    by 0x1496B3: set_pin_length (security.c:514)
==23899==    by 0x168399: pincode_cb (dbus-hci.c:179)
==23899==    by 0x162E0D: pincode_cb (device.c:2135)
==23899==    by 0x15AD55: pincode_reply (agent.c:416)
==23899==    by 0x49467E0: ??? (in /lib/libdbus-1.so.3.5.2)
==23899==    by 0x4934975: ??? (in /lib/libdbus-1.so.3.5.2)
==23899==    by 0x4937B81: dbus_connection_dispatch (in /lib/libdbus-1.so.3.5.2)
---
 src/dbus-hci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index b93dbcd..7309883 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -167,6 +167,7 @@ static void pincode_cb(struct agent *agent, DBusError *derr,
 	bdaddr_t sba, dba;
 	int err;
 
+	adapter_get_address(adapter, &sba);
 	device_get_address(device, &dba);
 
 	err = btd_adapter_pincode_reply(adapter, &dba, derr ? NULL : pincode);
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH] Bluetooth: Support for new firmware for ath3k USB Bluetooth device
From: Marcel Holtmann @ 2010-10-05  7:38 UTC (permalink / raw)
  To: David Vrabel; +Cc: Suraj Sumangala, linux-bluetooth, Jothikumar.Mothilal
In-Reply-To: <4CAACD5A.8040304@csr.com>

Hi David,

> > This patch add support for new ath3k USB Bluetooth device firmare.
> > The firmware implements shared antenna support and
> > fixes few critical bugs.
> [...]
> >  static int ath3k_probe(struct usb_interface *intf,
> >  			const struct usb_device_id *id)
> >  {
> > @@ -110,6 +113,8 @@ static int ath3k_probe(struct usb_interface *intf,
> >  	struct usb_device *udev = interface_to_usbdev(intf);
> >  	struct ath3k_data *data;
> >  	int size;
> > +	int i;
> > +	char fw_file[MAXPATHLEN]
> 
> Suggest FW_PATH_LEN here to avoid confusion as most people would expect
> MAXPATHLEN to be enormous.
> 
> >  	BT_DBG("intf %p id %p", intf, id);
> >  
> > @@ -122,7 +127,16 @@ static int ath3k_probe(struct usb_interface *intf,
> >  
> >  	data->udev = udev;
> >  
> > -	if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {
> 
> Why not request ath3k-1.fw (for backward compatibility) and ath3k.fw and
> symlink this name to ath3k-2.fw or any future version of firmware?
> 
> I think users should be able to update firmware without a kernel update
> where possible.

yes, it is mandatory to support at least the last two firmware version.
Just forcing a firmware update with a new kernel is not going to work.

So NAK on this patch.

Regards

Marcel



^ permalink raw reply

* Re: Inquiry_with_RSSI compatible dongles
From: Marcel Holtmann @ 2010-10-05  7:36 UTC (permalink / raw)
  To: Giedo Mak; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=uuyRQ-9Qj1THDvOroE3yLU7ZBJS_tX26h0O4y@mail.gmail.com>

Hi Giedo,

> I'm working on a bluetooth program with some sort of distance sensing/tracking.
> To make this easier I came across a feature called inquiry_with_RSSI.
> Could somebody tell me what kind of dongles support this feature. I
> mean, does every 2.1 BT dongle support it, or can you only find out
> once you get one in your hands?

in general every Bluetooth 1.2 dongle and later should support Inquiry
with RSSI. I still have to come across a 2.1 dongle that doesn't.

Regards

Marcel



^ permalink raw reply

* Re: pull-request: bluetooth-2.6 2010-09-27
From: David Miller @ 2010-10-05  7:06 UTC (permalink / raw)
  To: padovan; +Cc: linville, marcel, linux-bluetooth, netdev
In-Reply-To: <20101004223513.GB3234@vigoh>

From: "Gustavo F. Padovan" <padovan@profusion.mobi>
Date: Mon, 4 Oct 2010 19:35:13 -0300

> Follow the output of git show for that change, if we agree on the change I
> can append it to the bluetooth pull request.

That makes sense to me, thanks for doing this audit.

Append that commit and send a new pull request.

Thanks!

^ permalink raw reply

* Re: [PATCH] Bluetooth: Support for new firmware for ath3k USB Bluetooth device
From: David Vrabel @ 2010-10-05  7:01 UTC (permalink / raw)
  To: Suraj Sumangala; +Cc: linux-bluetooth, Jothikumar.Mothilal
In-Reply-To: <1286259436-25424-1-git-send-email-suraj@atheros.com>

On 05/10/2010 07:17, Suraj Sumangala wrote:
> This patch add support for new ath3k USB Bluetooth device firmare.
> The firmware implements shared antenna support and
> fixes few critical bugs.
[...]
>  static int ath3k_probe(struct usb_interface *intf,
>  			const struct usb_device_id *id)
>  {
> @@ -110,6 +113,8 @@ static int ath3k_probe(struct usb_interface *intf,
>  	struct usb_device *udev = interface_to_usbdev(intf);
>  	struct ath3k_data *data;
>  	int size;
> +	int i;
> +	char fw_file[MAXPATHLEN]

Suggest FW_PATH_LEN here to avoid confusion as most people would expect
MAXPATHLEN to be enormous.

>  	BT_DBG("intf %p id %p", intf, id);
>  
> @@ -122,7 +127,16 @@ static int ath3k_probe(struct usb_interface *intf,
>  
>  	data->udev = udev;
>  
> -	if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {

Why not request ath3k-1.fw (for backward compatibility) and ath3k.fw and
symlink this name to ath3k-2.fw or any future version of firmware?

I think users should be able to update firmware without a kernel update
where possible.

David

^ permalink raw reply

* Re: [PATCH] Bluetooth: Add support to specify patch download location
From: Johan Hedberg @ 2010-10-05  6:56 UTC (permalink / raw)
  To: Suraj Sumangala; +Cc: linux-bluetooth, Jothikumar.Mothilal
In-Reply-To: <1286175976-20477-1-git-send-email-suraj@atheros.com>

Hi,

On Mon, Oct 04, 2010, Suraj Sumangala wrote:
> This patch add support for specifying patch download address for
> AR300x patch download logic.
> 
> ---
>  tools/hciattach_ath3k.c |   52 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 50 insertions(+), 2 deletions(-)

The patch has been pushed upstream. Thanks.

Johan

^ permalink raw reply

* [PATCH] Bluetooth: Support for new firmware for ath3k USB Bluetooth device
From: Suraj Sumangala @ 2010-10-05  6:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jothikumar.Mothilal, Suraj Sumangala

This patch add support for new ath3k USB Bluetooth device firmare.
The firmware implements shared antenna support and
fixes few critical bugs.

Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
 drivers/bluetooth/ath3k.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index 128cae4..eb79a8f 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -103,6 +103,9 @@ error:
 	return err;
 }
 
+#define FW_COUNT	2
+#define FW_PREFIX	"ath3k-"
+#define MAXPATHLEN	30
 static int ath3k_probe(struct usb_interface *intf,
 			const struct usb_device_id *id)
 {
@@ -110,6 +113,8 @@ static int ath3k_probe(struct usb_interface *intf,
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct ath3k_data *data;
 	int size;
+	int i;
+	char fw_file[MAXPATHLEN];
 
 	BT_DBG("intf %p id %p", intf, id);
 
@@ -122,7 +127,16 @@ static int ath3k_probe(struct usb_interface *intf,
 
 	data->udev = udev;
 
-	if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {
+	for (i = FW_COUNT; i > 0; i--) {
+		snprintf(fw_file, MAXPATHLEN - 1, "%s%d.fw", FW_PREFIX, i);
+
+		if (request_firmware(&firmware, fw_file,
+						&udev->dev) >= 0)
+			break;
+
+	}
+
+	if (!i) {
 		kfree(data);
 		return -EIO;
 	}
-- 
1.7.0.4


^ permalink raw reply related

* RE: How add support of LLMNR for PAN
From: Chetankumar Manjegowda @ 2010-10-05  4:47 UTC (permalink / raw)
  To: Arun Kumar; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTimguxTRuPB2Dgqm4w5k-MeOxDdCYDn-T_UvOFRL@mail.gmail.com>

SGkgQXJ1biwNClRoYW5rcyBmb3IgeW91ciByZXBseS4NCg0KSSBhbSBuZXcgdG8gbmV0d29ya2lu
ZyBJIGFtIG5vdCBzdXJlIGFib3V0IHRoYXQuDQoNCkkgd2FudCB0byBzZW5kIExMTU5SIHF1ZXJ5
IHNvIGkgYW0gcGxhbm5pbmcgdG8gdXNlIGF2YWhpLTAuNi4yNyhhdmFoaS1hdXRvaXBkKSBpcyB0
aGlzIGlzIGZpbmUgb3IgYW55IG90aGVyIGlzIGF2YWlsYWJsZSA/DQoNClJnZHMNCkNoZXRhbg0K
DQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBBcnVuIEt1bWFyIFttYWlsdG86
YXJ1bmthdEBnbWFpbC5jb21dIA0KU2VudDogTW9uZGF5LCBPY3RvYmVyIDA0LCAyMDEwIDk6MDYg
UE0NClRvOiBDaGV0YW5rdW1hciBNYW5qZWdvd2RhDQpDYzogbGludXgtYmx1ZXRvb3RoQHZnZXIu
a2VybmVsLm9yZw0KU3ViamVjdDogUmU6IEhvdyBhZGQgc3VwcG9ydCBvZiBMTE1OUiBmb3IgUEFO
DQoNClNob3VsZG4ndCB0aGlzIHN1cHBvcnQgY29tZSBmcm9tIGxpbnV4IGlwIHN0YWNrIHJhdGhl
ciB0aGFuIGJsdWV6Pw0KDQoNCkJlc3QgUmVnYXJkcywNCkFydW4gS3VtYXIgU2luZ2gNCnd3dy5j
cmF6eWRha3MuY29tDQoNCg0KT24gTW9uLCBPY3QgNCwgMjAxMCBhdCA2OjE1IFBNLCBDaGV0YW5r
dW1hciBNYW5qZWdvd2RhDQo8Q2hldGFua3VtYXJfTWFuamVnb3dkYUBtaW5kdHJlZS5jb20+IHdy
b3RlOg0KPg0KPiBIaSBBbGwsDQo+DQo+IEkgd2FudCB0byBrbm93IGhvdyB0byBlbmFibGUvSW50
ZWdyYXRlIExMTU5SIGZvciBQQU4gcHJvZmlsZS4NCj4NCj4gQ2FuIGFueW9uZSBnaXZlIHNvbWUg
c3VnZ2VzdGlvbnMgb3IgbGluayB3aGVyZSBJIGNhbiByZWZlciB0aGUgc2FtZSA/Pw0KPg0KPiBS
Z2RzDQo+IENoZXRhbg0KPg0KPiBfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXw0KPg0K
PiBodHRwOi8vd3d3Lm1pbmR0cmVlLmNvbS9lbWFpbC9kaXNjbGFpbWVyLmh0bWwNCj4gLS0NCj4g
VG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJl
IGxpbnV4LWJsdWV0b290aCIgaW4NCj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9t
b0B2Z2VyLmtlcm5lbC5vcmcNCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCDCoGh0dHA6Ly92Z2Vy
Lmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0KDQoNCg0KLS0NCg==

^ permalink raw reply

* Re: pull-request: bluetooth-2.6 2010-09-27
From: Gustavo F. Padovan @ 2010-10-04 22:35 UTC (permalink / raw)
  To: David Miller; +Cc: linville, marcel, linux-bluetooth, netdev
In-Reply-To: <20101001012203.GE3048@vigoh>

Hi Dave,

* Gustavo F. Padovan <padovan@profusion.mobi> [2010-09-30 22:22:03 -0300]:

> Hi Dave,
> 
> * David Miller <davem@davemloft.net> [2010-09-30 17:26:57 -0700]:
> 
> > From: "Gustavo F. Padovan" <padovan@profusion.mobi>
> > Date: Tue, 28 Sep 2010 19:49:41 -0300
> > 
> > > Actually sk_stream_wait_memory is another point why it's safe to release
> > > the lock and block waiting for memory. We've been doing that safely in
> > > protocols like TCP, SCTP and DCCP for a long time.
> > 
> > Do you notice what TCP does when sk_stream_wait_memory() returns?
> > 
> > It reloads all volatile state that might have changed in the socket
> > while the lock was dropped.
> > 
> > For example, TCP will reload the current MSS that can change
> > asynchronously while we don't have the socket lock.
> 
> I got your point. And what I tried to say in the last e-mail is that
> ERTM doesn't have such volatile states that need to restore after get
> the lock back. The others code path it affect are very simple and also
> doesn't have such problem. So we are safe against asynchronous changes.
> We obvious have volatiles states, but the code paths where
> bt_skb_send_alloc() is used doesn't rely on that states. I'm seeing no
> problem on release the lock, alloc memory, and lock it again.

I did a proper audit of the code paths:

sco_send_frame(): after bt_skb_send_alloc() returns the function doesn't
touch any sk state anymore, it just picks the skb and sends it to the
HCI.

l2cap_create_connless_pdu(): after bt_skb_send_alloc() returns the
function only reads l2cap_pi(sk)->dcid and l2cap_pi(sk)->psm, those
value are static and don't change with the connection alive

l2cap_create_connless_pdu(): after bt_skb_send_alloc() returns the
function only reads l2cap_pi(sk)->dcid, that value is static and doesn't
change with the connection alive.

l2cap_create_iframe_pdu(): after bt_skb_send_alloc() returns, we only 
reads l2cap_pi(sk)->dcid and l2cap_pi(sk)->fcs which doesn change with 
the connection alive, l2cap_create_iframe_pdu() returns the skb to 
l2cap_sar_segment_sdu(), which changes only the TX_QUEUE(sk), but it 
only appends in the end while the rest of the L2CAP code reads from 
the begin. After that we have the process of sending of ERTM and 
Streaming Mode, this process is independent from the skb alloc process,
so no problem here.

Other point already discussed are the check after get the lock again, 
they guarantee that the socket is still connected, otherwise returns 
error.

An extra check will be added to setsockopt() just to make sure that
l2cap_pi(sk)->fcs won't change while the sk_state is BT_CONNECTED. That
was an issue even before this backlog queue issue.

Follow the output of git show for that change, if we agree on the change I
can append it to the bluetooth pull request.


Author: Gustavo F. Padovan <padovan@profusion.mobi>
Date:   Mon Oct 4 19:28:52 2010 -0300

    Bluetooth: Disallow to change L2CAP_OPTIONS values when connected
    
    L2CAP doesn't permit change like MTU, FCS, TxWindow values while the
    connection is alive, we can only set that before the
    connection/configuration process. That can lead to bugs in the L2CAP
    operation.
    
    Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 44a8fb0..0b54b7d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1950,6 +1950,11 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 
        switch (optname) {
        case L2CAP_OPTIONS:
+               if (sk->sk_state == BT_CONNECTED) {
+                       err = -EINVAL;
+                       break;
+               }
+
                opts.imtu     = l2cap_pi(sk)->imtu;
                opts.omtu     = l2cap_pi(sk)->omtu;
                opts.flush_to = l2cap_pi(sk)->flush_to;


-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply related

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuous flag
From: Gustavo F. Padovan @ 2010-10-04 21:00 UTC (permalink / raw)
  To: haijun liu; +Cc: linux-bluetooth@vger.kernel.org, dan.tian, suraj, Ville Tervo
In-Reply-To: <AANLkTi=-4yF2qew7YVnESPd_jt7g8bvS12XYkhx27ko8@mail.gmail.com>

Hi Haijun,

* haijun liu <liuhaijun.er@gmail.com> [2010-10-01 22:43:52 +0800]:

> Replace hard code of configuration continuous flag with
> self-commented macro L2CAP_CONF_FLAG_CONT in functions
> l2cap_config_req() and l2cap_config_rsp().
> 
> Signed-off-by: Haijun.Liu <haijun.liu@atheros.com>

Put a ' ' instead of '.' to separate your surname.

> ---
>  include/net/bluetooth/l2cap.h |    2 ++
>  net/bluetooth/l2cap_core.c    |    6 +++---
>  2 files changed, 5 insertions(+), 3 deletions(-)

Patch is corrupted again, please check where you are screwing up on
sending patches to the mailing list, then we can save my time and your
time. ;)

Applying: Bluetooth: Replace hard code of configuration continuous flag
fatal: corrupt patch at line 11
Patch failed at 0001 Bluetooth: Replace hard code of configuration
continuous flag

Regards,

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: Pull request: git://git.infradead.org/users/cktakahasi/bluez.git for-upstream
From: Johan Hedberg @ 2010-10-04 20:43 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: BlueZ development
In-Reply-To: <AANLkTinF1OWKMzuArUS6ptWiDTtvQuXxxW9B5ks3tebW@mail.gmail.com>

Hi Claudio,

On Mon, Oct 04, 2010, Claudio Takahasi wrote:
> The following changes since commit 19c8e310329bb6b369f11501afec9ff4223c2971:
> 
>   Allow errors to propagate from connection callback (2010-10-04 17:06:49 +0200)
> 
> are available in the git repository at:
>   git://git.infradead.org/users/cktakahasi/bluez.git for-upstream
> 
> Claudio Takahasi (23):
>       Add LE start and stop scanning
>       Remove RSSI field from the advertising report event
>       Decoding the RSSI parameter from the advertising report event
>       Send Discovering property "FALSE" when the interleave finishes
>       Add length argument on hciops start discovery function
>       Stop inquiry using the length parameter
>       Fix remote name resolution for interleave discovery
>       Add Write LE host supported function
>       Set the LE host supported and disable simultaneous LE and BR/EDR flags
>       Add extended feature mask constants definition
>       Read the local extended features
>       Stop LE scanning when discovery is suspended
>       Rename hciops {start, stop}_discovery to {start, stop}_inquiry
>       Don't enter on interleave mode if there isn't active sessions
>       Code cleanup: improving inquiry logic
>       Clear the remote device found list in the state transition
>       Fix periodic inquiry signals
>       Fixing DeviceDisappeared signal
>       Postpone discovery if still resolving names
>       Add adapter discovery type function
>       Do not send another Discovering TRUE signal if still resolving names
>       Forcing periodic inquiry exit
>       Fix interleave scanning
> 
> Vinicius Costa Gomes (1):
>       Add BR/EDR LE interleaved discovery
> 
>  lib/hci.c        |   29 +++++
>  lib/hci.h        |   17 +++-
>  lib/hci_lib.h    |    1 +
>  plugins/hciops.c |   69 +++++++++++-
>  src/adapter.c    |  301 +++++++++++++++++++++++++++++++++++++-----------------
>  src/adapter.h    |   38 ++++---
>  src/dbus-hci.c   |   45 +++++++--
>  src/dbus-hci.h   |    1 +
>  src/security.c   |  131 ++++++++++++++----------
>  9 files changed, 449 insertions(+), 183 deletions(-)

Thanks. Pushed upstream.

Johan

^ permalink raw reply

* Re: [PATCH v2 1/4] Fix constness of att_get_u{8,16,32}() functions
From: Johan Hedberg @ 2010-10-04 19:17 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1286219584-8199-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Anderson,

All four patches have been pushed upstream. Thanks.

Johan

^ permalink raw reply

* [PATCH v2 4/4] Modify dec_read_req() to get PDU length as parameter
From: Anderson Lizardo @ 2010-10-04 19:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1286219584-8199-1-git-send-email-anderson.lizardo@openbossa.org>

This is consistent with other att.h functions, and allows length checks.
---
 attrib/att.c        |    5 ++++-
 attrib/att.h        |    2 +-
 src/attrib-server.c |    2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index b18e1d6..2ffa8ce 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -379,7 +379,7 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
 	return min_len;
 }
 
-uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
+uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
 {
 	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
 
@@ -389,6 +389,9 @@ uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 	if (handle == NULL)
 		return 0;
 
+	if (len < min_len)
+		return 0;
+
 	if (pdu[0] != ATT_OP_READ_REQ)
 		return 0;
 
diff --git a/attrib/att.h b/attrib/att.h
index 3f1e239..e63c58e 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -179,7 +179,7 @@ uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
 						uint8_t *value, int *vlen);
 struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, int len);
 uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
-uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle);
+uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
 uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
 uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
 uint16_t enc_error_resp(uint8_t opcode, uint16_t handle, uint8_t status,
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 475b68b..b45f300 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -478,7 +478,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 		length = read_by_type(start, end, &uuid, opdu, channel->mtu);
 		break;
 	case ATT_OP_READ_REQ:
-		length = dec_read_req(ipdu, &start);
+		length = dec_read_req(ipdu, len, &start);
 		if (length == 0) {
 			status = ATT_ECODE_INVALID_PDU;
 			goto done;
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH v2 3/4] Replace hardcoded minimum length values with constants
From: Anderson Lizardo @ 2010-10-04 19:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1286219584-8199-1-git-send-email-anderson.lizardo@openbossa.org>

---
 attrib/att.c |  128 ++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 84 insertions(+), 44 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index 21659f0..b18e1d6 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -90,6 +90,7 @@ void att_data_list_free(struct att_data_list *list)
 uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end);
 	uint16_t length;
 
 	if (!uuid)
@@ -102,7 +103,7 @@ uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		return 0;
 
-	if (len < 5 + length)
+	if (len < min_len + length)
 		return 0;
 
 	pdu[0] = ATT_OP_READ_BY_GROUP_REQ;
@@ -114,12 +115,14 @@ uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		memcpy(&pdu[5], &uuid->value.uuid128, length);
 
-	return 5 + length;
+	return min_len + length;
 }
 
 uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
 						uint16_t *end, uuid_t *uuid)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+
 	if (pdu == NULL)
 		return 0;
 
@@ -129,12 +132,12 @@ uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (pdu[0] != ATT_OP_READ_BY_GROUP_REQ)
 		return 0;
 
-	if (len < 7)
+	if (len < min_len + 2)
 		return 0;
 
 	*start = att_get_u16(&pdu[1]);
 	*end = att_get_u16(&pdu[3]);
-	if (len == 7)
+	if (len == min_len + 2)
 		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
@@ -203,6 +206,7 @@ uint16_t enc_find_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end);
 	uint16_t length;
 
 	if (!uuid)
@@ -215,7 +219,7 @@ uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		return 0;
 
-	if (len < 5 + length)
+	if (len < min_len + length)
 		return 0;
 
 	pdu[0] = ATT_OP_READ_BY_TYPE_REQ;
@@ -227,19 +231,21 @@ uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		memcpy(&pdu[5], &uuid->value.uuid128, length);
 
-	return 5 + length;
+	return min_len + length;
 }
 
 uint16_t dec_read_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
 						uint16_t *end, uuid_t *uuid)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (start == NULL || end == NULL || uuid == NULL)
 		return 0;
 
-	if (len < 7)
+	if (len < min_len + 2)
 		return 0;
 
 	if (pdu[0] != ATT_OP_READ_BY_TYPE_REQ)
@@ -248,7 +254,7 @@ uint16_t dec_read_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
 	*start = att_get_u16(&pdu[1]);
 	*end = att_get_u16(&pdu[3]);
 
-	if (len == 7)
+	if (len == min_len + 2)
 		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
@@ -311,64 +317,72 @@ struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, int len)
 uint16_t enc_write_cmd(uint16_t handle, const uint8_t *value, int vlen,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
-	if (vlen > len - 3)
-		vlen = len - 3;
+	if (vlen > len - min_len)
+		vlen = len - min_len;
 
 	pdu[0] = ATT_OP_WRITE_CMD;
 	att_put_u16(handle, &pdu[1]);
 
 	if (vlen > 0) {
-		memcpy(pdu + 3, value, vlen);
-		return 3 + vlen;
+		memcpy(&pdu[3], value, vlen);
+		return min_len + vlen;
 	}
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
 						uint8_t *value, int *vlen)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (value == NULL || vlen == NULL || handle == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	if (pdu[0] != ATT_OP_WRITE_CMD)
 		return 0;
 
 	*handle = att_get_u16(&pdu[1]);
-	memcpy(value, pdu + 3, len - 3);
-	*vlen = len - 3;
+	memcpy(value, pdu + min_len, len - min_len);
+	*vlen = len - min_len;
 
 	return len;
 }
 
 uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_READ_REQ;
 	att_put_u16(handle, &pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
+
 	if (pdu == NULL)
 		return 0;
 
@@ -380,7 +394,7 @@ uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 
 	*handle = att_get_u16(&pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len)
@@ -422,9 +436,11 @@ uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen)
 uint16_t enc_error_resp(uint8_t opcode, uint16_t handle, uint8_t status,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(opcode) +
+						sizeof(handle) + sizeof(status);
 	uint16_t u16;
 
-	if (len < 5)
+	if (len < min_len)
 		return 0;
 
 	u16 = htobs(handle);
@@ -433,31 +449,35 @@ uint16_t enc_error_resp(uint8_t opcode, uint16_t handle, uint8_t status,
 	memcpy(&pdu[2], &u16, sizeof(u16));
 	pdu[4] = status;
 
-	return 5;
+	return min_len;
 }
 
 uint16_t enc_find_info_req(uint16_t start, uint16_t end, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 5)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_FIND_INFO_REQ;
 	att_put_u16(start, &pdu[1]);
 	att_put_u16(end, &pdu[3]);
 
-	return 5;
+	return min_len;
 }
 
 uint16_t dec_find_info_req(const uint8_t *pdu, int len, uint16_t *start,
 								uint16_t *end)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 5)
+	if (len < min_len)
 		return 0;
 
 	if (start == NULL || end == NULL)
@@ -469,7 +489,7 @@ uint16_t dec_find_info_req(const uint8_t *pdu, int len, uint16_t *start,
 	*start = att_get_u16(&pdu[1]);
 	*end = att_get_u16(&pdu[3]);
 
-	return 5;
+	return min_len;
 }
 
 uint16_t enc_find_info_resp(uint8_t format, struct att_data_list *list,
@@ -520,10 +540,11 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
 
 	list = malloc(sizeof(struct att_data_list));
 
+	list->len = sizeof(pdu[0]) + sizeof(*format);
 	if (*format == 0x01)
-		list->len = 4;
+		list->len += 2;
 	else if (*format == 0x02)
-		list->len = 18;
+		list->len += 16;
 
 	list->num = (len - 2) / list->len;
 	list->data = malloc(sizeof(uint8_t *) * list->num);
@@ -541,36 +562,42 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
 
 uint16_t enc_notification(struct attribute *a, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < (a->len + 3))
+	if (len < (a->len + min_len))
 		return 0;
 
 	pdu[0] = ATT_OP_HANDLE_NOTIFY;
 	att_put_u16(a->handle, &pdu[1]);
 	memcpy(&pdu[3], a->data, a->len);
 
-	return a->len + 3;
+	return a->len + min_len;
 }
 
 uint16_t enc_indication(struct attribute *a, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < (a->len + 3))
+	if (len < (a->len + min_len))
 		return 0;
 
 	pdu[0] = ATT_OP_HANDLE_IND;
 	att_put_u16(a->handle, &pdu[1]);
 	memcpy(&pdu[3], a->data, a->len);
 
-	return a->len + 3;
+	return a->len + min_len;
 }
 
 struct attribute *dec_indication(const uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
+
 	struct attribute *a;
 
 	if (pdu == NULL)
@@ -579,11 +606,14 @@ struct attribute *dec_indication(const uint8_t *pdu, int len)
 	if (pdu[0] != ATT_OP_HANDLE_IND)
 		return NULL;
 
-	a = malloc(sizeof(struct attribute) + len - 3);
+	if (len < min_len)
+		return NULL;
+
+	a = malloc(sizeof(struct attribute) + len - min_len);
 	if (a == NULL)
 		return NULL;
 
-	a->len = len - 3;
+	a->len = len - min_len;
 
 	a->handle = att_get_u16(&pdu[1]);
 	memcpy(a->data, &pdu[3], a->len);
@@ -593,40 +623,46 @@ struct attribute *dec_indication(const uint8_t *pdu, int len)
 
 uint16_t enc_confirmation(uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 1)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_HANDLE_CNF;
 
-	return 1;
+	return min_len;
 }
 
 uint16_t enc_mtu_req(uint16_t mtu, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(mtu);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_MTU_REQ;
 	att_put_u16(mtu, &pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*mtu);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (mtu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	if (pdu[0] != ATT_OP_MTU_REQ)
@@ -634,32 +670,36 @@ uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu)
 
 	*mtu = att_get_u16(&pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t enc_mtu_resp(uint16_t mtu, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(mtu);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_MTU_RESP;
 	att_put_u16(mtu, &pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*mtu);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (mtu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	if (pdu[0] != ATT_OP_MTU_RESP)
@@ -667,5 +707,5 @@ uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
 
 	*mtu = att_get_u16(&pdu[1]);
 
-	return 3;
+	return min_len;
 }
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH v2 2/4] Remove typecast from att_get_u16() calls
From: Anderson Lizardo @ 2010-10-04 19:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1286219584-8199-1-git-send-email-anderson.lizardo@openbossa.org>

att_get_u16() already does a cast of the void* parameter to uint16_t*.
---
 attrib/att.c      |   26 +++++++++++++-------------
 attrib/client.c   |   19 +++++++++----------
 attrib/gatttool.c |   21 +++++++++------------
 3 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index 6c697f8..21659f0 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -132,10 +132,10 @@ uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (len < 7)
 		return 0;
 
-	*start = att_get_u16((uint16_t *) &pdu[1]);
-	*end = att_get_u16((uint16_t *) &pdu[3]);
+	*start = att_get_u16(&pdu[1]);
+	*end = att_get_u16(&pdu[3]);
 	if (len == 7)
-		sdp_uuid16_create(uuid, att_get_u16((uint16_t *) &pdu[5]));
+		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
 
@@ -245,11 +245,11 @@ uint16_t dec_read_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (pdu[0] != ATT_OP_READ_BY_TYPE_REQ)
 		return 0;
 
-	*start = att_get_u16((uint16_t *) &pdu[1]);
-	*end = att_get_u16((uint16_t *) &pdu[3]);
+	*start = att_get_u16(&pdu[1]);
+	*end = att_get_u16(&pdu[3]);
 
 	if (len == 7)
-		sdp_uuid16_create(uuid, att_get_u16((uint16_t *) &pdu[5]));
+		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
 
@@ -346,7 +346,7 @@ uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
 	if (pdu[0] != ATT_OP_WRITE_CMD)
 		return 0;
 
-	*handle = att_get_u16((uint16_t *) &pdu[1]);
+	*handle = att_get_u16(&pdu[1]);
 	memcpy(value, pdu + 3, len - 3);
 	*vlen = len - 3;
 
@@ -378,7 +378,7 @@ uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 	if (pdu[0] != ATT_OP_READ_REQ)
 		return 0;
 
-	*handle = att_get_u16((uint16_t *) &pdu[1]);
+	*handle = att_get_u16(&pdu[1]);
 
 	return 3;
 }
@@ -466,8 +466,8 @@ uint16_t dec_find_info_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (pdu[0] != ATT_OP_FIND_INFO_REQ)
 		return 0;
 
-	*start = att_get_u16((uint16_t *) &pdu[1]);
-	*end = att_get_u16((uint16_t *) &pdu[3]);
+	*start = att_get_u16(&pdu[1]);
+	*end = att_get_u16(&pdu[3]);
 
 	return 5;
 }
@@ -585,7 +585,7 @@ struct attribute *dec_indication(const uint8_t *pdu, int len)
 
 	a->len = len - 3;
 
-	a->handle = att_get_u16((uint16_t *) &pdu[1]);
+	a->handle = att_get_u16(&pdu[1]);
 	memcpy(a->data, &pdu[3], a->len);
 
 	return a;
@@ -632,7 +632,7 @@ uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu)
 	if (pdu[0] != ATT_OP_MTU_REQ)
 		return 0;
 
-	*mtu = att_get_u16((uint16_t *) &pdu[1]);
+	*mtu = att_get_u16(&pdu[1]);
 
 	return 3;
 }
@@ -665,7 +665,7 @@ uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
 	if (pdu[0] != ATT_OP_MTU_RESP)
 		return 0;
 
-	*mtu = att_get_u16((uint16_t *) &pdu[1]);
+	*mtu = att_get_u16(&pdu[1]);
 
 	return 3;
 }
diff --git a/attrib/client.c b/attrib/client.c
index db84b78..cd720e6 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -286,7 +286,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len,
 	struct primary *prim;
 	GSList *lprim, *lchr;
 	uint8_t opdu[ATT_MAX_MTU];
-	guint handle = att_get_u16((uint16_t *) &pdu[1]);
+	guint handle = att_get_u16(&pdu[1]);
 	uint16_t olen;
 
 	for (lprim = gatt->primary, prim = NULL, chr = NULL; lprim;
@@ -872,11 +872,10 @@ static void descriptor_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		uint8_t *info = list->data[i];
 		struct query_data *qfmt;
 
-		handle = att_get_u16((uint16_t *) info);
+		handle = att_get_u16(info);
 
 		if (format == 0x01) {
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&info[2]));
+			sdp_uuid16_create(&uuid, att_get_u16(&info[2]));
 		} else {
 			/* Currently, only "user description" and "presentation
 			 * format" descriptors are used, and both have 16-bit
@@ -963,17 +962,17 @@ static void char_discovered_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		chr = g_new0(struct characteristic, 1);
 		chr->prim = prim;
 		chr->perm = decl[2];
-		chr->handle = att_get_u16((uint16_t *) &decl[3]);
+		chr->handle = att_get_u16(&decl[3]);
 		chr->path = g_strdup_printf("%s/characteristic%04x",
 						prim->path, chr->handle);
 		if (list->len == 7) {
 			sdp_uuid16_create(&chr->type,
-					att_get_u16((uint16_t *) &decl[5]));
+					att_get_u16(&decl[5]));
 		} else
 			sdp_uuid128_create(&chr->type, &decl[5]);
 
 		if (previous_end) {
-			*previous_end = att_get_u16((uint16_t *) decl);
+			*previous_end = att_get_u16(decl);
 		}
 
 		last = chr->handle;
@@ -1270,8 +1269,8 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 
 		/* Each element contains: attribute handle, end group handle
 		 * and attribute value */
-		start = att_get_u16((uint16_t *) info);
-		end = att_get_u16((uint16_t *) &info[2]);
+		start = att_get_u16(info);
+		end = att_get_u16(&info[2]);
 
 		prim = g_new0(struct primary, 1);
 		prim->gatt = gatt;
@@ -1280,7 +1279,7 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 
 		if (list->len == 6) {
 			sdp_uuid16_create(&prim->uuid,
-					att_get_u16((uint16_t *) &info[4]));
+					att_get_u16(&info[4]));
 
 		} else if (list->len == 20) {
 			/* FIXME: endianness */
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index 18f32a2..4a66340 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -167,14 +167,13 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		/* Each element contains: attribute handle, end group handle
 		 * and attribute value */
 		length = list->len - 2 * sizeof(uint16_t);
-		start = att_get_u16((uint16_t *) value);
-		end = att_get_u16((uint16_t *) &value[2]);
+		start = att_get_u16(value);
+		end = att_get_u16(&value[2]);
 
 		g_print("attr handle = 0x%04x, end grp handle = 0x%04x, ",
 								start, end);
 		if (length == 2)
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&value[4]));
+			sdp_uuid16_create(&uuid, att_get_u16(&value[4]));
 		else
 			sdp_uuid128_create(&uuid, value + 4);
 
@@ -204,7 +203,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
 	uint8_t opdu[ATT_MAX_MTU];
 	uint16_t handle, i, olen = 0;
 
-	handle = att_get_u16((uint16_t *) &pdu[1]);
+	handle = att_get_u16(&pdu[1]);
 
 	switch (pdu[0]) {
 	case ATT_OP_HANDLE_NOTIFY:
@@ -279,15 +278,14 @@ static void char_discovered_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		char uuidstr[MAX_LEN_UUID_STR];
 		uuid_t uuid;
 
-		last = att_get_u16((uint16_t *) value);
+		last = att_get_u16(value);
 
 		g_print("handle = 0x%04x, char properties = 0x%02x, "
 			"char value handle = 0x%04x, ", last, value[2],
-			att_get_u16((uint16_t *) &value[3]));
+			att_get_u16(&value[3]));
 
 		if (list->len == 7)
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&value[5]));
+			sdp_uuid16_create(&uuid, att_get_u16(&value[5]));
 		else
 			sdp_uuid128_create(&uuid, value + 5);
 
@@ -447,11 +445,10 @@ static void char_desc_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		uuid_t uuid;
 
 		value = list->data[i];
-		handle = att_get_u16((uint16_t *) value);
+		handle = att_get_u16(value);
 
 		if (format == 0x01)
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&value[2]));
+			sdp_uuid16_create(&uuid, att_get_u16(&value[2]));
 		else
 			sdp_uuid128_create(&uuid, &value[2]);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH v2 1/4] Fix constness of att_get_u{8,16,32}() functions
From: Anderson Lizardo @ 2010-10-04 19:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

---
 attrib/att.h |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 69071b4..3f1e239 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -123,21 +123,21 @@ struct att_data_list {
 };
 
 /* These functions do byte conversion */
-static inline uint8_t att_get_u8(void *ptr)
+static inline uint8_t att_get_u8(const void *ptr)
 {
-	uint8_t *u8_ptr = ptr;
+	const uint8_t *u8_ptr = ptr;
 	return bt_get_unaligned(u8_ptr);
 }
 
-static inline uint16_t att_get_u16(void *ptr)
+static inline uint16_t att_get_u16(const void *ptr)
 {
-	uint16_t *u16_ptr = ptr;
+	const uint16_t *u16_ptr = ptr;
 	return btohs(bt_get_unaligned(u16_ptr));
 }
 
-static inline uint32_t att_get_u32(void *ptr)
+static inline uint32_t att_get_u32(const void *ptr)
 {
-	uint32_t *u32_ptr = ptr;
+	const uint32_t *u32_ptr = ptr;
 	return btohl(bt_get_unaligned(u32_ptr));
 }
 
-- 
1.7.0.4


^ permalink raw reply related

* Re: 2.6.36-rc6-git2: Reported regressions from 2.6.35
From: Justin P. Mattock @ 2010-10-04 18:57 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Rafael J. Wysocki, linux-wireless, linux-bluetooth, Bob Copeland,
	Pavel Machek, linux
In-Reply-To: <AANLkTimUdaUkpXmNt5mPkCdWmHvBbXWA6smAP4U80yc4@mail.gmail.com>

On 10/04/2010 11:35 AM, Luis R. Rodriguez wrote:
> Below are the 802.11 and Bluetooth ones:
>
> On Sun, Oct 3, 2010 at 2:15 PM, Rafael J. Wysocki<rjw@sisk.pl>  wrote:
>> Unresolved regressions
>> ----------------------
>
>> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=19392
>> Subject         : WARNING: at drivers/net/wireless/ath/ath5k/base.c:3475 ath5k_bss_info_changed+0x44/0x168 [ath5k]()
>> Submitter       : Justin Mattock<justinmattock@gmail.com>
>> Date            : 2010-09-28 22:30 (6 days old)
>> Message-ID      :<AANLkTim5WCGKPvEkOkO_YnMF9pg8mvLfQoFBNUFpfa_k@mail.gmail.com>
>> References      : http://marc.info/?l=linux-kernel&m=128571307018635&w=2
>
> WTF -- this ended up in a bisect pointing to some PCMCIA patch! Justin:
>
>    * does this happen with wireless-testing.git ?

no this is the current Linus tree(have not tried any other tree).
as for the unresolved regression, this just was filed a few days ago..

>    * Did you do the bisect on the entire kernel? What git tree are you
> using to bisect?
>

the bisect had to be between 2.6.34 and 2.6.32 due to pcmcia breaking 
after 2.6.34(there is a bug report for that, but got resolved)

> Did you simply bring up the interface and then suspsend? Did you not
> add a new interface in between this? ath5k's add_interace has:
>
>          if (sc->vif) {
>                  ret = 0;
>                  goto end;
>          }
>
> But why does it just allow this to go through without complaining if
> multiple vifs are not supported?
>
>

the system is opensuse 11.2, I just simply start the machine boot up,
after nm connects I open a terminal then suspend in the terminal, upon 
wakeup this message appears.

>>
>> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=17061
>> Subject         : 2.6.36-rc1 on zaurus: bluetooth regression
>> Submitter       : Pavel Machek<pavel@ucw.cz>
>> Date            : 2010-08-21 15:24 (44 days old)
>> Message-ID      :<20100821152445.GA1536@ucw.cz>
>> References      : http://marc.info/?l=linux-kernel&m=128240433828087&w=2
>
> I see no updates to this since August. There are boat load of PCMCIA
> changes recently, is bisect possible here?
>
>

yeah the first pcmcia bug I hit with this machine was back in may/june 
ended up doing a bisect for that(which was painful due to a slow 
machine)which ended up getting fixed.(never thought to check s2ram after 
that)

as for bluetooth the only issue I have with that is for my other 
machines which is hitting this: http://lkml.org/lkml/2010/9/1/61
(but have not had a chance to follow through due to doing the update 
broken web addresses in the kernel)

>> Regressions with patches
>> ------------------------
>
>
>> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=17722
>> Subject         : 2.6.36-rc3: WARNING: at net/mac80211/scan.c:269 ieee80211_scan_completed
>> Submitter       : Thomas Meyer<thomas@m3y3r.de>
>> Date            : 2010-08-31 20:14 (34 days old)
>> Message-ID      :<201008312214.52473.thomas@m3y3r.de>
>> References      : http://marc.info/?l=linux-kernel&m=128328580504227&w=2
>>                   http://www.spinics.net/lists/netdev/msg140769.html
>> Handled-By      : Florian Mickler<florian@mickler.org>
>> Patch           : https://bugzilla.kernel.org/attachment.cgi?id=31671
>
> This seems fixed.
>
>    Luis
>

alright if this is with the latest wireless tree, then I can throw that 
in to verify that it is indeed fixed..

Justin P. Mattock

^ permalink raw reply

* [PATCH 3/3] Modify dec_read_req() to get PDU length as parameter
From: Anderson Lizardo @ 2010-10-04 18:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1286218482-972-1-git-send-email-anderson.lizardo@openbossa.org>

This is consistent with other att.h functions, and allows length checks.
---
 attrib/att.c        |    5 ++++-
 attrib/att.h        |    2 +-
 src/attrib-server.c |    2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index a795ddd..fc87d98 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -379,7 +379,7 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
 	return min_len;
 }
 
-uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
+uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
 {
 	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
 
@@ -389,6 +389,9 @@ uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 	if (handle == NULL)
 		return 0;
 
+	if (len < min_len)
+		return 0;
+
 	if (pdu[0] != ATT_OP_READ_REQ)
 		return 0;
 
diff --git a/attrib/att.h b/attrib/att.h
index 69071b4..89fc52d 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -179,7 +179,7 @@ uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
 						uint8_t *value, int *vlen);
 struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, int len);
 uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
-uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle);
+uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
 uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
 uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
 uint16_t enc_error_resp(uint8_t opcode, uint16_t handle, uint8_t status,
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 475b68b..b45f300 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -478,7 +478,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 		length = read_by_type(start, end, &uuid, opdu, channel->mtu);
 		break;
 	case ATT_OP_READ_REQ:
-		length = dec_read_req(ipdu, &start);
+		length = dec_read_req(ipdu, len, &start);
 		if (length == 0) {
 			status = ATT_ECODE_INVALID_PDU;
 			goto done;
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] Replace hardcoded minimum length values with constants
From: Anderson Lizardo @ 2010-10-04 18:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1286218482-972-1-git-send-email-anderson.lizardo@openbossa.org>

---
 attrib/att.c |  126 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 82 insertions(+), 44 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index 21659f0..a795ddd 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -90,6 +90,7 @@ void att_data_list_free(struct att_data_list *list)
 uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end);
 	uint16_t length;
 
 	if (!uuid)
@@ -102,7 +103,7 @@ uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		return 0;
 
-	if (len < 5 + length)
+	if (len < min_len + length)
 		return 0;
 
 	pdu[0] = ATT_OP_READ_BY_GROUP_REQ;
@@ -114,12 +115,14 @@ uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		memcpy(&pdu[5], &uuid->value.uuid128, length);
 
-	return 5 + length;
+	return min_len + length;
 }
 
 uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
 						uint16_t *end, uuid_t *uuid)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+
 	if (pdu == NULL)
 		return 0;
 
@@ -129,12 +132,12 @@ uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (pdu[0] != ATT_OP_READ_BY_GROUP_REQ)
 		return 0;
 
-	if (len < 7)
+	if (len < min_len + 2)
 		return 0;
 
 	*start = att_get_u16(&pdu[1]);
 	*end = att_get_u16(&pdu[3]);
-	if (len == 7)
+	if (len == min_len + 2)
 		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
@@ -203,6 +206,7 @@ uint16_t enc_find_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end);
 	uint16_t length;
 
 	if (!uuid)
@@ -215,7 +219,7 @@ uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		return 0;
 
-	if (len < 5 + length)
+	if (len < min_len + length)
 		return 0;
 
 	pdu[0] = ATT_OP_READ_BY_TYPE_REQ;
@@ -227,19 +231,21 @@ uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
 	else
 		memcpy(&pdu[5], &uuid->value.uuid128, length);
 
-	return 5 + length;
+	return min_len + length;
 }
 
 uint16_t dec_read_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
 						uint16_t *end, uuid_t *uuid)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (start == NULL || end == NULL || uuid == NULL)
 		return 0;
 
-	if (len < 7)
+	if (len < min_len + 2)
 		return 0;
 
 	if (pdu[0] != ATT_OP_READ_BY_TYPE_REQ)
@@ -248,7 +254,7 @@ uint16_t dec_read_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
 	*start = att_get_u16(&pdu[1]);
 	*end = att_get_u16(&pdu[3]);
 
-	if (len == 7)
+	if (len == min_len + 2)
 		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
@@ -311,64 +317,72 @@ struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, int len)
 uint16_t enc_write_cmd(uint16_t handle, const uint8_t *value, int vlen,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
-	if (vlen > len - 3)
-		vlen = len - 3;
+	if (vlen > len - min_len)
+		vlen = len - min_len;
 
 	pdu[0] = ATT_OP_WRITE_CMD;
 	att_put_u16(handle, &pdu[1]);
 
 	if (vlen > 0) {
-		memcpy(pdu + 3, value, vlen);
-		return 3 + vlen;
+		memcpy(&pdu[3], value, vlen);
+		return min_len + vlen;
 	}
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
 						uint8_t *value, int *vlen)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (value == NULL || vlen == NULL || handle == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	if (pdu[0] != ATT_OP_WRITE_CMD)
 		return 0;
 
 	*handle = att_get_u16(&pdu[1]);
-	memcpy(value, pdu + 3, len - 3);
-	*vlen = len - 3;
+	memcpy(value, pdu + min_len, len - min_len);
+	*vlen = len - min_len;
 
 	return len;
 }
 
 uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_READ_REQ;
 	att_put_u16(handle, &pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
+
 	if (pdu == NULL)
 		return 0;
 
@@ -380,7 +394,7 @@ uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 
 	*handle = att_get_u16(&pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len)
@@ -422,9 +436,11 @@ uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen)
 uint16_t enc_error_resp(uint8_t opcode, uint16_t handle, uint8_t status,
 							uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(opcode) +
+						sizeof(handle) + sizeof(status);
 	uint16_t u16;
 
-	if (len < 5)
+	if (len < min_len)
 		return 0;
 
 	u16 = htobs(handle);
@@ -433,31 +449,35 @@ uint16_t enc_error_resp(uint8_t opcode, uint16_t handle, uint8_t status,
 	memcpy(&pdu[2], &u16, sizeof(u16));
 	pdu[4] = status;
 
-	return 5;
+	return min_len;
 }
 
 uint16_t enc_find_info_req(uint16_t start, uint16_t end, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 5)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_FIND_INFO_REQ;
 	att_put_u16(start, &pdu[1]);
 	att_put_u16(end, &pdu[3]);
 
-	return 5;
+	return min_len;
 }
 
 uint16_t dec_find_info_req(const uint8_t *pdu, int len, uint16_t *start,
 								uint16_t *end)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 5)
+	if (len < min_len)
 		return 0;
 
 	if (start == NULL || end == NULL)
@@ -469,7 +489,7 @@ uint16_t dec_find_info_req(const uint8_t *pdu, int len, uint16_t *start,
 	*start = att_get_u16(&pdu[1]);
 	*end = att_get_u16(&pdu[3]);
 
-	return 5;
+	return min_len;
 }
 
 uint16_t enc_find_info_resp(uint8_t format, struct att_data_list *list,
@@ -520,10 +540,11 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
 
 	list = malloc(sizeof(struct att_data_list));
 
+	list->len = sizeof(pdu[0]) + sizeof(*format);
 	if (*format == 0x01)
-		list->len = 4;
+		list->len += 2;
 	else if (*format == 0x02)
-		list->len = 18;
+		list->len += 16;
 
 	list->num = (len - 2) / list->len;
 	list->data = malloc(sizeof(uint8_t *) * list->num);
@@ -541,36 +562,42 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
 
 uint16_t enc_notification(struct attribute *a, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < (a->len + 3))
+	if (len < (a->len + min_len))
 		return 0;
 
 	pdu[0] = ATT_OP_HANDLE_NOTIFY;
 	att_put_u16(a->handle, &pdu[1]);
 	memcpy(&pdu[3], a->data, a->len);
 
-	return a->len + 3;
+	return a->len + min_len;
 }
 
 uint16_t enc_indication(struct attribute *a, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < (a->len + 3))
+	if (len < (a->len + min_len))
 		return 0;
 
 	pdu[0] = ATT_OP_HANDLE_IND;
 	att_put_u16(a->handle, &pdu[1]);
 	memcpy(&pdu[3], a->data, a->len);
 
-	return a->len + 3;
+	return a->len + min_len;
 }
 
 struct attribute *dec_indication(const uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
+
 	struct attribute *a;
 
 	if (pdu == NULL)
@@ -579,11 +606,14 @@ struct attribute *dec_indication(const uint8_t *pdu, int len)
 	if (pdu[0] != ATT_OP_HANDLE_IND)
 		return NULL;
 
-	a = malloc(sizeof(struct attribute) + len - 3);
+	if (len < min_len)
+		return NULL;
+
+	a = malloc(sizeof(struct attribute) + len - min_len);
 	if (a == NULL)
 		return NULL;
 
-	a->len = len - 3;
+	a->len = len - min_len;
 
 	a->handle = att_get_u16(&pdu[1]);
 	memcpy(a->data, &pdu[3], a->len);
@@ -596,37 +626,41 @@ uint16_t enc_confirmation(uint8_t *pdu, int len)
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 1)
+	if (len < sizeof(pdu[0]))
 		return 0;
 
 	pdu[0] = ATT_OP_HANDLE_CNF;
 
-	return 1;
+	return sizeof(pdu[0]);
 }
 
 uint16_t enc_mtu_req(uint16_t mtu, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(mtu);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_MTU_REQ;
 	att_put_u16(mtu, &pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*mtu);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (mtu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	if (pdu[0] != ATT_OP_MTU_REQ)
@@ -634,32 +668,36 @@ uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu)
 
 	*mtu = att_get_u16(&pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t enc_mtu_resp(uint16_t mtu, uint8_t *pdu, int len)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(mtu);
+
 	if (pdu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	pdu[0] = ATT_OP_MTU_RESP;
 	att_put_u16(mtu, &pdu[1]);
 
-	return 3;
+	return min_len;
 }
 
 uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
 {
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*mtu);
+
 	if (pdu == NULL)
 		return 0;
 
 	if (mtu == NULL)
 		return 0;
 
-	if (len < 3)
+	if (len < min_len)
 		return 0;
 
 	if (pdu[0] != ATT_OP_MTU_RESP)
@@ -667,5 +705,5 @@ uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
 
 	*mtu = att_get_u16(&pdu[1]);
 
-	return 3;
+	return min_len;
 }
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/3] Remove typecast from att_get_u16() calls
From: Anderson Lizardo @ 2010-10-04 18:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

att_get_u16() already does a cast of the void* parameter to uint16_t*.
---
 attrib/att.c      |   26 +++++++++++++-------------
 attrib/client.c   |   19 +++++++++----------
 attrib/gatttool.c |   21 +++++++++------------
 3 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index 6c697f8..21659f0 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -132,10 +132,10 @@ uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (len < 7)
 		return 0;
 
-	*start = att_get_u16((uint16_t *) &pdu[1]);
-	*end = att_get_u16((uint16_t *) &pdu[3]);
+	*start = att_get_u16(&pdu[1]);
+	*end = att_get_u16(&pdu[3]);
 	if (len == 7)
-		sdp_uuid16_create(uuid, att_get_u16((uint16_t *) &pdu[5]));
+		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
 
@@ -245,11 +245,11 @@ uint16_t dec_read_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (pdu[0] != ATT_OP_READ_BY_TYPE_REQ)
 		return 0;
 
-	*start = att_get_u16((uint16_t *) &pdu[1]);
-	*end = att_get_u16((uint16_t *) &pdu[3]);
+	*start = att_get_u16(&pdu[1]);
+	*end = att_get_u16(&pdu[3]);
 
 	if (len == 7)
-		sdp_uuid16_create(uuid, att_get_u16((uint16_t *) &pdu[5]));
+		sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
 	else
 		sdp_uuid128_create(uuid, &pdu[5]);
 
@@ -346,7 +346,7 @@ uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
 	if (pdu[0] != ATT_OP_WRITE_CMD)
 		return 0;
 
-	*handle = att_get_u16((uint16_t *) &pdu[1]);
+	*handle = att_get_u16(&pdu[1]);
 	memcpy(value, pdu + 3, len - 3);
 	*vlen = len - 3;
 
@@ -378,7 +378,7 @@ uint16_t dec_read_req(const uint8_t *pdu, uint16_t *handle)
 	if (pdu[0] != ATT_OP_READ_REQ)
 		return 0;
 
-	*handle = att_get_u16((uint16_t *) &pdu[1]);
+	*handle = att_get_u16(&pdu[1]);
 
 	return 3;
 }
@@ -466,8 +466,8 @@ uint16_t dec_find_info_req(const uint8_t *pdu, int len, uint16_t *start,
 	if (pdu[0] != ATT_OP_FIND_INFO_REQ)
 		return 0;
 
-	*start = att_get_u16((uint16_t *) &pdu[1]);
-	*end = att_get_u16((uint16_t *) &pdu[3]);
+	*start = att_get_u16(&pdu[1]);
+	*end = att_get_u16(&pdu[3]);
 
 	return 5;
 }
@@ -585,7 +585,7 @@ struct attribute *dec_indication(const uint8_t *pdu, int len)
 
 	a->len = len - 3;
 
-	a->handle = att_get_u16((uint16_t *) &pdu[1]);
+	a->handle = att_get_u16(&pdu[1]);
 	memcpy(a->data, &pdu[3], a->len);
 
 	return a;
@@ -632,7 +632,7 @@ uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu)
 	if (pdu[0] != ATT_OP_MTU_REQ)
 		return 0;
 
-	*mtu = att_get_u16((uint16_t *) &pdu[1]);
+	*mtu = att_get_u16(&pdu[1]);
 
 	return 3;
 }
@@ -665,7 +665,7 @@ uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
 	if (pdu[0] != ATT_OP_MTU_RESP)
 		return 0;
 
-	*mtu = att_get_u16((uint16_t *) &pdu[1]);
+	*mtu = att_get_u16(&pdu[1]);
 
 	return 3;
 }
diff --git a/attrib/client.c b/attrib/client.c
index db84b78..cd720e6 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -286,7 +286,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len,
 	struct primary *prim;
 	GSList *lprim, *lchr;
 	uint8_t opdu[ATT_MAX_MTU];
-	guint handle = att_get_u16((uint16_t *) &pdu[1]);
+	guint handle = att_get_u16(&pdu[1]);
 	uint16_t olen;
 
 	for (lprim = gatt->primary, prim = NULL, chr = NULL; lprim;
@@ -872,11 +872,10 @@ static void descriptor_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		uint8_t *info = list->data[i];
 		struct query_data *qfmt;
 
-		handle = att_get_u16((uint16_t *) info);
+		handle = att_get_u16(info);
 
 		if (format == 0x01) {
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&info[2]));
+			sdp_uuid16_create(&uuid, att_get_u16(&info[2]));
 		} else {
 			/* Currently, only "user description" and "presentation
 			 * format" descriptors are used, and both have 16-bit
@@ -963,17 +962,17 @@ static void char_discovered_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		chr = g_new0(struct characteristic, 1);
 		chr->prim = prim;
 		chr->perm = decl[2];
-		chr->handle = att_get_u16((uint16_t *) &decl[3]);
+		chr->handle = att_get_u16(&decl[3]);
 		chr->path = g_strdup_printf("%s/characteristic%04x",
 						prim->path, chr->handle);
 		if (list->len == 7) {
 			sdp_uuid16_create(&chr->type,
-					att_get_u16((uint16_t *) &decl[5]));
+					att_get_u16(&decl[5]));
 		} else
 			sdp_uuid128_create(&chr->type, &decl[5]);
 
 		if (previous_end) {
-			*previous_end = att_get_u16((uint16_t *) decl);
+			*previous_end = att_get_u16(decl);
 		}
 
 		last = chr->handle;
@@ -1270,8 +1269,8 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 
 		/* Each element contains: attribute handle, end group handle
 		 * and attribute value */
-		start = att_get_u16((uint16_t *) info);
-		end = att_get_u16((uint16_t *) &info[2]);
+		start = att_get_u16(info);
+		end = att_get_u16(&info[2]);
 
 		prim = g_new0(struct primary, 1);
 		prim->gatt = gatt;
@@ -1280,7 +1279,7 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 
 		if (list->len == 6) {
 			sdp_uuid16_create(&prim->uuid,
-					att_get_u16((uint16_t *) &info[4]));
+					att_get_u16(&info[4]));
 
 		} else if (list->len == 20) {
 			/* FIXME: endianness */
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index 18f32a2..4a66340 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -167,14 +167,13 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		/* Each element contains: attribute handle, end group handle
 		 * and attribute value */
 		length = list->len - 2 * sizeof(uint16_t);
-		start = att_get_u16((uint16_t *) value);
-		end = att_get_u16((uint16_t *) &value[2]);
+		start = att_get_u16(value);
+		end = att_get_u16(&value[2]);
 
 		g_print("attr handle = 0x%04x, end grp handle = 0x%04x, ",
 								start, end);
 		if (length == 2)
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&value[4]));
+			sdp_uuid16_create(&uuid, att_get_u16(&value[4]));
 		else
 			sdp_uuid128_create(&uuid, value + 4);
 
@@ -204,7 +203,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
 	uint8_t opdu[ATT_MAX_MTU];
 	uint16_t handle, i, olen = 0;
 
-	handle = att_get_u16((uint16_t *) &pdu[1]);
+	handle = att_get_u16(&pdu[1]);
 
 	switch (pdu[0]) {
 	case ATT_OP_HANDLE_NOTIFY:
@@ -279,15 +278,14 @@ static void char_discovered_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		char uuidstr[MAX_LEN_UUID_STR];
 		uuid_t uuid;
 
-		last = att_get_u16((uint16_t *) value);
+		last = att_get_u16(value);
 
 		g_print("handle = 0x%04x, char properties = 0x%02x, "
 			"char value handle = 0x%04x, ", last, value[2],
-			att_get_u16((uint16_t *) &value[3]));
+			att_get_u16(&value[3]));
 
 		if (list->len == 7)
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&value[5]));
+			sdp_uuid16_create(&uuid, att_get_u16(&value[5]));
 		else
 			sdp_uuid128_create(&uuid, value + 5);
 
@@ -447,11 +445,10 @@ static void char_desc_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		uuid_t uuid;
 
 		value = list->data[i];
-		handle = att_get_u16((uint16_t *) value);
+		handle = att_get_u16(value);
 
 		if (format == 0x01)
-			sdp_uuid16_create(&uuid, att_get_u16((uint16_t *)
-								&value[2]));
+			sdp_uuid16_create(&uuid, att_get_u16(&value[2]));
 		else
 			sdp_uuid128_create(&uuid, &value[2]);
 
-- 
1.7.0.4


^ permalink raw reply related

* Re: 2.6.36-rc6-git2: Reported regressions from 2.6.35
From: Dominik Brodowski @ 2010-10-04 18:44 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Rafael J. Wysocki, linux-wireless, linux-bluetooth, justinmattock,
	Bob Copeland, Pavel Machek
In-Reply-To: <AANLkTimUdaUkpXmNt5mPkCdWmHvBbXWA6smAP4U80yc4@mail.gmail.com>

Hey,

On Mon, Oct 04, 2010 at 11:35:25AM -0700, Luis R. Rodriguez wrote:
> > Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=17061
> > Subject         : 2.6.36-rc1 on zaurus: bluetooth regression
> > Submitter       : Pavel Machek <pavel@ucw.cz>
> > Date            : 2010-08-21 15:24 (44 days old)
> > Message-ID      : <20100821152445.GA1536@ucw.cz>
> > References      : http://marc.info/?l=linux-kernel&m=128240433828087&w=2
> 
> I see no updates to this since August. There are boat load of PCMCIA
> changes recently, is bisect possible here?

Pavel already verified the PCMCIA-bigdiff since 2.6.35 does work, so it is
_not_ a PCMCIA issue.

http://www.mail-archive.com/linux-pcmcia@lists.infradead.org/msg03460.html

Best,
	Dominik

^ permalink raw reply

* Re: 2.6.36-rc6-git2: Reported regressions from 2.6.35
From: Luis R. Rodriguez @ 2010-10-04 18:35 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-wireless, linux-bluetooth
  Cc: justinmattock, Bob Copeland, Pavel Machek, linux
In-Reply-To: <-jYMINoCtaK.A.F5.8CPqMB@chimera>

Below are the 802.11 and Bluetooth ones:

On Sun, Oct 3, 2010 at 2:15 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Unresolved regressions
> ----------------------

> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=19392
> Subject         : WARNING: at drivers/net/wireless/ath/ath5k/base.c:3475 ath5k_bss_info_changed+0x44/0x168 [ath5k]()
> Submitter       : Justin Mattock <justinmattock@gmail.com>
> Date            : 2010-09-28 22:30 (6 days old)
> Message-ID      : <AANLkTim5WCGKPvEkOkO_YnMF9pg8mvLfQoFBNUFpfa_k@mail.gmail.com>
> References      : http://marc.info/?l=linux-kernel&m=128571307018635&w=2

WTF -- this ended up in a bisect pointing to some PCMCIA patch! Justin:

  * does this happen with wireless-testing.git ?
  * Did you do the bisect on the entire kernel? What git tree are you
using to bisect?

Did you simply bring up the interface and then suspsend? Did you not
add a new interface in between this? ath5k's add_interace has:

        if (sc->vif) {
                ret = 0;
                goto end;
        }

But why does it just allow this to go through without complaining if
multiple vifs are not supported?


>
> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=17061
> Subject         : 2.6.36-rc1 on zaurus: bluetooth regression
> Submitter       : Pavel Machek <pavel@ucw.cz>
> Date            : 2010-08-21 15:24 (44 days old)
> Message-ID      : <20100821152445.GA1536@ucw.cz>
> References      : http://marc.info/?l=linux-kernel&m=128240433828087&w=2

I see no updates to this since August. There are boat load of PCMCIA
changes recently, is bisect possible here?


> Regressions with patches
> ------------------------


> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=17722
> Subject         : 2.6.36-rc3: WARNING: at net/mac80211/scan.c:269 ieee80211_scan_completed
> Submitter       : Thomas Meyer <thomas@m3y3r.de>
> Date            : 2010-08-31 20:14 (34 days old)
> Message-ID      : <201008312214.52473.thomas@m3y3r.de>
> References      : http://marc.info/?l=linux-kernel&m=128328580504227&w=2
>                  http://www.spinics.net/lists/netdev/msg140769.html
> Handled-By      : Florian Mickler <florian@mickler.org>
> Patch           : https://bugzilla.kernel.org/attachment.cgi?id=31671

This seems fixed.

  Luis

^ permalink raw reply

* Re: 2.6.36-rc6-git2: Reported regressions 2.6.34 -> 2.6.35
From: Luis R. Rodriguez @ 2010-10-04 18:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, linux-wireless, linux-bluetooth
  Cc: Guy, Wey-Yi W, dtonator, florian, oliver
In-Reply-To: <OjxVjYbf_EJ.A.ReC.pTPqMB@chimera>

Here are the 802.11 and Bluetooth ones:

On Sun, Oct 3, 2010 at 2:36 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:

> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=16691
> Subject         : IPW5100: iwlagn broken with 2.6.34.x to 2.6.35.2 update
> Submitter       : Can Celasun <dcelasun@gmail.com>
> Date            : 2010-08-21 08:28 (44 days old)

This is awaiting some new uCode code drop by Intel.

> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=16458
> Subject         : Bluetooth disabled after resume
> Submitter       : AttilaN <attila123456@gmail.com>
> Date            : 2010-07-25 09:33 (71 days old)

No followup on this one.

> Regressions with patches
> ------------------------
>
> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=16462
> Subject         : unable to connect to hidden SSID AP on legal channel 13
> Submitter       : Daniel J Blueman <daniel.blueman@gmail.com>
> Date            : 2010-07-25 17:06 (71 days old)
> Handled-By      : Johannes Berg <johannes.berg@intel.com>
> Patch           : https://bugzilla.kernel.org/attachment.cgi?id=31862

Fixed by Johannes, this was an Intel specific issue. Reviewed the
possibility of this also affecting mac80211 but since we lift the
passive scan on a beacon for the AP this should not affect mac80211 in
the future.

  Luis

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: hci open callback for hci UART transport driver
From: Suraj Sumangala @ 2010-10-04 17:29 UTC (permalink / raw)
  To: Suraj Sumangala; +Cc: linux-bluetooth@vger.kernel.org, Jothikumar Mothilal
In-Reply-To: <4CA44AF2.7050908@Atheros.com>

Hi,

On 9/30/2010 2:01 PM, Suraj Sumangala wrote:
> *ping*
>
> On 9/21/2010 7:03 PM, Suraj Sumangala wrote:
>> This patch provides option for hci transport driver protocol implementation
>> to have a callback for hci open.
>>
>> Signed-off-by: Suraj Sumangala<suraj@atheros.com>
>> ---
>>    drivers/bluetooth/hci_ldisc.c |    5 ++++-
>>    drivers/bluetooth/hci_uart.h  |    1 +
>>    2 files changed, 5 insertions(+), 1 deletions(-)
>>
>
> Regards
> Suraj

Is there any comments on this patch?

Regards
Suraj

^ permalink raw reply


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