Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] Initial attribute permission implementation
From: Anderson Lizardo @ 2010-12-02 13:33 UTC (permalink / raw)
  To: Anderson Lizardo, linux-bluetooth, Bruna Moreira
In-Reply-To: <20101202101031.GB18808@jh-x301>

On Thu, Dec 2, 2010 at 6:10 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi,
>
> On Wed, Dec 01, 2010, Anderson Lizardo wrote:
>> +     attrib_db_add(0x0685, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
>> +                                                                     len);
> <snip>
>> +/* Access permissions */
>> +#define ATT_READ     1
>> +#define ATT_WRITE    2
>> +/* Authentication/Authorization permissions */
>> +#define ATT_NONE             0
>> +#define ATT_AUTHENTICATION   2
>> +#define ATT_AUTHORIZATION    4
>> +/* Build bit mask for permission checks */
>> +#define ATT_ACCESS(x, y)     (((x) << y) | (x))
>
> The last macro doesn't make any sense to me. Based the core spec it
> seems that the values for the different permission types are
> implementation specific and not exposed publicly (vol 2, part G, 2.5.1:
> "Attribute Permissions is part of the Attribute that cannot be read from
> or written to using the Attribute Protocol").

Correct. The permission implementation is internal.

> Why not make sure that
> they are all orthogonal from the start instead of ssigning the same
> value for authentication and writability like you do now. I.e. instead
> you could have:
>
> none            0
> read            1
> write           2
> authentication  4
> authorization   8

This was the first attempt (as it seemed obvious). But see vol 3, part
F, section 4:

"For example, an attribute value may be allowed to be read by any device, but
only written by an authenticated device. An implementation should take this
into account, and not assume that just because it can read an attribute’s value,
it will also be able to write the value. [...]"

This makes it clear that the authentication/authorization permissions
are not orthogonal. Further on it is confirmed by the attribute
permissions of "Client Characteristic Configuration" (vol 3, part G,
3.3.3.4):

* Readable with no authentication or authorization.
* Writable with authentication and authorization defined by a higher
layer specification or is implementation specific.

The scheme implemented in this patch set is just a compacted version
of unix permissions using a 2-bit grouping (not sure if it will look
nice to read):

5 4 3 2 1 0  bit
| | | | | \_ Read
| | | | \___ Write
| | | \_____ Authenticated read
| | \_______ Authenticated write
| \_________ Authorized read
\___________ Authorized write

With an addition that, if the authenticated/authorized read/write bits
are set, the "none" read/write bits are set as well (I think the
reason is obvious).

> Then you can just or together whatever you want without having any
> complex macros for producing the final permissions value. Alternatively
> you could also have a simple enum and use set_bit/test_bit marcos (see
> lib/hci_lib.h).

If you have ideas to reuse the hci_lib.h macros and keep the semantics
defined in the spec at the same time, let us know.

>
> Johan
>

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

^ permalink raw reply

* Re: [PATCH 1/3] Initial attribute permission implementation
From: Johan Hedberg @ 2010-12-02 10:10 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1291219986-29913-2-git-send-email-anderson.lizardo@openbossa.org>

Hi,

On Wed, Dec 01, 2010, Anderson Lizardo wrote:
> +	attrib_db_add(0x0685, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
> +									len);
<snip>
> +/* Access permissions */
> +#define ATT_READ	1
> +#define ATT_WRITE	2
> +/* Authentication/Authorization permissions */
> +#define ATT_NONE		0
> +#define ATT_AUTHENTICATION	2
> +#define ATT_AUTHORIZATION	4
> +/* Build bit mask for permission checks */
> +#define ATT_ACCESS(x, y)	(((x) << y) | (x))

The last macro doesn't make any sense to me. Based the core spec it
seems that the values for the different permission types are
implementation specific and not exposed publicly (vol 2, part G, 2.5.1:
"Attribute Permissions is part of the Attribute that cannot be read from
or written to using the Attribute Protocol"). Why not make sure that
they are all orthogonal from the start instead of ssigning the same
value for authentication and writability like you do now. I.e. instead
you could have:

none		0
read		1
write		2
authentication	4
authorization	8

Then you can just or together whatever you want without having any
complex macros for producing the final permissions value. Alternatively
you could also have a simple enum and use set_bit/test_bit marcos (see
lib/hci_lib.h).

Johan

^ permalink raw reply

* Re: [PATCH] Fix deinitializing telephony backend when it wasn't initialized
From: Johan Hedberg @ 2010-12-02  9:04 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1291279564-16615-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Thu, Dec 02, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> 
> Telephony driver is now deinitialized on headset adapter driver remove so
> telephony_exit should not be called on plugin exit anymore.
> ---
>  audio/manager.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* [PATCH] Fix deinitializing telephony backend when it wasn't initialized
From: Luiz Augusto von Dentz @ 2010-12-02  8:46 UTC (permalink / raw)
  To: linux-bluetooth

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

Telephony driver is now deinitialized on headset adapter driver remove so
telephony_exit should not be called on plugin exit anymore.
---
 audio/manager.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index 12671c7..035656b 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -1264,10 +1264,8 @@ void audio_manager_exit(void)
 	if (enabled.media)
 		btd_unregister_adapter_driver(&media_server_driver);
 
-	if (enabled.headset) {
+	if (enabled.headset)
 		btd_unregister_adapter_driver(&headset_server_driver);
-		telephony_exit();
-	}
 
 	if (enabled.gateway)
 		btd_unregister_adapter_driver(&gateway_server_driver);
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Marcel Holtmann @ 2010-12-02  7:49 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: Gustavo F. Padovan, Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <20101202071224.GA15525@jh-x301>

Hi Johan,

> > > -static int disable_esco = 0;
> > > +static int disable_esco;
> > 
> > I don't think this change is right. Can we be sure that disable_esco
> > will be 0 by default?
> 
> AFAIK we can since static variables are initialized to 0 by default.
> However, I've understood that it's good style to have this
> initialization explicit in the code so imho the code should be left as
> it is.

personally I prefer to initialize the variables to an initial value to
make it clear what their default is.

That said, this comes with a cost in case of a 0 or NULL initialization
since the compiler has to store extra code to do so. Not doing an
explicit initialization saves code size in this case.

The general kernel recommendation is to not initialize and I am fine
with following that one.

Regards

Marcel



^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Johan Hedberg @ 2010-12-02  7:14 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: Gustavo F. Padovan, Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <AANLkTi==oXHyrtkjRD_mbmRFsDnTH7+S_JOFHRemXJLG@mail.gmail.com>

Hi Anderson,

On Wed, Dec 01, 2010, Anderson Lizardo wrote:
> On Wed, Dec 1, 2010 at 5:20 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> >> -static int disable_esco = 0;
> >> +static int disable_esco;
> >
> > I don't think this change is right. Can we be sure that disable_esco
> > will be 0 by default?
> 
> I think Andrei's patch is ok. IIRC kernel zeroes BSS on init, that's
> why checkpatch.pl complains when you initialize it explicitely.

That's interesting. It'd be nice to have some comment from Marcel on
this since at least on the user space side it's considered a good thing
to explicitly initialize static variables to zero.

Johan

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Johan Hedberg @ 2010-12-02  7:12 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <20101201212034.GE16125@vigoh>

Hi Gustavo,

On Wed, Dec 01, 2010, Gustavo F. Padovan wrote:
> > -static int disable_esco = 0;
> > +static int disable_esco;
> 
> I don't think this change is right. Can we be sure that disable_esco
> will be 0 by default?

AFAIK we can since static variables are initialized to 0 by default.
However, I've understood that it's good style to have this
initialization explicit in the code so imho the code should be left as
it is.

Johan

^ permalink raw reply

* Re: [PATCH v7] Bluetooth: btwilink driver
From: Pavan Savoy @ 2010-12-02  0:48 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTiktDkjFH=aGbu3f+mkVUgmFDwB6xLW3e1VNXKtz@mail.gmail.com>

Gustavo,

On Tue, Nov 30, 2010 at 9:30 PM, Pavan Savoy <pavan_savoy@ti.com> wrote:
>>> Marcel, Gustavo,
>>> comments attended to from v5 and v6,

There was 1 solution which was proposed for this,
See in tty_receive/st_int_recv() - we pretty much want only the
packet_id, header length and
the offset of the payload in the header structure.
this is to pack together the data coming in single frame or sets of
multiple frames...

so if bt driver can do something like,
st_register(acl_proto) -> where ACL_PKT=3D0x2, acl_hdr.len and
acl_hdr.payload_offset is sent to ST
st_register(sco_proto) -> where SCO_PKT=3D0x3, sco_hdr.len and
sco_hdr.payload_offset is sent to ST
st_register(evt_proto) -> where EVT_PKT=3D0x2, sco_hdr.len and
evt_hdr.payload_offset is sent to ST

and all 3 map to same st_recv() functions from ST, then
the ST driver as you suggested can be generic enough not to include
any net/bluetooth/ stuff,
and also can be ignorant about any bluetooth, fm or gps stuff in general...

So please suggest if this seems a valid solution, we had previously
declined it because it made bt_driver
sort of register to ST 3 times instead of 1 time like now....

Thanks,
Pavan

>>> 1. Inside ti_st_open, I previously only checked for EINPROGRESS & EPERM=
,
>>> Now I handle for EINPROGRESS - which is not really an error and
>>> return during all other error cases.
>>>
>>> 2. _write is still a function pointer and not an exported function, I
>>> need to change the underlying driver's code for this.
>>> However, previous lkml comments on the underlying driver's code
>>> suggested it to be kept as a function pointer and not EXPORT.
>>> Gustavo, Marcel - Please comment on this.
>>> Is this absolutely required? If so why?
>>>
>>> 3. test_and_set_bit of HCI_RUNNING is done at beginning of
>>> ti_st_open, and did not see issues during firmware download.
>>> However ideally I would still like to set HCI_RUNNING once the firmware
>>> download is done, because I don't want to allow a _send_frame during
>>> firmware download - Marcel, Gustavo - Please comment.
>>>
>>> 4. test_and_clear of HCI_RUNNING now done @ beginning of close.
>>>
>>> 5. EAGAIN on failure of st_write is to suggest to try and write again.
>>> I have never this happen - However only if UART goes bad this case may
>>> occur.
>>>
>>> 6. ti_st_tx_complete is very similar to hci_ldisc's tx_complete - in
>>> fact the code is pretty much borrowed from there.
>>> Marcel, Gustavo - Please suggest where should it be done? If not here.
>>>
>>> 7. comments cleaned-up + hst memory leak fixed when hci_alloc_dev fails=
.
>>>
>>> 8. platform_driver registration inside module_init now is similar to
>>> other drivers.
>>>
>>> 9. Dan Carpenter's comments on leaking hst memory on failed
>>> hci_register_dev and empty space after quotes in debug statements
>>> fixed.
>>>
>>> Thanks for the comments...
>>> Sorry, for previously not being very clear on which comments were
>>> handled and which were not.
>>>
>>> -- patch description --
>>>
>>> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
>>> Texas Instrument's WiLink chipsets combine wireless technologies
>>> like BT, FM, GPS and WLAN onto a single chip.
>>>
>>> This Bluetooth driver works on top of the TI_ST shared transport
>>> line discipline driver which also allows other drivers like
>>> FM V4L2 and GPS character driver to make use of the same UART interface=
.
>>>
>>> Kconfig and Makefile modifications to enable the Bluetooth
>>> driver for Texas Instrument's WiLink 7 chipset.
>>>
>>> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
>>> ---
>>> =C2=A0drivers/bluetooth/Kconfig =C2=A0 =C2=A0| =C2=A0 10 ++
>>> =C2=A0drivers/bluetooth/Makefile =C2=A0 | =C2=A0 =C2=A01 +
>>> =C2=A0drivers/bluetooth/btwilink.c | =C2=A0363 ++++++++++++++++++++++++=
++++++++++++++++++
>>> =C2=A03 files changed, 374 insertions(+), 0 deletions(-)
>>> =C2=A0create mode 100644 drivers/bluetooth/btwilink.c
>>
>> So as part of reviewing this I took a look at your underlying driver and
>> I didn't like what I saw there, you are handling Bluetooth stuff inside
>> the core driver and that is just wrong. You have a Bluetooth driver here
>> then you have to leave the Bluetooth data handling to the Bluetooth
>> driver and do not do that in the core.
>
> Thanks for reviewing this and the underlying driver.
> yes, we do have Bluetooth/FM/GPS handling inside the TI ST driver, on
> addition of further technologies
> we do plan to have them inside the ST driver too.
>
> The understanding of BT or FM or GPS is required for the ST driver
> because, the data coming from the chip
> can either be of these technologies, further-more the data might not
> come in a set.
> As in, an a2dp/ftp ACL frame might come in 2 frames instead of 1, and
> in other cases,
> there might be a HCI-EVENT + FM CH8 data in a single frame received by th=
e UART.
>
>> So I'm going to clear NACK this. Your TI ST driver architecture is
>> a mess, Ideally you should not have any #include <net/bluetoooth/...>
>> there. Implement a core driver that only gets the Shared Transport
>> data and pass it to the right driver without look into the data and
>> process it. Process the data is not the role of the core driver.
>
> So as I see it the tty_receive which translates to st_int_recv() in
> TI-ST is the area of concern for
> you ...
> So any suggestions as to how we can go about just abstracting the BT,
> FM and GPS data part to their individual drivers?
>
>> Please fix this and come back when you have a proper solution for your
>> driver.
>>
>> --
>> Gustavo F. Padovan
>> http://profusion.mobi
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetoot=
h" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* pull request: bluetooth-next-2.6 2010-11-01
From: Gustavo F. Padovan @ 2010-12-02  0:32 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, linux-bluetooth

Hi John,

Here are the first batch for 2.6.38, biggest change here is the move of
the Remote Name Request to the kernel to fix some synchronization
problems with other commands sent during the connection setup in
Bluetooth. The rest is just bug fixes and clean ups in the whole stack,
no big feature this time.

This pull request includes the patches that I sent in the pull request
to 2.6.37, but you should expect no problem when merging everything in
wireless-next-2.6.

Please pull or let me know anything wrong. Thanks.


The following changes since commit 61790c5f3c5f158821821a00797d94504531839f:

  iwlagn: fix microcode error on 4965 (2010-11-30 13:58:18 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next-2.6.git master

Anderson Lizardo (1):
      Bluetooth: Fix error handling for l2cap_init()

Andrei Emeltchenko (8):
      Bluetooth: Check sk is not owned before freeing l2cap_conn
      Bluetooth: timer check sk is not owned before freeing
      Bluetooth: do not use assignment in if condition
      Bluetooth: clean up sco code
      Bluetooth: clean up rfcomm code
      Bluetooth: clean up l2cap code
      Bluetooth: clean up hci code
      Bluetooth: clean up legal text

Bala Shanmugam (1):
      Bluetooth: Add new PID for Atheros 3011

Gustavo F. Padovan (4):
      Bluetooth: Fix not returning proper error in SCO
      Merge git://git.kernel.org/.../padovan/bluetooth-2.6 into test
      Bluetooth: Get rid of __l2cap_get_sock_by_psm()
      Bluetooth: Get rid of __rfcomm_get_sock_by_channel()

Johan Hedberg (3):
      Bluetooth: Simplify remote features callback function logic
      Bluetooth: Create a unified authentication request function
      Bluetooth: Automate remote name requests

Stefan Seyfried (1):
      Bluetooth: Fix log spamming in btusb due to autosuspend

Vasiliy Kulikov (3):
      Bluetooth: bnep: fix information leak to userland
      Bluetooth: cmtp: fix information leak to userland
      Bluetooth: hidp: fix information leak to userland

 drivers/bluetooth/ath3k.c        |    4 +
 drivers/bluetooth/btusb.c        |   12 ++-
 include/net/bluetooth/hci.h      |   16 ++--
 include/net/bluetooth/hci_core.h |   14 ++--
 include/net/bluetooth/l2cap.h    |   22 +++---
 include/net/bluetooth/rfcomm.h   |   18 ++--
 include/net/bluetooth/sco.h      |   20 ++--
 net/bluetooth/bnep/core.c        |    1 +
 net/bluetooth/cmtp/core.c        |    1 +
 net/bluetooth/hci_conn.c         |   23 +++--
 net/bluetooth/hci_core.c         |   66 +++++++++-----
 net/bluetooth/hci_event.c        |  177 +++++++++++++++++++++++++++-----------
 net/bluetooth/hci_sock.c         |   17 +++--
 net/bluetooth/hidp/core.c        |    2 +-
 net/bluetooth/l2cap.c            |   94 ++++++++++++++-------
 net/bluetooth/rfcomm/core.c      |    8 +-
 net/bluetooth/rfcomm/sock.c      |   24 ++---
 net/bluetooth/rfcomm/tty.c       |   28 ++++---
 net/bluetooth/sco.c              |   28 ++++---
 19 files changed, 363 insertions(+), 212 deletions(-)

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 4/5] Bluetooth: clean up hci code
From: Gustavo F. Padovan @ 2010-12-01 21:53 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-5-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:25 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not use assignment in IF condition, remove extra spaces,
> fixing typos, simplify code.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    4 +-
>  include/net/bluetooth/hci_core.h |   14 ++++----
>  net/bluetooth/hci_conn.c         |   23 ++++++++----
>  net/bluetooth/hci_core.c         |   66 ++++++++++++++++++++++++--------------
>  net/bluetooth/hci_event.c        |    8 +++--
>  net/bluetooth/hci_sock.c         |   17 ++++++---
>  6 files changed, 82 insertions(+), 50 deletions(-)

Applied.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 3/5] Bluetooth: clean up l2cap code
From: Gustavo F. Padovan @ 2010-12-01 21:53 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-4-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:24 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not initialize static vars to zero, macros with complex values
> shall be enclosed with (), remove unneeded braces.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/l2cap.h |   10 +++++-----
>  net/bluetooth/l2cap.c         |    7 +++----
>  2 files changed, 8 insertions(+), 9 deletions(-)

Applied, thanks for clean up this.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 2/5] Bluetooth: clean up rfcomm code
From: Gustavo F. Padovan @ 2010-12-01 21:52 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:23 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Remove extra spaces, assignments in if statement, zeroing static
> variables, extra braces. Fix includes.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/rfcomm.h |    4 ++--
>  net/bluetooth/rfcomm/core.c    |    8 ++++----
>  net/bluetooth/rfcomm/sock.c    |    5 +++--
>  net/bluetooth/rfcomm/tty.c     |   28 ++++++++++++++++------------
>  4 files changed, 25 insertions(+), 20 deletions(-)

Applied, thanks for clean up this.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Gustavo F. Padovan @ 2010-12-01 21:44 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <AANLkTi==oXHyrtkjRD_mbmRFsDnTH7+S_JOFHRemXJLG@mail.gmail.com>

Hi Anderson,

* Anderson Lizardo <anderson.lizardo@openbossa.org> [2010-12-01 17:31:15 -0400]:

> On Wed, Dec 1, 2010 at 5:20 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> >> -static int disable_esco = 0;
> >> +static int disable_esco;
> >
> > I don't think this change is right. Can we be sure that disable_esco
> > will be 0 by default?
> 
> I think Andrei's patch is ok. IIRC kernel zeroes BSS on init, that's
> why checkpatch.pl complains when you initialize it explicitely.

Great, I didn't know about that. Applied then. Thanks all!

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 5/5] Bluetooth: clean up legal text
From: Gustavo F. Padovan @ 2010-12-01 21:42 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-6-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:26 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Remove extra spaces from legal text so that legal stuff looks
> the same for all bluetooth code.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/hci.h    |   12 ++++++------
>  include/net/bluetooth/l2cap.h  |   12 ++++++------
>  include/net/bluetooth/rfcomm.h |   14 +++++++-------
>  include/net/bluetooth/sco.h    |   12 ++++++------
>  4 files changed, 25 insertions(+), 25 deletions(-)

This one is applied. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Anderson Lizardo @ 2010-12-01 21:31 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <20101201212034.GE16125@vigoh>

On Wed, Dec 1, 2010 at 5:20 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
>> -static int disable_esco = 0;
>> +static int disable_esco;
>
> I don't think this change is right. Can we be sure that disable_esco
> will be 0 by default?

I think Andrei's patch is ok. IIRC kernel zeroes BSS on init, that's
why checkpatch.pl complains when you initialize it explicitely.

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

^ permalink raw reply

* Re: [PATCHv2 3/5] Bluetooth: clean up l2cap code
From: Gustavo F. Padovan @ 2010-12-01 21:21 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-4-git-send-email-Andrei.Emeltchenko.news@gmail.com>

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:24 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not initialize static vars to zero, macros with complex values
> shall be enclosed with (), remove unneeded braces.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/l2cap.h |   10 +++++-----
>  net/bluetooth/l2cap.c         |    7 +++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c819c8b..217bb91 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -417,11 +417,11 @@ static inline int l2cap_tx_window_full(struct sock *sk)
>  	return sub == pi->remote_tx_win;
>  }
>  
> -#define __get_txseq(ctrl) ((ctrl) & L2CAP_CTRL_TXSEQ) >> 1
> -#define __get_reqseq(ctrl) ((ctrl) & L2CAP_CTRL_REQSEQ) >> 8
> -#define __is_iframe(ctrl) !((ctrl) & L2CAP_CTRL_FRAME_TYPE)
> -#define __is_sframe(ctrl) (ctrl) & L2CAP_CTRL_FRAME_TYPE
> -#define __is_sar_start(ctrl) ((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START
> +#define __get_txseq(ctrl)	(((ctrl) & L2CAP_CTRL_TXSEQ) >> 1)
> +#define __get_reqseq(ctrl)	(((ctrl) & L2CAP_CTRL_REQSEQ) >> 8)
> +#define __is_iframe(ctrl)	(!((ctrl) & L2CAP_CTRL_FRAME_TYPE))
> +#define __is_sframe(ctrl)	((ctrl) & L2CAP_CTRL_FRAME_TYPE)
> +#define __is_sar_start(ctrl)	(((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START)
>  
>  void l2cap_load(void);
>  
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 12b4aa2..d4b4fbd 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -57,7 +57,7 @@
>  
>  #define VERSION "2.15"
>  
> -static int disable_ertm = 0;
> +static int disable_ertm;

Same issue here.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Gustavo F. Padovan @ 2010-12-01 21:20 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:22 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not use assignments in IF condition, remove extra spaces
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/sco.h |    8 ++++----
>  net/bluetooth/sco.c         |   22 +++++++++++++---------
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index e28a2a7..ea5c664 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -55,11 +55,11 @@ struct sco_conninfo {
>  struct sco_conn {
>  	struct hci_conn	*hcon;
>  
> -	bdaddr_t 	*dst;
> -	bdaddr_t 	*src;
> -	
> +	bdaddr_t	*dst;
> +	bdaddr_t	*src;
> +
>  	spinlock_t	lock;
> -	struct sock 	*sk;
> +	struct sock	*sk;
>  
>  	unsigned int    mtu;
>  };
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 66b9e5c..960c6d1 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -44,7 +44,7 @@
>  #include <net/sock.h>
>  
>  #include <asm/system.h>
> -#include <asm/uaccess.h>
> +#include <linux/uaccess.h>
>  
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
> @@ -52,7 +52,7 @@
>  
>  #define VERSION "0.6"
>  
> -static int disable_esco = 0;
> +static int disable_esco;

I don't think this change is right. Can we be sure that disable_esco
will be 0 by default?

>  
>  static const struct proto_ops sco_sock_ops;
>  
> @@ -138,16 +138,17 @@ static inline struct sock *sco_chan_get(struct sco_conn *conn)
>  
>  static int sco_conn_del(struct hci_conn *hcon, int err)
>  {
> -	struct sco_conn *conn;
> +	struct sco_conn *conn = hcon->sco_data;
>  	struct sock *sk;
>  
> -	if (!(conn = hcon->sco_data))
> +	if (!conn)
>  		return 0;
>  
>  	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
>  
>  	/* Kill socket */
> -	if ((sk = sco_chan_get(conn))) {
> +	sk = sco_chan_get(conn);
> +	if (sk) {
>  		bh_lock_sock(sk);
>  		sco_sock_clear_timer(sk);
>  		sco_chan_del(sk, err);
> @@ -185,7 +186,8 @@ static int sco_connect(struct sock *sk)
>  
>  	BT_DBG("%s -> %s", batostr(src), batostr(dst));
>  
> -	if (!(hdev = hci_get_route(dst, src)))
> +	hdev = hci_get_route(dst, src);
> +	if (!hdev)
>  		return -EHOSTUNREACH;
>  
>  	hci_dev_lock_bh(hdev);
> @@ -510,7 +512,8 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
>  	/* Set destination address and psm */
>  	bacpy(&bt_sk(sk)->dst, &sa->sco_bdaddr);
>  
> -	if ((err = sco_connect(sk)))
> +	err = sco_connect(sk);
> +	if (err)
>  		goto done;
>  
>  	err = bt_sock_wait_state(sk, BT_CONNECTED,
> @@ -828,13 +831,14 @@ static void sco_chan_del(struct sock *sk, int err)
>  
>  static void sco_conn_ready(struct sco_conn *conn)
>  {
> -	struct sock *parent, *sk;
> +	struct sock *parent;
> +	struct sock *sk = conn->sk;
>  
>  	BT_DBG("conn %p", conn);
>  
>  	sco_conn_lock(conn);
>  
> -	if ((sk = conn->sk)) {
> +	if (sk) {
>  		sco_sock_clear_timer(sk);
>  		bh_lock_sock(sk);
>  		sk->sk_state = BT_CONNECTED;


Otherwise looks good.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC 1/4] Bluetooth: clean up sco code
From: Gustavo F. Padovan @ 2010-12-01 21:16 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: Anderson Lizardo, linux-bluetooth
In-Reply-To: <AANLkTikfeMqq-4fTRHwmL81TDnF2=sQsMhL9o7GoEcX2@mail.gmail.com>

Hi Andrei, 

* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2010-12-01 15:44:50 +0200]:

> Hi,
> 
> On Fri, Nov 26, 2010 at 6:14 PM, Anderson Lizardo
> <anderson.lizardo@openbossa.org> wrote:
> > Hi Andrei,
> >
> > On Fri, Nov 26, 2010 at 11:22 AM, Emeltchenko Andrei
> > <Andrei.Emeltchenko.news@gmail.com> wrote:
> >> @@ -828,13 +831,14 @@ static void sco_chan_del(struct sock *sk, int err)
> >>
> >>  static void sco_conn_ready(struct sco_conn *conn)
> >>  {
> >> -       struct sock *parent, *sk;
> >> +       struct sock *parent;
> >> +       struct sock *sk = conn->sk;
> >
> > I wonder if there is a problem with accessing conn->sk here outside
> > the lock protection?
> 
> I believe here is a safe access. We just acquire pointer and possible
> protected operations
> are executed inside lock.

Agreed.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* pull request: bluetooth-2.6 2010-11-01
From: Gustavo F. Padovan @ 2010-12-01 21:11 UTC (permalink / raw)
  To: linville; +Cc: marcel, linux-wireless, linux-bluetooth

Hi John,

Here goes 3 simple patches intended to 2.6.37. First one is from myself
and fix some trivial return values. Then we also have a fix from Stefan
Seyfried on btusb to avoid emitting an err when we USB is autosupended.
Last, a patch from Bala Shanmugam that fix the firmware loading process
for the ath3k driver.

Please pull, or let me know if you disagree with the patches. Thanks a
lot. ;)


The following changes since commit 916448e77f6bcaaa7f13c3de0c3851783ae2bfd0:

  ath9k: Fix STA disconnect issue due to received MIC failed bcast frames (2010-11-30 13:45:02 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git master

Bala Shanmugam (1):
      Bluetooth: Add new PID for Atheros 3011

Gustavo F. Padovan (1):
      Bluetooth: Fix not returning proper error in SCO

Stefan Seyfried (1):
      Bluetooth: Fix log spamming in btusb due to autosuspend

 drivers/bluetooth/ath3k.c |    4 ++++
 drivers/bluetooth/btusb.c |   12 +++++++++---
 net/bluetooth/sco.c       |    6 +++---
 3 files changed, 16 insertions(+), 6 deletions(-)

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH v3] Bluetooth: Add new PID for Atheros 3011
From: Gustavo F. Padovan @ 2010-12-01 17:50 UTC (permalink / raw)
  To: Bala Shanmugam; +Cc: linux-bluetooth
In-Reply-To: <1290773146-4705-1-git-send-email-sbalashanmugam@atheros.com>

Hi Bala,

* Bala Shanmugam <sbalashanmugam@atheros.com> [2010-11-26 17:35:46 +0530]:

> Atheros 3011 has small sflash firmware and needs to be
> blacklisted in transport driver to load actual firmware
> in DFU driver.
> 
> Signed-off-by: Bala Shanmugam <sbalashanmugam@atheros.com>
> ---
>  drivers/bluetooth/ath3k.c |    4 ++++
>  drivers/bluetooth/btusb.c |    3 +++
>  2 files changed, 7 insertions(+), 0 deletions(-)

Applied, thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH] btusb: Fix log spamming due to autosuspend
From: Gustavo F. Padovan @ 2010-12-01 17:49 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: Marcel Holtmann, linux-bluetooth, gregkh, Stefan Seyfried,
	Oliver Neukum
In-Reply-To: <20101201154714.517289f7@susi>

Hi Stefan,

* Stefan Seyfried <stefan.seyfried@googlemail.com> [2010-12-01 15:47:14 +0100]:

> Hi Marcel,
> 
> On Wed, 01 Dec 2010 11:42:20 +0100
> Marcel Holtmann <marcel@holtmann.org> wrote:
> 
> > Hi Stefan,
> > 
> > > If a device is autosuspended an inability to resubmit URBs is
> > > to be expected. Check the error code and only log real errors.
> > > (Now that autosuspend is default enabled for btusb, those log
> > > messages were happening all the time e.g. with a BT mouse)
> > > 
> > > Signed-off-by: Stefan Seyfried <seife+kernel@b1-systems.com>
> > > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > 
> > we had a similar one some time ago, but I am fine with this one as well.
> > Actually this one might be a bit better since it still keeps some
> > errors.
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> Could you (or Gustavo) send it to Linus? It's pretty trivial, but the
> messages are annoying and users will complain if they are still in 2.6.37
> final.

I'll send it, applied to bluetooth-2.6 tree. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* not able to connect mobile with laptop using bluetooth
From: Rahul Ruikar @ 2010-12-01 16:54 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

I am not able to pair my mobile with laptop.
when I try to pair it..it times out very soonwhile tying "pin"
i get following error
Dec  1 22:17:38 ganesha bluetoothd[1094]: Discovery session 0xb8fc95b8
with :1.73 activated
Dec  1 22:17:48 ganesha bluetoothd[1094]: Stopping discovery
Dec  1 22:17:50 ganesha bluetoothd[1094]: link_key_request
(sba=00:16:CF:FB:D4:99, dba=00:23:AF:D7:53:20)
Dec  1 22:17:50 ganesha bluetoothd[1094]: pin_code_request
(sba=00:16:CF:FB:D4:99, dba=00:23:AF:D7:53:20)
Dec  1 22:18:01 ganesha NetworkManager[914]: <info> BT device
00:23:AF:D7:53:20 removed
Dec  1 22:18:01 ganesha bluetoothd[1094]: Stopping discovery

sometimes i get this error if i use option "Send files to device"
Dec  1 20:13:48 ganesha bluetoothd[1267]: link_key_notify
(sba=00:16:CF:FB:D4:99, dba=00:23:AF:D7:53:20, type=0)
Dec  1 20:13:59 ganesha bluetoothd[1267]: probe failed with driver
input-headset for device /org/bluez/1252/hci0/dev_00_23_AF_D7_53_20
Dec  1 20:13:59 ganesha bluetoothd[1267]: Stopping discovery
Dec  1 20:14:06 ganesha kernel: btusb_intr_complete: hci0 urb d15cf580
failed to resubmit (1)
Dec  1 20:14:06 ganesha kernel: btusb_bulk_complete: hci0 urb d15cf080
failed to resubmit (1)
Dec  1 20:14:06 ganesha kernel: btusb_bulk_complete: hci0 urb d15cf480
failed to resubmit (1)
Dec  1 20:14:24 ganesha kernel: btusb_intr_complete: hci0 urb d1491a00
failed to resubmit (1)
Dec  1 20:14:24 ganesha kernel: btusb_bulk_complete: hci0 urb d1491300
failed to resubmit (1)
Dec  1 20:14:24 ganesha kernel: btusb_bulk_complete: hci0 urb d1491b80
failed to resubmit (1)
Dec  1 20:14:37 ganesha kernel: btusb_intr_complete: hci0 urb f6683400
failed to resubmit (1)
Dec  1 20:14:37 ganesha kernel: btusb_bulk_complete: hci0 urb f6683200
failed to resubmit (1)
Dec  1 20:14:37 ganesha kernel: btusb_bulk_complete: hci0 urb f6683180
failed to resubmit (1)


any idea what could be wrong ..also why it says "input-headset"

Thanks,
Rahul

^ permalink raw reply

* [PATCH 3/3] Check authentication permissions on attribute server
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1291219986-29913-1-git-send-email-anderson.lizardo@openbossa.org>

Attributes may require encryption for certain operations. This commit
adds checks to the attribute server which verify whether the current
connection is encrypted (currently done by checking the security level,
but may be changed later) and the attribute being accessed requires
authentication. If encryption requirements are not satisfied, the
"Insufficient Encryption" error is returned by the server.
---
 src/attrib-server.c |   76 +++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 32357f0..fee5462 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -57,6 +57,7 @@ struct gatt_channel {
 	GAttrib *attrib;
 	guint mtu;
 	guint id;
+	uint8_t perms;
 };
 
 struct group_elem {
@@ -138,8 +139,11 @@ static sdp_record_t *server_record_new(void)
 	return record;
 }
 
-static uint8_t att_check_perms(uint8_t opcode, uint8_t perms)
+static uint8_t att_check_perms(struct gatt_channel *channel, uint8_t opcode,
+								uint8_t perms)
 {
+	uint8_t chp = channel->perms;
+
 	switch (opcode) {
 	case ATT_OP_READ_BY_GROUP_REQ:
 	case ATT_OP_READ_BY_TYPE_REQ:
@@ -148,20 +152,41 @@ static uint8_t att_check_perms(uint8_t opcode, uint8_t perms)
 	case ATT_OP_READ_MULTI_REQ:
 		if (!(perms & ATT_ACCESS(ATT_READ, ATT_NONE)))
 			return ATT_ECODE_READ_NOT_PERM;
+
+		if ((perms & ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION))
+			/* Attribute does not require authentication for read */
+			return 0;
+
+		if ((chp & ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION))
+			/* Connection is not encrypted */
+			return ATT_ECODE_INSUFF_AUTHEN;
 		break;
 	case ATT_OP_PREP_WRITE_REQ:
 	case ATT_OP_WRITE_REQ:
 	case ATT_OP_WRITE_CMD:
 		if (!(perms & ATT_ACCESS(ATT_WRITE, ATT_NONE)))
 			return ATT_ECODE_WRITE_NOT_PERM;
+
+		if ((perms & ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION))
+			/* Attribute does not require authentication for write */
+			return 0;
+
+		if ((chp & ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION))
+			/* Connection is not encrypted */
+			return ATT_ECODE_INSUFF_AUTHEN;
 		break;
 	}
 
 	return 0;
 }
 
-static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
-							uint8_t *pdu, int len)
+static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
+						uint16_t end, uuid_t *uuid,
+						uint8_t *pdu, int len)
 {
 	struct att_data_list *adl;
 	struct attribute *a;
@@ -212,7 +237,8 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (last_size && (last_size != a->len))
 			break;
 
-		status = att_check_perms(ATT_OP_READ_BY_GROUP_REQ, a->perms);
+		status = att_check_perms(channel, ATT_OP_READ_BY_GROUP_REQ,
+								a->perms);
 		if (status) {
 			g_slist_foreach(groups, (GFunc) g_free, NULL);
 			g_slist_free(groups);
@@ -271,8 +297,9 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 	return length;
 }
 
-static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
-							uint8_t *pdu, int len)
+static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
+						uint16_t end, uuid_t *uuid,
+						uint8_t *pdu, int len)
 {
 	struct att_data_list *adl;
 	GSList *l, *types;
@@ -297,7 +324,8 @@ static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (sdp_uuid_cmp(&a->uuid, uuid)  != 0)
 			continue;
 
-		status = att_check_perms(ATT_OP_READ_BY_TYPE_REQ, a->perms);
+		status = att_check_perms(channel, ATT_OP_READ_BY_TYPE_REQ,
+								a->perms);
 		if (status) {
 			g_slist_free(types);
 			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
@@ -508,7 +536,8 @@ static int attribute_cmp(gconstpointer a1, gconstpointer a2)
 	return attrib1->handle - attrib2->handle;
 }
 
-static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
+static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
+							uint8_t *pdu, int len)
 {
 	struct attribute *a;
 	uint8_t status;
@@ -522,7 +551,7 @@ static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 
 	a = l->data;
 
-	status = att_check_perms(ATT_OP_READ_REQ, a->perms);
+	status = att_check_perms(channel, ATT_OP_READ_REQ, a->perms);
 	if (status)
 		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
 									len);
@@ -530,8 +559,9 @@ static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 	return enc_read_resp(a->data, a->len, pdu, len);
 }
 
-static uint16_t write_value(uint16_t handle, const uint8_t *value, int vlen,
-							uint8_t *pdu, int len)
+static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
+						const uint8_t *value, int vlen,
+						uint8_t *pdu, int len)
 {
 	struct attribute *a;
 	uint8_t status;
@@ -546,7 +576,7 @@ static uint16_t write_value(uint16_t handle, const uint8_t *value, int vlen,
 
 	a = l->data;
 
-	status = att_check_perms(ATT_OP_WRITE_REQ, a->perms);
+	status = att_check_perms(channel, ATT_OP_WRITE_REQ, a->perms);
 	if (status)
 		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
 									len);
@@ -595,7 +625,8 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = read_by_group(start, end, &uuid, opdu, channel->mtu);
+		length = read_by_group(channel, start, end, &uuid, opdu,
+								channel->mtu);
 		break;
 	case ATT_OP_READ_BY_TYPE_REQ:
 		length = dec_read_by_type_req(ipdu, len, &start, &end, &uuid);
@@ -604,7 +635,8 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = read_by_type(start, end, &uuid, opdu, channel->mtu);
+		length = read_by_type(channel, start, end, &uuid, opdu,
+								channel->mtu);
 		break;
 	case ATT_OP_READ_REQ:
 		length = dec_read_req(ipdu, len, &start);
@@ -613,7 +645,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = read_value(start, opdu, channel->mtu);
+		length = read_value(channel, start, opdu, channel->mtu);
 		break;
 	case ATT_OP_MTU_REQ:
 		length = dec_mtu_req(ipdu, len, &mtu);
@@ -640,12 +672,14 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = write_value(start, value, vlen, opdu, channel->mtu);
+		length = write_value(channel, start, value, vlen, opdu,
+								channel->mtu);
 		break;
 	case ATT_OP_WRITE_CMD:
 		length = dec_write_cmd(ipdu, len, &start, value, &vlen);
 		if (length > 0)
-			write_value(start, value, vlen, opdu, channel->mtu);
+			write_value(channel, start, value, vlen, opdu,
+								channel->mtu);
 		return;
 	case ATT_OP_FIND_BY_TYPE_REQ:
 		length = dec_find_by_type_req(ipdu, len, &start, &end,
@@ -682,6 +716,7 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 {
 	struct gatt_channel *channel;
 	GError *gerr = NULL;
+	int sec_level;
 
 	if (err) {
 		error("%s", err->message);
@@ -693,6 +728,7 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 	bt_io_get(io, BT_IO_L2CAP, &gerr,
 			BT_IO_OPT_SOURCE_BDADDR, &channel->src,
 			BT_IO_OPT_DEST_BDADDR, &channel->dst,
+			BT_IO_OPT_SEC_LEVEL, &sec_level,
 			BT_IO_OPT_INVALID);
 	if (gerr) {
 		error("bt_io_get: %s", gerr->message);
@@ -705,6 +741,12 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 	channel->attrib = g_attrib_new(io);
 	channel->mtu = ATT_DEFAULT_MTU;
 
+	if (sec_level > BT_IO_SEC_LOW)
+		channel->perms = ATT_ACCESS(ATT_READ | ATT_WRITE,
+							ATT_AUTHENTICATION);
+	else
+		channel->perms = ATT_ACCESS(ATT_READ | ATT_WRITE, ATT_NONE);
+
 	channel->id = g_attrib_register(channel->attrib, GATTRIB_ALL_EVENTS,
 				channel_handler, channel, NULL);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] Check attribute permissions in attribute server
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1291219986-29913-1-git-send-email-anderson.lizardo@openbossa.org>

From: Bruna Moreira <bruna.moreira@openbossa.org>

The attribute server must verify if the operation (read/write) is
permitted before running the request, and send "Read Not Permitted" or
"Write Not Permitted" error response to client if appropriate.
---
 TODO                |    7 -----
 src/attrib-server.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/TODO b/TODO
index 49a9e76..32d3a61 100644
--- a/TODO
+++ b/TODO
@@ -136,13 +136,6 @@ ATT/GATT
   Priority: Low
   Complexity: C2
 
-- Attribute server shall implement attribute permission verification,
-  returning an error code if necessary. See Volume 3, Part F, 3.2.5
-  for more information.
-
-  Priority: Low
-  Complexity: C2
-
 - Implement Client Characteristic Configuration support in the attribute
   server to manage indications and notications. This is a per client attribute
   to control how the client wants to receive reports of changes in a given
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 4199355..32357f0 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -138,6 +138,28 @@ static sdp_record_t *server_record_new(void)
 	return record;
 }
 
+static uint8_t att_check_perms(uint8_t opcode, uint8_t perms)
+{
+	switch (opcode) {
+	case ATT_OP_READ_BY_GROUP_REQ:
+	case ATT_OP_READ_BY_TYPE_REQ:
+	case ATT_OP_READ_REQ:
+	case ATT_OP_READ_BLOB_REQ:
+	case ATT_OP_READ_MULTI_REQ:
+		if (!(perms & ATT_ACCESS(ATT_READ, ATT_NONE)))
+			return ATT_ECODE_READ_NOT_PERM;
+		break;
+	case ATT_OP_PREP_WRITE_REQ:
+	case ATT_OP_WRITE_REQ:
+	case ATT_OP_WRITE_CMD:
+		if (!(perms & ATT_ACCESS(ATT_WRITE, ATT_NONE)))
+			return ATT_ECODE_WRITE_NOT_PERM;
+		break;
+	}
+
+	return 0;
+}
+
 static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 							uint8_t *pdu, int len)
 {
@@ -146,6 +168,7 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 	struct group_elem *cur, *old = NULL;
 	GSList *l, *groups;
 	uint16_t length, last_handle, last_size = 0;
+	uint8_t status;
 	int i;
 
 	if (start > end || start == 0x0000)
@@ -189,6 +212,14 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (last_size && (last_size != a->len))
 			break;
 
+		status = att_check_perms(ATT_OP_READ_BY_GROUP_REQ, a->perms);
+		if (status) {
+			g_slist_foreach(groups, (GFunc) g_free, NULL);
+			g_slist_free(groups);
+			return enc_error_resp(ATT_OP_READ_BY_GROUP_REQ,
+						a->handle, status, pdu, len);
+		}
+
 		cur = g_new0(struct group_elem, 1);
 		cur->handle = a->handle;
 		cur->data = a->data;
@@ -247,6 +278,7 @@ static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 	GSList *l, *types;
 	struct attribute *a;
 	uint16_t num, length;
+	uint8_t status;
 	int i;
 
 	if (start > end || start == 0x0000)
@@ -265,6 +297,13 @@ static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (sdp_uuid_cmp(&a->uuid, uuid)  != 0)
 			continue;
 
+		status = att_check_perms(ATT_OP_READ_BY_TYPE_REQ, a->perms);
+		if (status) {
+			g_slist_free(types);
+			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
+						a->handle, status, pdu, len);
+		}
+
 		/* All elements must have the same length */
 		if (length == 0)
 			length = a->len;
@@ -472,6 +511,7 @@ static int attribute_cmp(gconstpointer a1, gconstpointer a2)
 static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 {
 	struct attribute *a;
+	uint8_t status;
 	GSList *l;
 	guint h = handle;
 
@@ -482,23 +522,41 @@ static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 
 	a = l->data;
 
+	status = att_check_perms(ATT_OP_READ_REQ, a->perms);
+	if (status)
+		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
+									len);
+
 	return enc_read_resp(a->data, a->len, pdu, len);
 }
 
-static void write_value(uint16_t handle, const uint8_t *value, int vlen)
+static uint16_t write_value(uint16_t handle, const uint8_t *value, int vlen,
+							uint8_t *pdu, int len)
 {
 	struct attribute *a;
+	uint8_t status;
 	GSList *l;
 	guint h = handle;
 	uuid_t uuid;
 
 	l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
 	if (!l)
-		return;
+		return enc_error_resp(ATT_OP_WRITE_REQ, handle,
+				ATT_ECODE_INVALID_HANDLE, pdu, len);
 
 	a = l->data;
+
+	status = att_check_perms(ATT_OP_WRITE_REQ, a->perms);
+	if (status)
+		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
+									len);
+
 	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
 	attrib_db_update(handle, &uuid, value, vlen);
+
+	pdu[0] = ATT_OP_WRITE_RESP;
+
+	return sizeof(pdu[0]);
 }
 
 static uint16_t mtu_exchange(struct gatt_channel *channel, uint16_t mtu,
@@ -582,14 +640,12 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		write_value(start, value, vlen);
-		opdu[0] = ATT_OP_WRITE_RESP;
-		length = sizeof(opdu[0]);
+		length = write_value(start, value, vlen, opdu, channel->mtu);
 		break;
 	case ATT_OP_WRITE_CMD:
 		length = dec_write_cmd(ipdu, len, &start, value, &vlen);
 		if (length > 0)
-			write_value(start, value, vlen);
+			write_value(start, value, vlen, opdu, channel->mtu);
 		return;
 	case ATT_OP_FIND_BY_TYPE_REQ:
 		length = dec_find_by_type_req(ipdu, len, &start, &end,
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/3] Initial attribute permission implementation
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1291219986-29913-1-git-send-email-anderson.lizardo@openbossa.org>

From: Bruna Moreira <bruna.moreira@openbossa.org>

Create definitions for attribute permissions, add "perms" field to
struct attribute and add attribute permissions to attribute server.
---
 attrib/att.h        |    1 +
 attrib/example.c    |   89 ++++++++++++++++++++++++++++-----------------------
 src/attrib-server.c |    4 ++-
 src/attrib-server.h |   13 +++++++-
 4 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 7c98b4a..a29aae2 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -112,6 +112,7 @@
 struct attribute {
 	uint16_t handle;
 	uuid_t uuid;
+	uint8_t perms;
 	int len;
 	uint8_t data[0];
 };
diff --git a/attrib/example.c b/attrib/example.c
index c29e1e4..2fedfc7 100644
--- a/attrib/example.c
+++ b/attrib/example.c
@@ -101,7 +101,7 @@ static int register_attributes(void)
 	u16 = htons(GENERIC_ACCESS_PROFILE_ID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0001, &uuid, atval, 2);
+	attrib_db_add(0x0001, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* GAP service: device name characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -111,20 +111,21 @@ static int register_attributes(void)
 	atval[2] = 0x00;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0004, &uuid, atval, 5);
+	attrib_db_add(0x0004, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* GAP service: device name attribute */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_DEVICE_NAME);
 	len = strlen(devname);
 	strncpy((char *) atval, devname, len);
-	attrib_db_add(0x0006, &uuid, atval, len);
+	attrib_db_add(0x0006, &uuid, ATT_ACCESS(ATT_READ | ATT_WRITE, ATT_NONE),
+								atval, len);
 
 	/* GATT service: primary service definition */
 	sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
 	u16 = htons(GENERIC_ATTRIB_PROFILE_ID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0010, &uuid, atval, 2);
+	attrib_db_add(0x0010, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* GATT service: attributes opcodes characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -134,20 +135,20 @@ static int register_attributes(void)
 	atval[2] = 0x00;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0011, &uuid, atval, 5);
+	attrib_db_add(0x0011, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* GATT service: attribute opcodes supported */
 	sdp_uuid16_create(&uuid, OPCODES_SUPPORTED_UUID);
 	atval[0] = 0xFF;
 	atval[1] = 0x01;
-	attrib_db_add(0x0012, &uuid, atval, 2);
+	attrib_db_add(0x0012, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Battery state service: primary service definition */
 	sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
 	u16 = htons(BATTERY_STATE_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0100, &uuid, atval, 2);
+	attrib_db_add(0x0100, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Battery: battery state characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -157,18 +158,19 @@ static int register_attributes(void)
 	atval[2] = 0x01;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0106, &uuid, atval, 5);
+	attrib_db_add(0x0106, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Battery: battery state attribute */
 	sdp_uuid16_create(&uuid, BATTERY_STATE_UUID);
 	atval[0] = 0x04;
-	attrib_db_add(0x0110, &uuid, atval, 1);
+	attrib_db_add(0x0110, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 1);
 
 	/* Battery: Client Characteristic Configuration */
 	sdp_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
 	atval[0] = 0x00;
 	atval[1] = 0x00;
-	attrib_db_add(0x0111, &uuid, atval, 2);
+	attrib_db_add(0x0111, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE) |
+			ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION), atval, 2);
 
 	timeout_id = g_timeout_add_seconds(10, change_battery_state, NULL);
 
@@ -177,7 +179,7 @@ static int register_attributes(void)
 	u16 = htons(THERM_HUMIDITY_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0200, &uuid, atval, 2);
+	attrib_db_add(0x0200, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Thermometer: Include */
 	sdp_uuid16_create(&uuid, GATT_INCLUDE_UUID);
@@ -188,14 +190,14 @@ static int register_attributes(void)
 	atval[3] = 0x05;
 	atval[4] = u16 >> 8;
 	atval[5] = u16;
-	attrib_db_add(0x0201, &uuid, atval, 6);
+	attrib_db_add(0x0201, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 6);
 
 	/* Thermometer: Include */
 	atval[0] = 0x50;
 	atval[1] = 0x05;
 	atval[2] = 0x68;
 	atval[3] = 0x05;
-	attrib_db_add(0x0202, &uuid, atval, 4);
+	attrib_db_add(0x0202, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 4);
 
 	/* Thermometer: temperature characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -205,13 +207,13 @@ static int register_attributes(void)
 	atval[2] = 0x02;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0203, &uuid, atval, 5);
+	attrib_db_add(0x0203, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Thermometer: temperature characteristic value */
 	sdp_uuid16_create(&uuid, TEMPERATURE_UUID);
 	atval[0] = 0x8A;
 	atval[1] = 0x02;
-	attrib_db_add(0x0204, &uuid, atval, 2);
+	attrib_db_add(0x0204, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Thermometer: temperature characteristic format */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID);
@@ -224,13 +226,14 @@ static int register_attributes(void)
 	u16 = htons(FMT_OUTSIDE_UUID);
 	atval[5] = u16 >> 8;
 	atval[6] = u16;
-	attrib_db_add(0x0205, &uuid, atval, 7);
+	attrib_db_add(0x0205, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 7);
 
 	/* Thermometer: characteristic user description */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
 	len = strlen(desc_out_temp);
 	strncpy((char *) atval, desc_out_temp, len);
-	attrib_db_add(0x0206, &uuid, atval, len);
+	attrib_db_add(0x0206, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Thermometer: relative humidity characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -240,12 +243,12 @@ static int register_attributes(void)
 	atval[2] = 0x02;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0210, &uuid, atval, 5);
+	attrib_db_add(0x0210, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Thermometer: relative humidity value */
 	sdp_uuid16_create(&uuid, RELATIVE_HUMIDITY_UUID);
 	atval[0] = 0x27;
-	attrib_db_add(0x0212, &uuid, atval, 1);
+	attrib_db_add(0x0212, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 1);
 
 	/* Thermometer: relative humidity characteristic format */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID);
@@ -260,20 +263,21 @@ static int register_attributes(void)
 	u16 = htons(FMT_OUTSIDE_UUID);
 	atval[6] = u16 >> 8;
 	atval[7] = u16;
-	attrib_db_add(0x0213, &uuid, atval, 8);
+	attrib_db_add(0x0213, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 8);
 
 	/* Thermometer: characteristic user description */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
 	len = strlen(desc_out_hum);
 	strncpy((char *) atval, desc_out_hum, len);
-	attrib_db_add(0x0214, &uuid, atval, len);
+	attrib_db_add(0x0214, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Secondary Service: Manufacturer Service */
 	sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID);
 	u16 = htons(MANUFACTURER_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0500, &uuid, atval, 2);
+	attrib_db_add(0x0500, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Manufacturer name characteristic definition */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -283,13 +287,14 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0501, &uuid, atval, 5);
+	attrib_db_add(0x0501, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Manufacturer name characteristic value */
 	sdp_uuid16_create(&uuid, MANUFACTURER_NAME_UUID);
 	len = strlen(manufacturer_name1);
 	strncpy((char *) atval, manufacturer_name1, len);
-	attrib_db_add(0x0502, &uuid, atval, len);
+	attrib_db_add(0x0502, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Manufacturer serial number characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -299,20 +304,21 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0503, &uuid, atval, 5);
+	attrib_db_add(0x0503, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Manufacturer serial number characteristic value */
 	sdp_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID);
 	len = strlen(serial1);
 	strncpy((char *) atval, serial1, len);
-	attrib_db_add(0x0504, &uuid, atval, len);
+	attrib_db_add(0x0504, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Secondary Service: Manufacturer Service */
 	sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID);
 	u16 = htons(MANUFACTURER_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0505, &uuid, atval, 2);
+	attrib_db_add(0x0505, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Manufacturer name characteristic definition */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -322,14 +328,14 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0506, &uuid, atval, 5);
+	attrib_db_add(0x0506, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Secondary Service: Vendor Specific Service */
 	sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID);
 	u16 = htons(VENDOR_SPECIFIC_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0550, &uuid, atval, 2);
+	attrib_db_add(0x0550, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Vendor Specific Type characteristic definition */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -339,7 +345,7 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0560, &uuid, atval, 5);
+	attrib_db_add(0x0560, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Vendor Specific Type characteristic value */
 	sdp_uuid16_create(&uuid, VENDOR_SPECIFIC_TYPE_UUID);
@@ -349,13 +355,14 @@ static int register_attributes(void)
 	atval[3] = 0x64;
 	atval[4] = 0x6F;
 	atval[5] = 0x72;
-	attrib_db_add(0x0568, &uuid, atval, 6);
+	attrib_db_add(0x0568, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 6);
 
 	/* Manufacturer name attribute */
 	sdp_uuid16_create(&uuid, MANUFACTURER_NAME_UUID);
 	len = strlen(manufacturer_name2);
 	strncpy((char *) atval, manufacturer_name2, len);
-	attrib_db_add(0x0507, &uuid, atval, len);
+	attrib_db_add(0x0507, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Characteristic: serial number */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -365,18 +372,19 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0508, &uuid, atval, 5);
+	attrib_db_add(0x0508, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Serial number characteristic value */
 	sdp_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID);
 	len = strlen(serial2);
 	strncpy((char *) atval, serial2, len);
-	attrib_db_add(0x0509, &uuid, atval, len);
+	attrib_db_add(0x0509, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Weight service: primary service definition */
 	sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
 	memcpy(atval, prim_weight_uuid, 16);
-	attrib_db_add(0x0680, &uuid, atval, 16);
+	attrib_db_add(0x0680, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 16);
 
 	/* Weight: include */
 	sdp_uuid16_create(&uuid, GATT_INCLUDE_UUID);
@@ -387,7 +395,7 @@ static int register_attributes(void)
 	atval[3] = 0x05;
 	atval[4] = u16 >> 8;
 	atval[5] = u16;
-	attrib_db_add(0x0681, &uuid, atval, 6);
+	attrib_db_add(0x0681, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 6);
 
 	/* Weight: characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -395,7 +403,7 @@ static int register_attributes(void)
 	atval[1] = 0x83;
 	atval[2] = 0x06;
 	memcpy(atval + 3, char_weight_uuid, 16);
-	attrib_db_add(0x0682, &uuid, atval, 19);
+	attrib_db_add(0x0682, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 19);
 
 	/* Weight: characteristic value */
 	sdp_uuid128_create(&uuid, char_weight_uuid);
@@ -403,7 +411,7 @@ static int register_attributes(void)
 	atval[1] = 0x55;
 	atval[2] = 0x00;
 	atval[3] = 0x00;
-	attrib_db_add(0x0683, &uuid, atval, 4);
+	attrib_db_add(0x0683, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 4);
 
 	/* Weight: characteristic format */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID);
@@ -418,13 +426,14 @@ static int register_attributes(void)
 	u16 = htons(FMT_HANGING_UUID);
 	atval[6] = u16 >> 8;
 	atval[7] = u16;
-	attrib_db_add(0x0684, &uuid, atval, 8);
+	attrib_db_add(0x0684, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 8);
 
 	/* Weight: characteristic user description */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
 	len = strlen(desc_weight);
 	strncpy((char *) atval, desc_weight, len);
-	attrib_db_add(0x0685, &uuid, atval, len);
+	attrib_db_add(0x0685, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	return 0;
 }
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 41c0ffc..4199355 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -760,7 +760,8 @@ void attrib_server_exit(void)
 		remove_record_from_server(sdp_handle);
 }
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len)
+int attrib_db_add(uint16_t handle, uuid_t *uuid, uint8_t perms,
+						const uint8_t *value, int len)
 {
 	struct attribute *a;
 
@@ -769,6 +770,7 @@ int attrib_db_add(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len)
 	a = g_malloc0(sizeof(struct attribute) + len);
 	a->handle = handle;
 	memcpy(&a->uuid, uuid, sizeof(uuid_t));
+	a->perms = perms;
 	a->len = len;
 	memcpy(a->data, value, len);
 
diff --git a/src/attrib-server.h b/src/attrib-server.h
index 4a0afa6..e74a134 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -25,7 +25,18 @@
 int attrib_server_init(void);
 void attrib_server_exit(void);
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len);
+/* Access permissions */
+#define ATT_READ	1
+#define ATT_WRITE	2
+/* Authentication/Authorization permissions */
+#define ATT_NONE		0
+#define ATT_AUTHENTICATION	2
+#define ATT_AUTHORIZATION	4
+/* Build bit mask for permission checks */
+#define ATT_ACCESS(x, y)	(((x) << y) | (x))
+
+int attrib_db_add(uint16_t handle, uuid_t *uuid, uint8_t perms,
+						const uint8_t *value, int len);
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 								int len);
 int attrib_db_del(uint16_t handle);
-- 
1.7.0.4


^ permalink raw reply related


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