Linux bluetooth development
 help / color / mirror / Atom feed
* RE: [PATCHv4 0/7] Support for out of band association model
From: Mike Tsai @ 2010-11-30 21:26 UTC (permalink / raw)
  To: Mike Tsai, Szymon Janc, linux-bluetooth@vger.kernel.org
In-Reply-To: <35B17FE5076C7040809188FBE7913F98406D68CFAB@SC1EXMB-MBCL.global.atheros.com>

Hi Szymon,

	Correction on my comments below,

-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Mike Tsai
Sent: Tuesday, November 30, 2010 8:02 AM
To: Szymon Janc; linux-bluetooth@vger.kernel.org
Subject: RE: [PATCHv4 0/7] Support for out of band association model

Hi Szymon,

-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Szymon Janc
Sent: Tuesday, November 23, 2010 2:53 AM
To: linux-bluetooth@vger.kernel.org
Cc: Szymon Janc
Subject: [PATCHv4 0/7] Support for out of band association model

Changes in this version:
  - code style/flow comments from Johan included
  - changes in D-Bus OOB api - ReadLocalOobData and provider
    register/unregister moved to adapter path (OobManager interface), provider
    is registered per adapter now
  - some bug fixes

Some things I'd like to collect comments about:
  - OobManager interface could be only available on 2.1 or higher adapters
    (this would require some adapter interface to be able check this in
    dbusoob plugin)
  - Agent's RequestPairing method name
  - RequestPairing is called only on device accepting incoming connection when
    it has OOB data for remote device (on RequestRemoteOobData event).
    Consequence is that when only initiator has OOB data RequestPairing is not
    called. We could store initiator's OOB capability in device structure (now
    only auth and cap are stored) to be able to know if initiator has OOB data.
    Yet, in such case RequestRemoteOobData on accepting device will not be 
    called and I have no idea where/when OOB capability should be check to call
    RequestPairing (and reject pairing if necessary)..

[MTsai]7.7.44 Remote OOB Data Request Event.
If host has received remote peer's OOB data before pairing, the host shall set the "OOB present" flag of the peer device. This "OOB present" flag will be sent over the air as part of pairing process with lmp_ioCap_Req or lmp_iocap_rsp.
If either one of the devices have OOB data, then OOB shall be used to do the pairing.
The event "Remote OOB Data Request Event" shall be sent to host before calculating the commitment value.
The LM state machine shall call "device_request_oob_data" when either local or remote peer has "OOB data present". I think this part of the code is not present.

[Mtsai correction]
The local OOB data present is set when host send HCI command "Read Local OOB Data". And this "OOB data present" flag is sent over the air to peer device during pairing.

The LM state machine is inside the controller firmware, so there is no issue on the stack code you submitted.

Comments are welcome.

BR,
Szymon Janc
on behalf of ST-Ericsson


Szymon Janc (7):
  Add support for Out of Band (OOB) association model
  Add D-Bus OOB plugin
  Add D-Bus OOB API documentation
  Add simple-oobprovider for testing
  Add request for accepting incoming pairing requests with OOB
    mechanism
  Update D-Bus OOB API with RequestPairing method
  Add RequestPairing method in simple-agent for accepting incoming OOB 
       pairing requests

 Makefile.am             |   10 +-
 acinclude.m4            |    6 +
 doc/oob-api.txt         |   76 +++++++++
 doc/oob-api.txt.orig    |   79 +++++++++
 lib/hci.h               |    3 +
 plugins/dbusoob.c       |  412 +++++++++++++++++++++++++++++++++++++++++++++++
 plugins/hciops.c        |  107 +++++++++++--
 src/adapter.c           |   21 +++-
 src/adapter.h           |   10 ++
 src/agent.c             |   59 +++++++-
 src/agent.h             |    3 +
 src/device.c            |   96 +++++++++++
 src/device.h            |   13 ++
 src/event.c             |   89 ++++++++---
 src/event.h             |    4 +-
 src/oob.c               |   67 ++++++++
 src/oob.h               |   47 ++++++
 test/simple-agent       |   10 ++
 test/simple-oobprovider |   57 +++++++
 19 files changed, 1131 insertions(+), 38 deletions(-)
 create mode 100644 doc/oob-api.txt
 create mode 100644 doc/oob-api.txt.orig
 create mode 100644 plugins/dbusoob.c
 create mode 100644 src/oob.c
 create mode 100644 src/oob.h
 create mode 100755 test/simple-oobprovider

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] btusb: Fix log spamming due to autosuspend
From: Stefan Seyfried @ 2010-11-30 21:51 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291150148-14376-1-git-send-email-stefan.seyfried@googlemail.com>

On Tue, 30 Nov 2010 21:49:08 +0100
stefan.seyfried@googlemail.com wrote:

> From: Stefan Seyfried <seife+kernel@b1-systems.com>
> 
> 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>

As Greg has told me, this needs to go through the bluetooth Maintainer.
Personally, with 2.6.37 having autosuspend enabled by default I think this
is pretty urgent, as it spams the log continuously when using a Bluetooth
mouse. So please forward this if deemed acceptable.

Thanks
-- 
Stefan Seyfried

"Any ideas, John?"
"Well, surrounding them's out."

^ permalink raw reply

* Re: [PATCH] btusb: Fix log spamming due to autosuspend
From: Greg KH @ 2010-11-30 23:47 UTC (permalink / raw)
  To: stefan.seyfried; +Cc: linux-bluetooth, Stefan Seyfried, Oliver Neukum
In-Reply-To: <1291150148-14376-1-git-send-email-stefan.seyfried@googlemail.com>

On Tue, Nov 30, 2010 at 09:49:08PM +0100, stefan.seyfried@googlemail.com wrote:
> From: Stefan Seyfried <seife+kernel@b1-systems.com>
> 
> 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>
> ---
>  drivers/bluetooth/btusb.c |    9 ++++++---

This one doesn't go through me, sorry:

	> ./scripts/get_maintainer.pl --file --roles drivers/bluetooth/btusb.c
	Marcel Holtmann <marcel@holtmann.org> (maintainer:BLUETOOTH DRIVERS)
	"Gustavo F. Padovan" <padovan@profusion.mobi> (maintainer:BLUETOOTH DRIVERS)
	linux-bluetooth@vger.kernel.org (open list:BLUETOOTH DRIVERS)
	linux-kernel@vger.kernel.org (open list)

Marcel and Gustavo are the ones that need to handle it.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] btusb: Fix log spamming due to autosuspend
From: Marcel Holtmann @ 2010-12-01 10:42 UTC (permalink / raw)
  To: stefan.seyfried; +Cc: gregkh, linux-bluetooth, Stefan Seyfried, Oliver Neukum
In-Reply-To: <1291150148-14376-1-git-send-email-stefan.seyfried@googlemail.com>

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>

Regards

Marcel



^ permalink raw reply

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

Hi Bala,

> 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(-)

looks good to me now.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [RFC 2/4] Bluetooth: clean up rfcomm code
From: Marcel Holtmann @ 2010-12-01 10:45 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <AANLkTimDn_oVX2YMHhEAqyw=YpeYZuX5WL6mr_F463dC@mail.gmail.com>

Hi Andrei,

> > * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-11-26 17:22:43 +0200]:
> >
> >> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >>
> >> Remove extra spaces, assignments in if statement, zeroing static
> >> variables.
> >>
> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >> ---
> >>  include/net/bluetooth/rfcomm.h |   18 +++++++++---------
> >>  net/bluetooth/rfcomm/core.c    |    8 ++++----
> >>  net/bluetooth/rfcomm/sock.c    |    5 +++--
> >>  net/bluetooth/rfcomm/tty.c     |   28 ++++++++++++++++------------
> >>  4 files changed, 32 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
> >> index 71047bc..6eac4a7 100644
> >> --- a/include/net/bluetooth/rfcomm.h
> >> +++ b/include/net/bluetooth/rfcomm.h
> >> @@ -1,5 +1,5 @@
> >> -/*
> >> -   RFCOMM implementation for Linux Bluetooth stack (BlueZ).
> >> +/*
> >> +   RFCOMM implementation for Linux Bluetooth stack (BlueZ)
> >>     Copyright (C) 2002 Maxim Krasnyansky <maxk@qualcomm.com>
> >>     Copyright (C) 2002 Marcel Holtmann <marcel@holtmann.org>
> >>
> >> @@ -11,13 +11,13 @@
> >>     OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >>     FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
> >>     IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
> >> -   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
> >> -   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> >> -   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> >> +   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
> >> +   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> >> +   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> >>     OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> >>
> >> -   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
> >> -   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
> >> +   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
> >> +   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
> >>     SOFTWARE IS DISCLAIMED.
> >
> > Marcel refused a patch from me in the past because its was touching legal
> > stuff, so or you remove these changes for your patches or wait for
> > Marcel's comments here.
> 
> I fixed extra spaces at the end of the sentences, no legal stuff
> touched in legal terms :-)
> I believe that it is better to have legal text identical to other
> parts of the kernel otherwise
> the legal stuff looks different when comparing with diff tools.

make it a separate patch and not just fix it while at it.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/2] Sim Access Profile API
From: Marcel Holtmann @ 2010-12-01 10:49 UTC (permalink / raw)
  To: Waldemar Rymarkiewicz; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <1291113364-6401-2-git-send-email-waldemar.rymarkiewicz@tieto.com>

Hi Waldemar,

> New API for Sim Access Profile.
> ---
>  Makefile.am     |    2 +-
>  doc/sap-api.txt |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+), 1 deletions(-)
>  create mode 100644 doc/sap-api.txt
> 
> diff --git a/Makefile.am b/Makefile.am
> index 5f96975..97345a3 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -354,7 +354,7 @@ EXTRA_DIST += doc/manager-api.txt \
>  		doc/service-api.txt doc/agent-api.txt doc/attribute-api.txt \
>  		doc/serial-api.txt doc/network-api.txt \
>  		doc/input-api.txt doc/audio-api.txt doc/control-api.txt \
> -		doc/hfp-api.txt doc/assigned-numbers.txt
> +		doc/hfp-api.txt doc/assigned-numbers.txt doc/sap-api.txt
>  
>  AM_YFLAGS = -d
>  
> diff --git a/doc/sap-api.txt b/doc/sap-api.txt
> new file mode 100644
> index 0000000..4e5626e
> --- /dev/null
> +++ b/doc/sap-api.txt
> @@ -0,0 +1,47 @@
> +BlueZ D-Bus Sim Access Profile API description
> +***********************************
> +
> +Copyright (C) 2010 ST-Ericsson SA
> +
> +
> +Sim Access Profile hierarchy
> +============================
> +
> +Service		org.bluez
> +Interface	org.bluez.SimAccess
> +Object path	[variable prefix]/{hci0,hci1,...}
> +
> +Methods		void Disconnect()
> +
> +			Disconnects SAP client from the server.
> +
> +			Possible errors: org.bluez.Error.Failed
> +
> +		void SetProperty(string name, variant value)
> +
> +			Changes the value of the specified property. Only
> +			properties that are listed a read-write are changeable.
> +
> +			Possible Errors: org.bluez.Error.DoesNotExist
> +					 org.bluez.Error.InvalidArguments
> +
> +		dict GetProperties()
> +
> +			Return all properties for the interface. See the
> +			properties section for available properties.
> +
> +			Possible Errors: org.bluez.Error.Failed
> +
> +Signals		PropertyChanged(string name, variant value)
> +
> +			This signal indicates a changed value of the given
> +			property.
> +
> +Properties	boolean Enabled [readwrite]
> +
> +			Set to true to start-up SAP server and register SDP record for
> +			it. Set to false to shutdown SAP server and remove the SDP record.

do we want this really as property? Wouldn't be configuration option be
better to enable this. None of the other profiles has this anymore. And
if their server depends on external programs then it either goes via an
agent or some method call that can be easily tracked.

Regards

Marcel



^ permalink raw reply

* RE: [PATCH 1/2] Sim Access Profile API
From: Waldemar.Rymarkiewicz @ 2010-12-01 11:12 UTC (permalink / raw)
  To: marcel; +Cc: johan.hedberg, linux-bluetooth
In-Reply-To: <1291200591.4795.95.camel@aeonflux>

Hi Marcel,=20

>From: Marcel Holtmann [mailto:marcel@holtmann.org]=20
>Hi Waldemar,
>
>
>do we want this really as property? Wouldn't be configuration=20
>option be better to enable this. None of the other profiles=20
>has this anymore.=20

Well, that is actually a proposal and I don't see any problems if we go for=
 conf options in here. Even more, that would be if fact more usef-ull.  Is =
the main.conf a right place for say EnableSAP=3Dtrue option ?

>And if their server depends on external=20
>programs then it either goes via an agent or some method call=20
>that can be easily tracked.
>
I'm not sure I get it. Do you mean if an external program needs to enable s=
ap they will use method call?=20

Regards,
/Waldek

^ permalink raw reply

* Re: [PATCH 1/2] Sim Access Profile API
From: Johan Hedberg @ 2010-12-01 11:21 UTC (permalink / raw)
  To: Waldemar.Rymarkiewicz; +Cc: marcel, linux-bluetooth
In-Reply-To: <99B09243E1A5DA4898CDD8B7001114480BDADF1B1B@EXMB04.eu.tieto.com>

Hi Waldek,

On Wed, Dec 01, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
> >do we want this really as property? Wouldn't be configuration 
> >option be better to enable this. None of the other profiles 
> >has this anymore. 
> 
> Well, that is actually a proposal and I don't see any problems if we
> go for conf options in here. Even more, that would be if fact more
> usef-ull.  Is the main.conf a right place for say EnableSAP=true
> option ?

If the SAP support is all contained within a single plugin you probably
don't need a new option at all but can simply use the DisablePlugins
option for it.

Johan

^ permalink raw reply

* RE: [PATCH 1/2] Sim Access Profile API
From: Waldemar.Rymarkiewicz @ 2010-12-01 11:27 UTC (permalink / raw)
  To: johan.hedberg; +Cc: marcel, linux-bluetooth
In-Reply-To: <20101201112138.GA1855@jh-x301>

Hi Johan,=20

>-----Original Message-----
>From: Johan Hedberg [mailto:johan.hedberg@gmail.com]=20
>
>On Wed, Dec 01, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
>> >do we want this really as property? Wouldn't be=20
>configuration option=20
>> >be better to enable this. None of the other profiles has this=20
>> >anymore.
>>=20
>> Well, that is actually a proposal and I don't see any problems if we=20
>> go for conf options in here. Even more, that would be if fact more=20
>> usef-ull.  Is the main.conf a right place for say EnableSAP=3Dtrue=20
>> option ?
>
>If the SAP support is all contained within a single plugin you=20
>probably don't need a new option at all but can simply use the=20
>DisablePlugins option for it.
>

Ok. I see that.

/Waldek

^ permalink raw reply

* Re: [RFC 2/4] Bluetooth: clean up rfcomm code
From: Andrei Emeltchenko @ 2010-12-01 12:20 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <1291200356.4795.93.camel@aeonflux>

Hi Marcel,

On Wed, Dec 1, 2010 at 12:45 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andrei,
>
>> > * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-11-26 1=
7:22:43 +0200]:
>> >
>> >> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> >>
>> >> Remove extra spaces, assignments in if statement, zeroing static
>> >> variables.
>> >>
>> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> >> ---
>> >> =A0include/net/bluetooth/rfcomm.h | =A0 18 +++++++++---------
>> >> =A0net/bluetooth/rfcomm/core.c =A0 =A0| =A0 =A08 ++++----
>> >> =A0net/bluetooth/rfcomm/sock.c =A0 =A0| =A0 =A05 +++--
>> >> =A0net/bluetooth/rfcomm/tty.c =A0 =A0 | =A0 28 ++++++++++++++++------=
------
>> >> =A04 files changed, 32 insertions(+), 27 deletions(-)
>> >>
>> >> diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/r=
fcomm.h
>> >> index 71047bc..6eac4a7 100644
>> >> --- a/include/net/bluetooth/rfcomm.h
>> >> +++ b/include/net/bluetooth/rfcomm.h
>> >> @@ -1,5 +1,5 @@
>> >> -/*
>> >> - =A0 RFCOMM implementation for Linux Bluetooth stack (BlueZ).
>> >> +/*
>> >> + =A0 RFCOMM implementation for Linux Bluetooth stack (BlueZ)
>> >> =A0 =A0 Copyright (C) 2002 Maxim Krasnyansky <maxk@qualcomm.com>
>> >> =A0 =A0 Copyright (C) 2002 Marcel Holtmann <marcel@holtmann.org>
>> >>
>> >> @@ -11,13 +11,13 @@
>> >> =A0 =A0 OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF ME=
RCHANTABILITY,
>> >> =A0 =A0 FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD=
 PARTY RIGHTS.
>> >> =A0 =A0 IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LI=
ABLE FOR ANY
>> >> - =A0 CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY=
 DAMAGES
>> >> - =A0 WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER=
 IN AN
>> >> - =A0 ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISIN=
G OUT OF
>> >> + =A0 CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY=
 DAMAGES
>> >> + =A0 WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER=
 IN AN
>> >> + =A0 ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISIN=
G OUT OF
>> >> =A0 =A0 OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE=
.
>> >>
>> >> - =A0 ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATE=
NTS,
>> >> - =A0 COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
>> >> + =A0 ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATE=
NTS,
>> >> + =A0 COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
>> >> =A0 =A0 SOFTWARE IS DISCLAIMED.
>> >
>> > Marcel refused a patch from me in the past because its was touching le=
gal
>> > stuff, so or you remove these changes for your patches or wait for
>> > Marcel's comments here.
>>
>> I fixed extra spaces at the end of the sentences, no legal stuff
>> touched in legal terms :-)
>> I believe that it is better to have legal text identical to other
>> parts of the kernel otherwise
>> the legal stuff looks different when comparing with diff tools.
>
> make it a separate patch and not just fix it while at it.

OK, I'll do it this way.

Regards,
Andrei

^ permalink raw reply

* Re: [RFC 1/4] Bluetooth: clean up sco code
From: Andrei Emeltchenko @ 2010-12-01 13:44 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=_a1omXZdLXg_eEqDD7c0h=f0FgnTnxn1DvGDQ@mail.gmail.com>

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.

>
>>
>>        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;

-- 
Regards,
Andrei

^ permalink raw reply

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

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.

It will probably have more weight if the maintainer sends it than if I
send it ;)
-- 
Stefan Seyfried

"Any ideas, John?"
"Well, surrounding them's out."

^ permalink raw reply

* [PATCHv2 0/5] Clean up sco, rfcomm and hci code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Remove extra spaces, do not use assignments in "if" statements,
remove initialization to zero static vars, other errors
reported by checkpatch.
*v1 RFC
*v2 taken comments from Anderson, Marcel and Gustavo:
	- legal stuff moved to special patch
	- checking for do_inquiry before removing assignments

Andrei Emeltchenko (5):
  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

 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/hci_conn.c         |   23 ++++++++----
 net/bluetooth/hci_core.c         |   66 ++++++++++++++++++++++++--------------
 net/bluetooth/hci_event.c        |    8 +++--
 net/bluetooth/hci_sock.c         |   17 ++++++---
 net/bluetooth/l2cap.c            |    7 ++--
 net/bluetooth/rfcomm/core.c      |    8 ++--
 net/bluetooth/rfcomm/sock.c      |    5 ++-
 net/bluetooth/rfcomm/tty.c       |   28 +++++++++-------
 net/bluetooth/sco.c              |   22 +++++++-----
 14 files changed, 157 insertions(+), 117 deletions(-)


^ permalink raw reply

* [PATCHv2 1/5] Bluetooth: clean up sco code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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;
 
 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;
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 2/5] Bluetooth: clean up rfcomm code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 71047bc..2e78756 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -105,7 +105,7 @@
 struct rfcomm_hdr {
 	u8 addr;
 	u8 ctrl;
-	u8 len;    // Actual size can be 2 bytes
+	u8 len;    /* Actual size can be 2 bytes */
 } __packed;
 
 struct rfcomm_cmd {
@@ -228,7 +228,7 @@ struct rfcomm_dlc {
 /* ---- RFCOMM SEND RPN ---- */
 int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
 			u8 bit_rate, u8 data_bits, u8 stop_bits,
-			u8 parity, u8 flow_ctrl_settings, 
+			u8 parity, u8 flow_ctrl_settings,
 			u8 xon_char, u8 xoff_char, u16 param_mask);
 
 /* ---- RFCOMM DLCs (channels) ---- */
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index fa642aa..c1e2bba 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -41,7 +41,7 @@
 #include <linux/slab.h>
 
 #include <net/sock.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -51,10 +51,10 @@
 
 #define VERSION "1.11"
 
-static int disable_cfc = 0;
+static int disable_cfc;
+static int l2cap_ertm;
 static int channel_mtu = -1;
 static unsigned int l2cap_mtu = RFCOMM_MAX_L2CAP_MTU;
-static int l2cap_ertm = 0;
 
 static struct task_struct *rfcomm_thread;
 
@@ -1901,7 +1901,7 @@ static inline void rfcomm_check_connection(struct rfcomm_session *s)
 
 	BT_DBG("%p state %ld", s, s->state);
 
-	switch(sk->sk_state) {
+	switch (sk->sk_state) {
 	case BT_CONNECTED:
 		s->state = BT_CONNECT;
 
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 0207bd6..66cc1f0 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -45,7 +45,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>
@@ -888,7 +888,8 @@ static int rfcomm_sock_shutdown(struct socket *sock, int how)
 
 	BT_DBG("sock %p, sk %p", sock, sk);
 
-	if (!sk) return 0;
+	if (!sk)
+		return 0;
 
 	lock_sock(sk);
 	if (!sk->sk_shutdown) {
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a9b81f5..2575c2d 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -58,9 +58,9 @@ struct rfcomm_dev {
 
 	bdaddr_t		src;
 	bdaddr_t		dst;
-	u8 			channel;
+	u8			channel;
 
-	uint 			modem_status;
+	uint			modem_status;
 
 	struct rfcomm_dlc	*dlc;
 	struct tty_struct	*tty;
@@ -69,7 +69,7 @@ struct rfcomm_dev {
 
 	struct device		*tty_dev;
 
-	atomic_t 		wmem_alloc;
+	atomic_t		wmem_alloc;
 
 	struct sk_buff_head	pending;
 };
@@ -431,7 +431,8 @@ static int rfcomm_release_dev(void __user *arg)
 
 	BT_DBG("dev_id %d flags 0x%x", req.dev_id, req.flags);
 
-	if (!(dev = rfcomm_dev_get(req.dev_id)))
+	dev = rfcomm_dev_get(req.dev_id);
+	if (!dev)
 		return -ENODEV;
 
 	if (dev->flags != NOCAP_FLAGS && !capable(CAP_NET_ADMIN)) {
@@ -470,7 +471,8 @@ static int rfcomm_get_dev_list(void __user *arg)
 
 	size = sizeof(*dl) + dev_num * sizeof(*di);
 
-	if (!(dl = kmalloc(size, GFP_KERNEL)))
+	dl = kmalloc(size, GFP_KERNEL);
+	if (!dl)
 		return -ENOMEM;
 
 	di = dl->dev_info;
@@ -513,7 +515,8 @@ static int rfcomm_get_dev_info(void __user *arg)
 	if (copy_from_user(&di, arg, sizeof(di)))
 		return -EFAULT;
 
-	if (!(dev = rfcomm_dev_get(di.id)))
+	dev = rfcomm_dev_get(di.id);
+	if (!dev)
 		return -ENODEV;
 
 	di.flags   = dev->flags;
@@ -561,7 +564,8 @@ static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc, struct sk_buff *skb)
 		return;
 	}
 
-	if (!(tty = dev->tty) || !skb_queue_empty(&dev->pending)) {
+	tty = dev->tty;
+	if (!tty || !skb_queue_empty(&dev->pending)) {
 		skb_queue_tail(&dev->pending, skb);
 		return;
 	}
@@ -796,7 +800,8 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 
 		memcpy(skb_put(skb, size), buf + sent, size);
 
-		if ((err = rfcomm_dlc_send(dlc, skb)) < 0) {
+		err = rfcomm_dlc_send(dlc, skb);
+		if (err < 0) {
 			kfree_skb(skb);
 			break;
 		}
@@ -892,7 +897,7 @@ static void rfcomm_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 
 	/* Parity on/off and when on, odd/even */
 	if (((old->c_cflag & PARENB) != (new->c_cflag & PARENB)) ||
-			((old->c_cflag & PARODD) != (new->c_cflag & PARODD)) ) {
+			((old->c_cflag & PARODD) != (new->c_cflag & PARODD))) {
 		changes |= RFCOMM_RPN_PM_PARITY;
 		BT_DBG("Parity change detected.");
 	}
@@ -937,11 +942,10 @@ static void rfcomm_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 	/* POSIX does not support 1.5 stop bits and RFCOMM does not
 	 * support 2 stop bits. So a request for 2 stop bits gets
 	 * translated to 1.5 stop bits */
-	if (new->c_cflag & CSTOPB) {
+	if (new->c_cflag & CSTOPB)
 		stop_bits = RFCOMM_RPN_STOP_15;
-	} else {
+	else
 		stop_bits = RFCOMM_RPN_STOP_1;
-	}
 
 	/* Handle number of data bits [5-8] */
 	if ((old->c_cflag & CSIZE) != (new->c_cflag & CSIZE))
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 3/5] Bluetooth: clean up l2cap code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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;
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };
@@ -4162,11 +4162,10 @@ static inline void l2cap_data_channel_rrframe(struct sock *sk, u16 rx_control)
 			__mod_retrans_timer();
 
 		pi->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
-		if (pi->conn_state & L2CAP_CONN_SREJ_SENT) {
+		if (pi->conn_state & L2CAP_CONN_SREJ_SENT)
 			l2cap_send_ack(pi);
-		} else {
+		else
 			l2cap_ertm_send(sk);
-		}
 	}
 }
 
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 4/5] Bluetooth: clean up hci code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e30e008..e9527c5 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -489,7 +489,7 @@ struct hci_rp_read_local_name {
 
 #define HCI_OP_WRITE_PG_TIMEOUT		0x0c18
 
-#define HCI_OP_WRITE_SCAN_ENABLE 	0x0c1a
+#define HCI_OP_WRITE_SCAN_ENABLE	0x0c1a
 	#define SCAN_DISABLED		0x00
 	#define SCAN_INQUIRY		0x01
 	#define SCAN_PAGE		0x02
@@ -874,7 +874,7 @@ struct hci_ev_si_security {
 
 struct hci_command_hdr {
 	__le16	opcode;		/* OCF & OGF */
-	__u8 	plen;
+	__u8	plen;
 } __packed;
 
 struct hci_event_hdr {
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebec8c9..9c08625 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -44,15 +44,15 @@ struct inquiry_data {
 };
 
 struct inquiry_entry {
-	struct inquiry_entry 	*next;
+	struct inquiry_entry	*next;
 	__u32			timestamp;
 	struct inquiry_data	data;
 };
 
 struct inquiry_cache {
-	spinlock_t 		lock;
+	spinlock_t		lock;
 	__u32			timestamp;
-	struct inquiry_entry 	*list;
+	struct inquiry_entry	*list;
 };
 
 struct hci_conn_hash {
@@ -141,7 +141,7 @@ struct hci_dev {
 	void			*driver_data;
 	void			*core_data;
 
-	atomic_t 		promisc;
+	atomic_t		promisc;
 
 	struct dentry		*debugfs;
 
@@ -150,7 +150,7 @@ struct hci_dev {
 
 	struct rfkill		*rfkill;
 
-	struct module 		*owner;
+	struct module		*owner;
 
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
@@ -215,8 +215,8 @@ extern rwlock_t hci_dev_list_lock;
 extern rwlock_t hci_cb_list_lock;
 
 /* ----- Inquiry cache ----- */
-#define INQUIRY_CACHE_AGE_MAX   (HZ*30)   // 30 seconds
-#define INQUIRY_ENTRY_AGE_MAX   (HZ*60)   // 60 seconds
+#define INQUIRY_CACHE_AGE_MAX   (HZ*30)   /* 30 seconds */
+#define INQUIRY_ENTRY_AGE_MAX   (HZ*60)   /* 60 seconds */
 
 #define inquiry_cache_lock(c)		spin_lock(&c->lock)
 #define inquiry_cache_unlock(c)		spin_unlock(&c->lock)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0b1e460..6b90a41 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -39,7 +39,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -66,7 +66,8 @@ void hci_acl_connect(struct hci_conn *conn)
 	bacpy(&cp.bdaddr, &conn->dst);
 	cp.pscan_rep_mode = 0x02;
 
-	if ((ie = hci_inquiry_cache_lookup(hdev, &conn->dst))) {
+	ie = hci_inquiry_cache_lookup(hdev, &conn->dst);
+	if (ie) {
 		if (inquiry_entry_age(ie) <= INQUIRY_ENTRY_AGE_MAX) {
 			cp.pscan_rep_mode = ie->data.pscan_rep_mode;
 			cp.pscan_mode     = ie->data.pscan_mode;
@@ -368,8 +369,10 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 
 	BT_DBG("%s dst %s", hdev->name, batostr(dst));
 
-	if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
-		if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
+	acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
+	if (!acl) {
+		acl = hci_conn_add(hdev, ACL_LINK, dst);
+		if (!acl)
 			return NULL;
 	}
 
@@ -389,8 +392,10 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 	if (type == ACL_LINK)
 		return acl;
 
-	if (!(sco = hci_conn_hash_lookup_ba(hdev, type, dst))) {
-		if (!(sco = hci_conn_add(hdev, type, dst))) {
+	sco = hci_conn_hash_lookup_ba(hdev, type, dst);
+	if (!sco) {
+		sco = hci_conn_add(hdev, type, dst);
+		if (!sco) {
 			hci_conn_put(acl);
 			return NULL;
 		}
@@ -647,10 +652,12 @@ int hci_get_conn_list(void __user *arg)
 
 	size = sizeof(req) + req.conn_num * sizeof(*ci);
 
-	if (!(cl = kmalloc(size, GFP_KERNEL)))
+	cl = kmalloc(size, GFP_KERNEL);
+	if (!cl)
 		return -ENOMEM;
 
-	if (!(hdev = hci_dev_get(req.dev_id))) {
+	hdev = hci_dev_get(req.dev_id);
+	if (!hdev) {
 		kfree(cl);
 		return -ENODEV;
 	}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bc2a052..51c61f7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -44,7 +44,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -349,20 +349,23 @@ struct inquiry_entry *hci_inquiry_cache_lookup(struct hci_dev *hdev, bdaddr_t *b
 void hci_inquiry_cache_update(struct hci_dev *hdev, struct inquiry_data *data)
 {
 	struct inquiry_cache *cache = &hdev->inq_cache;
-	struct inquiry_entry *e;
+	struct inquiry_entry *ie;
 
 	BT_DBG("cache %p, %s", cache, batostr(&data->bdaddr));
 
-	if (!(e = hci_inquiry_cache_lookup(hdev, &data->bdaddr))) {
+	ie = hci_inquiry_cache_lookup(hdev, &data->bdaddr);
+	if (!ie) {
 		/* Entry not in the cache. Add new one. */
-		if (!(e = kzalloc(sizeof(struct inquiry_entry), GFP_ATOMIC)))
+		ie = kzalloc(sizeof(struct inquiry_entry), GFP_ATOMIC);
+		if (!ie)
 			return;
-		e->next     = cache->list;
-		cache->list = e;
+
+		ie->next = cache->list;
+		cache->list = ie;
 	}
 
-	memcpy(&e->data, data, sizeof(*data));
-	e->timestamp = jiffies;
+	memcpy(&ie->data, data, sizeof(*data));
+	ie->timestamp = jiffies;
 	cache->timestamp = jiffies;
 }
 
@@ -422,16 +425,20 @@ int hci_inquiry(void __user *arg)
 
 	hci_dev_lock_bh(hdev);
 	if (inquiry_cache_age(hdev) > INQUIRY_CACHE_AGE_MAX ||
-					inquiry_cache_empty(hdev) ||
-					ir.flags & IREQ_CACHE_FLUSH) {
+				inquiry_cache_empty(hdev) ||
+				ir.flags & IREQ_CACHE_FLUSH) {
 		inquiry_cache_flush(hdev);
 		do_inquiry = 1;
 	}
 	hci_dev_unlock_bh(hdev);
 
 	timeo = ir.length * msecs_to_jiffies(2000);
-	if (do_inquiry && (err = hci_request(hdev, hci_inq_req, (unsigned long)&ir, timeo)) < 0)
-		goto done;
+
+	if (do_inquiry) {
+		err = hci_request(hdev, hci_inq_req, (unsigned long)&ir, timeo);
+		if (err < 0)
+			goto done;
+	}
 
 	/* for unlimited number of responses we will use buffer with 255 entries */
 	max_rsp = (ir.num_rsp == 0) ? 255 : ir.num_rsp;
@@ -439,7 +446,8 @@ int hci_inquiry(void __user *arg)
 	/* cache_dump can't sleep. Therefore we allocate temp buffer and then
 	 * copy it to the user space.
 	 */
-	if (!(buf = kmalloc(sizeof(struct inquiry_info) * max_rsp, GFP_KERNEL))) {
+	buf = kmalloc(sizeof(struct inquiry_info) *max_rsp, GFP_KERNEL);
+	if (!buf) {
 		err = -ENOMEM;
 		goto done;
 	}
@@ -611,7 +619,8 @@ int hci_dev_close(__u16 dev)
 	struct hci_dev *hdev;
 	int err;
 
-	if (!(hdev = hci_dev_get(dev)))
+	hdev = hci_dev_get(dev);
+	if (!hdev)
 		return -ENODEV;
 	err = hci_dev_do_close(hdev);
 	hci_dev_put(hdev);
@@ -623,7 +632,8 @@ int hci_dev_reset(__u16 dev)
 	struct hci_dev *hdev;
 	int ret = 0;
 
-	if (!(hdev = hci_dev_get(dev)))
+	hdev = hci_dev_get(dev);
+	if (!hdev)
 		return -ENODEV;
 
 	hci_req_lock(hdev);
@@ -663,7 +673,8 @@ int hci_dev_reset_stat(__u16 dev)
 	struct hci_dev *hdev;
 	int ret = 0;
 
-	if (!(hdev = hci_dev_get(dev)))
+	hdev = hci_dev_get(dev);
+	if (!hdev)
 		return -ENODEV;
 
 	memset(&hdev->stat, 0, sizeof(struct hci_dev_stats));
@@ -682,7 +693,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 	if (copy_from_user(&dr, arg, sizeof(dr)))
 		return -EFAULT;
 
-	if (!(hdev = hci_dev_get(dr.dev_id)))
+	hdev = hci_dev_get(dr.dev_id);
+	if (!hdev)
 		return -ENODEV;
 
 	switch (cmd) {
@@ -763,7 +775,8 @@ int hci_get_dev_list(void __user *arg)
 
 	size = sizeof(*dl) + dev_num * sizeof(*dr);
 
-	if (!(dl = kzalloc(size, GFP_KERNEL)))
+	dl = kzalloc(size, GFP_KERNEL);
+	if (!dl)
 		return -ENOMEM;
 
 	dr = dl->dev_req;
@@ -797,7 +810,8 @@ int hci_get_dev_info(void __user *arg)
 	if (copy_from_user(&di, arg, sizeof(di)))
 		return -EFAULT;
 
-	if (!(hdev = hci_dev_get(di.dev_id)))
+	hdev = hci_dev_get(di.dev_id);
+	if (!hdev)
 		return -ENODEV;
 
 	strcpy(di.name, hdev->name);
@@ -905,7 +919,7 @@ int hci_register_dev(struct hci_dev *hdev)
 	hdev->sniff_max_interval = 800;
 	hdev->sniff_min_interval = 80;
 
-	tasklet_init(&hdev->cmd_task, hci_cmd_task,(unsigned long) hdev);
+	tasklet_init(&hdev->cmd_task, hci_cmd_task, (unsigned long) hdev);
 	tasklet_init(&hdev->rx_task, hci_rx_task, (unsigned long) hdev);
 	tasklet_init(&hdev->tx_task, hci_tx_task, (unsigned long) hdev);
 
@@ -1368,7 +1382,8 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
 	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
 	hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
 
-	if (!(list = skb_shinfo(skb)->frag_list)) {
+	list = skb_shinfo(skb)->frag_list;
+	if (!list) {
 		/* Non fragmented */
 		BT_DBG("%s nonfrag skb %p len %d", hdev->name, skb, skb->len);
 
@@ -1609,7 +1624,8 @@ static inline void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_conn_enter_active_mode(conn);
 
 		/* Send to upper protocol */
-		if ((hp = hci_proto[HCI_PROTO_L2CAP]) && hp->recv_acldata) {
+		hp = hci_proto[HCI_PROTO_L2CAP];
+		if (hp && hp->recv_acldata) {
 			hp->recv_acldata(conn, skb, flags);
 			return;
 		}
@@ -1644,7 +1660,8 @@ static inline void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		register struct hci_proto *hp;
 
 		/* Send to upper protocol */
-		if ((hp = hci_proto[HCI_PROTO_SCO]) && hp->recv_scodata) {
+		hp = hci_proto[HCI_PROTO_SCO];
+		if (hp && hp->recv_scodata) {
 			hp->recv_scodata(conn, skb);
 			return;
 		}
@@ -1727,7 +1744,8 @@ static void hci_cmd_task(unsigned long arg)
 	if (atomic_read(&hdev->cmd_cnt) && (skb = skb_dequeue(&hdev->cmd_q))) {
 		kfree_skb(hdev->sent_cmd);
 
-		if ((hdev->sent_cmd = skb_clone(skb, GFP_ATOMIC))) {
+		hdev->sent_cmd = skb_clone(skb, GFP_ATOMIC);
+		if (hdev->sent_cmd) {
 			atomic_dec(&hdev->cmd_cnt);
 			hci_send_frame(skb);
 			hdev->cmd_last_tx = jiffies;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3c1957c..8923b36 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -39,7 +39,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -1512,10 +1512,12 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
 			conn->sent -= count;
 
 			if (conn->type == ACL_LINK) {
-				if ((hdev->acl_cnt += count) > hdev->acl_pkts)
+				hdev->acl_cnt += count;
+				if (hdev->acl_cnt > hdev->acl_pkts)
 					hdev->acl_cnt = hdev->acl_pkts;
 			} else {
-				if ((hdev->sco_cnt += count) > hdev->sco_pkts)
+				hdev->sco_cnt += count;
+				if (hdev->sco_cnt > hdev->sco_pkts)
 					hdev->sco_cnt = hdev->sco_pkts;
 			}
 		}
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 83acd16..b3753ba 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -43,7 +43,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -125,7 +125,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 				continue;
 		}
 
-		if (!(nskb = skb_clone(skb, GFP_ATOMIC)))
+		nskb = skb_clone(skb, GFP_ATOMIC);
+		if (!nskb)
 			continue;
 
 		/* Put type byte before the data */
@@ -370,7 +371,8 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
 	}
 
 	if (haddr->hci_dev != HCI_DEV_NONE) {
-		if (!(hdev = hci_dev_get(haddr->hci_dev))) {
+		hdev = hci_dev_get(haddr->hci_dev);
+		if (!hdev) {
 			err = -ENODEV;
 			goto done;
 		}
@@ -457,7 +459,8 @@ static int hci_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 	if (sk->sk_state == BT_CLOSED)
 		return 0;
 
-	if (!(skb = skb_recv_datagram(sk, flags, noblock, &err)))
+	skb = skb_recv_datagram(sk, flags, noblock, &err);
+	if (!skb)
 		return err;
 
 	msg->msg_namelen = 0;
@@ -499,7 +502,8 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	lock_sock(sk);
 
-	if (!(hdev = hci_pi(sk)->hdev)) {
+	hdev = hci_pi(sk)->hdev;
+	if (!hdev) {
 		err = -EBADFD;
 		goto done;
 	}
@@ -509,7 +513,8 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 		goto done;
 	}
 
-	if (!(skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err)))
+	skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
+	if (!skb)
 		goto done;
 
 	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 5/5] Bluetooth: clean up legal text
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e9527c5..f3c5ed6 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
 
@@ -12,13 +12,13 @@
    OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
    IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
-   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
-   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
-   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
-   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 217bb91..7ad25ca 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
    Copyright (C) 2009-2010 Gustavo F. Padovan <gustavo@padovan.org>
@@ -14,13 +14,13 @@
    OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
    IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
-   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
-   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
-   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
-   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 2e78756..6eac4a7 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -1,5 +1,5 @@
-/* 
-   RFCOMM implementation for Linux Bluetooth stack (BlueZ).
+/*
+   RFCOMM implementation for Linux Bluetooth stack (BlueZ)
    Copyright (C) 2002 Maxim Krasnyansky <maxk@qualcomm.com>
    Copyright (C) 2002 Marcel Holtmann <marcel@holtmann.org>
 
@@ -11,13 +11,13 @@
    OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
    IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
-   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
-   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
-   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
-   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index ea5c664..1e35c43 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
 
@@ -12,13 +12,13 @@
    OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
    IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
-   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
-   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
-   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
-   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 0/3] Basic attribute permission support
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

This patchset adds initial support for attribute permission checks. Currently,
only access and authentication permissions are checked. Authorization
permissions require integration with the BlueZ agent, which is not implemented
yet.

There are some pending issues necessary for a minimum complete attribute
permission support (all of them are being worked on):

* The attribute client, upon receiving the "Insufficient Encryption" error,
  shall increase the security level and resend the failed request.
* The attribute server shall verify the connection permissions on each ATT
  request, and not just once on connection callback.
* On kernel side, increasing the security level (using setsockopt()) shall
  trigger SMP negotiation for a LE connection, blocking next socket I/O until
  negotiation is finished.
* On BR/EDR, link encryption needs to be done "on the fly" before resending the
  failed ATT request.

Albeit the above issues, we believe these patches are ready for commit.

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


^ permalink raw reply

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

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

* 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

* 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


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