Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] Changed obex_service_driver put() signature.
From: Johan Hedberg @ 2010-12-08 15:18 UTC (permalink / raw)
  To: Slawomir Bochenski; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikzUraZ9ac5qf9ePG8YKo0Sx0B2YZoKz8HoRTmn@mail.gmail.com>

Hi,

On Wed, Dec 08, 2010, Slawomir Bochenski wrote:
> This API change is needed for implementing Message Access Profile, as
> MAP uses application parameters in puts also (for example
> SetMessageStatus function, see MAP specification, chapter 5.7) and in
> order to access application parameters headers access to obex object
> is required.
> 
> ---
>  plugins/ftp.c           |    3 ++-
>  plugins/opp.c           |    3 ++-
>  plugins/syncevolution.c |    3 ++-
>  src/obex.c              |    2 +-
>  src/service.h           |    3 ++-
>  5 files changed, 9 insertions(+), 5 deletions(-)

The patch looks good, but I can't apply it with git am. I get the
following:

fatal: corrupt patch at line 14
Patch failed at 0001 Changed obex_service_driver put() signature.

Could you try recreating it with git format-patch and resending with git
send-email? Thanks.

Btw, summary line should start with "Change ..." instead of "Changed
..." and it shouldn't end with a .

Johan

^ permalink raw reply

* Re: [PATCH 1/3] Add support for stat files bigger than 2GB on 32-bit systems
From: Johan Hedberg @ 2010-12-08 15:15 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1291821222-18990-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Wed, Dec 08, 2010, Luiz Augusto von Dentz wrote:
> From stat documentation:
> 
> "(stat())  path  refers to a file whose size cannot be represented in the
> type off_t.  This can occur when an application compiled on a 32-bit
> platform without -D_FILE_OFFSET_BITS=64 calls stat() on a file whose size
> exceeds (2<<31)-1 bits."
> 
> To fix this now size header is omitted when the file is over 32-bit, but
> it is able to transfer it by using 64-bit variables. In addition to that
> folder-listing now should report such big sizes properly.
> ---
>  acinclude.m4         |    2 +-
>  client/transfer.c    |    5 +++--
>  plugins/filesystem.c |    7 ++++---
>  src/obex-priv.h      |    6 +++---
>  src/obex.c           |    7 ++++---
>  5 files changed, 15 insertions(+), 12 deletions(-)

Thanks. The patch has been pushed upstream.

Johan

^ permalink raw reply

* [PATCH 1/3] Add support for stat files bigger than 2GB on 32-bit systems
From: Luiz Augusto von Dentz @ 2010-12-08 15:13 UTC (permalink / raw)
  To: linux-bluetooth

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

>From stat documentation:

"(stat())  path  refers to a file whose size cannot be represented in the
type off_t.  This can occur when an application compiled on a 32-bit
platform without -D_FILE_OFFSET_BITS=64 calls stat() on a file whose size
exceeds (2<<31)-1 bits."

To fix this now size header is omitted when the file is over 32-bit, but
it is able to transfer it by using 64-bit variables. In addition to that
folder-listing now should report such big sizes properly.
---
 acinclude.m4         |    2 +-
 client/transfer.c    |    5 +++--
 plugins/filesystem.c |    7 ++++---
 src/obex-priv.h      |    6 +++---
 src/obex.c           |    7 ++++---
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 4594073..3d1f511 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -12,7 +12,7 @@ AC_DEFUN([AC_PROG_CC_PIE], [
 
 AC_DEFUN([COMPILER_FLAGS], [
 	if (test "${CFLAGS}" = ""); then
-		CFLAGS="-Wall -O2 -D_FORTIFY_SOURCE=2"
+		CFLAGS="-Wall -O2 -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64"
 	fi
 	if (test "$USE_MAINTAINER_MODE" = "yes"); then
 		CFLAGS+=" -Werror -Wextra"
diff --git a/client/transfer.c b/client/transfer.c
index 6eec513..fdcaa46 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -470,7 +470,7 @@ int transfer_put(struct transfer_data *transfer, transfer_callback_t func,
 	struct session_data *session = transfer->session;
 	gw_obex_xfer_cb_t cb;
 	struct stat st;
-	int fd;
+	int fd, size;
 
 	if (transfer->xfer != NULL)
 		return -EALREADY;
@@ -497,8 +497,9 @@ int transfer_put(struct transfer_data *transfer, transfer_callback_t func,
 	cb = put_xfer_progress;
 
 done:
+	size = transfer->size < UINT32_MAX ? transfer->size : 0;
 	transfer->xfer = gw_obex_put_async(session->obex, transfer->name,
-						transfer->type, transfer->size,
+						transfer->type, size,
 						-1, NULL);
 	if (transfer->xfer == NULL)
 		return -ENOTCONN;
diff --git a/plugins/filesystem.c b/plugins/filesystem.c
index bb758ab..b4ff556 100644
--- a/plugins/filesystem.c
+++ b/plugins/filesystem.c
@@ -66,7 +66,7 @@
 
 #define FL_PARENT_FOLDER_ELEMENT "<parent-folder/>" EOL_CHARS
 
-#define FL_FILE_ELEMENT "<file name=\"%s\" size=\"%lu\"" \
+#define FL_FILE_ELEMENT "<file name=\"%s\" size=\"%" PRIu64 "\"" \
 			" %s accessed=\"%s\" " \
 			"modified=\"%s\" created=\"%s\"/>" EOL_CHARS
 
@@ -121,8 +121,9 @@ static char *file_stat_line(char *filename, struct stat *fstat,
 			ret = g_strdup_printf(FL_FOLDER_ELEMENT, escaped, perm,
 							atime, mtime, ctime);
 	} else if (S_ISREG(fstat->st_mode))
-		ret = g_strdup_printf(FL_FILE_ELEMENT, escaped, fstat->st_size,
-						perm, atime, mtime, ctime);
+		ret = g_strdup_printf(FL_FILE_ELEMENT, escaped,
+					(uint64_t) fstat->st_size,
+					perm, atime, mtime, ctime);
 
 	g_free(escaped);
 
diff --git a/src/obex-priv.h b/src/obex-priv.h
index c9be1c5..0806a56 100644
--- a/src/obex-priv.h
+++ b/src/obex-priv.h
@@ -33,9 +33,9 @@ struct obex_session {
 	char *path;
 	time_t time;
 	uint8_t *buf;
-	int32_t pending;
-	int32_t offset;
-	int32_t size;
+	int64_t pending;
+	int64_t offset;
+	int64_t size;
 	void *object;
 	gboolean aborted;
 	struct obex_service_driver *service;
diff --git a/src/obex.c b/src/obex.c
index 6d4430d..effb81e 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -590,7 +590,8 @@ static int obex_read_stream(struct obex_session *os, obex_t *obex,
 
 	/* only write if both object and driver are valid */
 	if (os->object == NULL || os->driver == NULL) {
-		DBG("Stored %u bytes into temporary buffer", os->pending);
+		DBG("Stored %" PRIu64 " bytes into temporary buffer",
+								os->pending);
 		return 0;
 	}
 
@@ -796,7 +797,7 @@ static void cmd_get(struct obex_session *os, obex_t *obex, obex_object_t *obj)
 	if (err < 0)
 		goto done;
 
-	if (os->size != OBJECT_SIZE_UNKNOWN) {
+	if (os->size != OBJECT_SIZE_UNKNOWN && os->size < UINT32_MAX) {
 		hd.bq4 = os->size;
 		OBEX_ObjectAddHeader(obex, obj,
 				OBEX_HDR_LENGTH, hd, 4, 0);
@@ -1005,7 +1006,7 @@ static gboolean check_put(obex_t *obex, obex_object_t *obj)
 
 		case OBEX_HDR_LENGTH:
 			os->size = hd.bq4;
-			DBG("OBEX_HDR_LENGTH: %d", os->size);
+			DBG("OBEX_HDR_LENGTH: %" PRIu64, os->size);
 			break;
 		case OBEX_HDR_TIME:
 			os->time = parse_iso8610((const char *) hd.bs, hlen);
-- 
1.7.1


^ permalink raw reply related

* [PATCH] Changed obex_service_driver put() signature.
From: Slawomir Bochenski @ 2010-12-08 14:50 UTC (permalink / raw)
  To: linux-bluetooth

This API change is needed for implementing Message Access Profile, as
MAP uses application parameters in puts also (for example
SetMessageStatus function, see MAP specification, chapter 5.7) and in
order to access application parameters headers access to obex object
is required.

---
 plugins/ftp.c           |    3 ++-
 plugins/opp.c           |    3 ++-
 plugins/syncevolution.c |    3 ++-
 src/obex.c              |    2 +-
 src/service.h           |    3 ++-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/plugins/ftp.c b/plugins/ftp.c
index 91c77a3..a952f64 100644
--- a/plugins/ftp.c
+++ b/plugins/ftp.c
@@ -258,7 +258,8 @@ static int ftp_chkput(struct obex_session *os,
void *user_data)
 	return ret;
 }

-static int ftp_put(struct obex_session *os, void *user_data)
+static int ftp_put(struct obex_session *os, obex_object_t *obj,
+						void *user_data)
 {
 	struct ftp_session *ftp = user_data;
 	const char *name = obex_get_name(os);
diff --git a/plugins/opp.c b/plugins/opp.c
index 05f944f..17c4356 100644
--- a/plugins/opp.c
+++ b/plugins/opp.c
@@ -155,7 +155,8 @@ skip_auth:
 	return ret;
 }

-static int opp_put(struct obex_session *os, void *user_data)
+static int opp_put(struct obex_session *os, obex_object_t *obj,
+						void *user_data)
 {
 	const char *name = obex_get_name(os);
 	const char *folder = obex_get_root_folder(os);
diff --git a/plugins/syncevolution.c b/plugins/syncevolution.c
index bf436a9..8041df6 100644
--- a/plugins/syncevolution.c
+++ b/plugins/syncevolution.c
@@ -243,7 +243,8 @@ failed:
 	return NULL;
 }

-static int synce_put(struct obex_session *os, void *user_data)
+static int synce_put(struct obex_session *os, obex_object_t *obj,
+					void *user_data)
 {
 	return 0;
 }
diff --git a/src/obex.c b/src/obex.c
index 6d4430d..1649cd0 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -1089,7 +1089,7 @@ static void cmd_put(struct obex_session *os,
obex_t *obex, obex_object_t *obj)
 		return;
 	}

-	err = os->service->put(os, os->service_data);
+	err = os->service->put(os, obj, os->service_data);
 	if (err < 0)
 		os_set_response(obj, err);
 }
diff --git a/src/service.h b/src/service.h
index 1726c43..a844885 100644
--- a/src/service.h
+++ b/src/service.h
@@ -34,7 +34,8 @@ struct obex_service_driver {
 	void (*progress) (struct obex_session *os, void *user_data);
 	int (*get) (struct obex_session *os, obex_object_t *obj,
 			gboolean *stream, void *user_data);
-	int (*put) (struct obex_session *os, void *user_data);
+	int (*put) (struct obex_session *os, obex_object_t *obj,
+			void *user_data);
 	int (*chkput) (struct obex_session *os, void *user_data);
 	int (*setpath) (struct obex_session *os, obex_object_t *obj,
 							void *user_data);


-- 
Slawomir Bochenski

^ permalink raw reply related

* Re: [PATCH v2] Fix regression causing crash in 3-way calling
From: Johan Hedberg @ 2010-12-08 12:54 UTC (permalink / raw)
  To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1291809471-806-2-git-send-email-dmitriy.paliy@nokia.com>

Hi Dmitriy,

On Wed, Dec 08, 2010, Dmitriy Paliy wrote:
> Fix obexd crash in 3-way calling scenario. Crash happens when there
> is redialed second incoming call. Cache for the PBAP session is
> already created at that moment, but PBAP object is destroyed. Crash
> happens when object is dereferenced in vobject_list_open.
> 
> Therefore, PBAP object has to be created before any attempt to write
> cached data to buffer associated to this object.
> 
> However, cache_ready_notify function, which is invoked in
> vobject_vcard_open for valid cache case, sends also PBAP object data
> via callback function to obex.c and written to OBEX stream as GET
> response in handle_async_io handler function.
> 
> A new response is sent to OBEX stream after cache_ready_notify exists
> to vobject_list_open function, which is callback function for
> obex_mime_type_driver. Such leads to undefined befavior. Therefore,
> cache_ready_notify is splitted in two cache_ready_notify and
> generate_response functions.
> 
> generate_response fills data to buffer and returns error, if any,
> while cache_ready_notify notifies OBEX core to write this data to
> stream.
> 
> In order to avoid writing to stream twice, cache_ready_notify is
> replaced by generate_response in vobject_list_open. As a result,
> PBAP buffer data is generated from existing cache and sent to
> stream upon start of OBEX stream after vobject_list_open exits.
> ---
>  plugins/pbap.c |   98 +++++++++++++++++++++++++++++++-------------------------
>  1 files changed, 54 insertions(+), 44 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-12-08 12:51 UTC (permalink / raw)
  To: Par-Gunnar Hjalmdahl
  Cc: Pavan Savoy, Vitaly Wool, Alan Cox, linus.walleij,
	linux-bluetooth, linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTi=B4wT5SbN2qyyO5RZd7qMnkZXHpB6d=RTQeRtX@mail.gmail.com>

On Wednesday 08 December 2010, Par-Gunnar Hjalmdahl wrote:
> > Marcel did previously suggest a bus-driver sort of a structure, where
> > the transport are sort of adapter drivers, the data interpretation
> > logic like splitting HCI-events, FM CH-8 packets all fall into the
> > algos drivers and the actual Bluetooth driver or GPS or FM-V4L2
> > drivers falling into the client/protocol drivers....
> 
> What the cg2900 driver has now turned into is almost like the
> cg2900_core acting as a bus, identifying which chip is connected and
> choosing correct chip driver from this. This cg2900_core module should
> not contain any vendor specific code. When identified, the chip driver
> then uses MFD to setup channels according to the functionality it
> supports (one MFD device per H4 channel). It's hard to explain
> everything of the new architecture here. I would like you to check the
> new patch set I'm trying hard to create at the moment. I will try to
> get it out on Friday, but it's hard to promise anything (there is a
> lot of work to do with it). There you could then see if it is
> something you could consider useful.

It sounds really good, I'm looking forward to your new patch set!
If it's generic enough to replace the ti-st code, that would be a
clear sign that you are on the right track ;-)

I'm not asking to fix their code yourself, but it might be good to
look at it and see if it would work.

	Arnd

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Par-Gunnar Hjalmdahl @ 2010-12-08 12:21 UTC (permalink / raw)
  To: Pavan Savoy
  Cc: Arnd Bergmann, Vitaly Wool, Alan Cox, linus.walleij,
	linux-bluetooth, linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTimCKK9NAiXD4wsCkJt8bsy_exYEBWdb1+M6+NOE@mail.gmail.com>

Hi Pavan et al,

2010/12/8 Pavan Savoy <pavan_savoy@sify.com>:
> On Tue, Dec 7, 2010 at 4:37 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Monday 06 December 2010 22:24:34 Vitaly Wool wrote:
>>> On Mon, Dec 6, 2010 at 5:54 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>
>>> >> But I was trying to make a different point here. On a basic level,
>>> >> there's this cg2000 chip from STE that does BT, FM and GPS. There's
>>> >> the chip from TI that does BT, FM and GPS, and there's the Broadcom
>>> >> chip that does BT+FM. They all use HCI to access the other functions
>>> >> of the combo chip and they do it in a really simiar way, with the
>>> >> differences mostly in power management techniques. So I think it's
>>> >> quite sensible to have some kind of framework that is suitable for
>>> >> such devices.
>>> >
>>> > Yes, I agree 100% in principle. I could not find the code that
>>> > Broadcom/TI FM and GPS stuff so far, can you point us to that?
>>>
>>> Sure, the TI "shared transport" code is mostly at drivers/misc/ti-st.
>>>
>>> Some Broadcom BCM43xx chips work in a similar way AFAIK but I'm not
>>> sure about the mainline support for those.
>>
>> Ah, it had not occured to me to look in drivers/misc. Looking at the
>> code over there, it becomes much clearer what your point is. Evidently
>> the ti-st driver implements a line discipline similar to, but conflictin=
g
>> with hci_ldisc, and it has its own copy of the hci_ll code.
>>
>> I guess this is exactly what we're trying to avoid having with the
>> cg2900 code ;-).
>>
>>> > The cg2900 solution for this was to use MFD (plus another layer
>>> > in the posted version, but that will go away I assume). Using
>>> > MFD is not the only possibility here, but I could not see anything
>>> > wrong with it either. Do you think we can move them all over to
>>> > use MFD infrastructure?
>>>
>>> I guess so but it's probably more of a detail than what I'm up to now :=
)
>>
>> Agreed.
>>
>>> >> But generally speaking, isn't a line discipline/driver attached to a
>>> >> tty? We can use dumb tty for e. g. SPI and still be able to use
>>> >> hci_ll, right?
>>> >
>>> > I suggested that as well, but the point was made that this would
>>> > add an unnecessary indirection for the SPI case, which is not
>>> > really much like a serial port. It's certainly possible to do it
>>> > like you say, but if we add a way to register the high-level
>>> > protocols with an HCI-like multi-function device, we could
>>> > also do it in a way that does not rely on tty-ldisc but keeps it
>>> > as one of the options.
>>>
>>> I actually don't think it's such a big indirection, I prefer to think
>>> of it more as of the abstraction layer. If not use this, are we going
>>> to have direct SPI device drivers? I'm afraid we might end up with a
>>> huge duplication of code in that case.
>>
>> The question is how it should be modeled in a better way than today.
>>
>> I believe we already agree it should be something like
>>
>> =A0 bt =A0 ti-radio =A0 =A0st-e-radio =A0 =A0ti-gps =A0 st-e-gps =A0broa=
dcom-radio ...
>> =A0 | =A0 =A0 =A0 =A0 | =A0 =A0 =A0 =A0 =A0| =A0 =A0 =A0 =A0 =A0 =A0| =
=A0 =A0 =A0 =A0 =A0| =A0 =A0 =A0 =A0 =A0| =A0 =A0 =A0 =A0 |
>> =A0 +---------+----------+---------+--+----------+----------+---------+
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0common-hci-module
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+-----------+-----------+
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A0 =A0 =A0| =A0 =A0 =
=A0 | =A0 =A0 =A0|
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0serial =A0 =A0spi =A0 =A0 i2c =A0=
 =A0...
>
> Yes, this sort of architecture would certainly help...
> However, I suppose the most common question as one of you stated above
> was how can the channel -ID and other factors such as offset-header
> packet format be generalized?
>
> As in over here, will the common-hci-module work fine, If say ti-radio
> says he is expecting packets starting from id-8 which is of length 10?
> and also work for st-e-radio which would say expecting data from id-6
> with 15 max bytes?
> Answering this I suppose would solve a lot of our problems....
>
> Marcel did previously suggest a bus-driver sort of a structure, where
> the transport are sort of adapter drivers, the data interpretation
> logic like splitting HCI-events, FM CH-8 packets all fall into the
> algos drivers and the actual Bluetooth driver or GPS or FM-V4L2
> drivers falling into the client/protocol drivers....
>

What the cg2900 driver has now turned into is almost like the
cg2900_core acting as a bus, identifying which chip is connected and
choosing correct chip driver from this. This cg2900_core module should
not contain any vendor specific code. When identified, the chip driver
then uses MFD to setup channels according to the functionality it
supports (one MFD device per H4 channel). It's hard to explain
everything of the new architecture here. I would like you to check the
new patch set I'm trying hard to create at the moment. I will try to
get it out on Friday, but it's hard to promise anything (there is a
lot of work to do with it). There you could then see if it is
something you could consider useful.

/P-G

>
>
>> Any objections to this?
>>
>> If you agree, I think we should discuss the alternatives for the common
>> module. I think you are saying we should make the common module a single
>> ldisc doing the multiplexing and have all transports register as ttys in=
to
>> it, right?
>>
>> What we discussed before was to split it into multiple modules, one for
>> the tty ldisc and one for the common code, and then have the spi/i2c/...
>> drivers feed into the common multiplexer without going through tty.
>>
>> I don't currently see a significant advantage or disadvantage with eithe=
r
>> approach.
>>
>> =A0 =A0 =A0 =A0Arnd
>> --
>> 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 =A0http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* [PATCH v2] Fix regression causing crash in 3-way calling
From: Dmitriy Paliy @ 2010-12-08 11:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1291809471-806-1-git-send-email-dmitriy.paliy@nokia.com>

Fix obexd crash in 3-way calling scenario. Crash happens when there
is redialed second incoming call. Cache for the PBAP session is
already created at that moment, but PBAP object is destroyed. Crash
happens when object is dereferenced in vobject_list_open.

Therefore, PBAP object has to be created before any attempt to write
cached data to buffer associated to this object.

However, cache_ready_notify function, which is invoked in
vobject_vcard_open for valid cache case, sends also PBAP object data
via callback function to obex.c and written to OBEX stream as GET
response in handle_async_io handler function.

A new response is sent to OBEX stream after cache_ready_notify exists
to vobject_list_open function, which is callback function for
obex_mime_type_driver. Such leads to undefined befavior. Therefore,
cache_ready_notify is splitted in two cache_ready_notify and
generate_response functions.

generate_response fills data to buffer and returns error, if any,
while cache_ready_notify notifies OBEX core to write this data to
stream.

In order to avoid writing to stream twice, cache_ready_notify is
replaced by generate_response in vobject_list_open. As a result,
PBAP buffer data is generated from existing cache and sent to
stream upon start of OBEX stream after vobject_list_open exits.
---
 plugins/pbap.c |   98 +++++++++++++++++++++++++++++++-------------------------
 1 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 1a7d001..4086227 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -378,7 +378,7 @@ static GSList *sort_entries(GSList *l, uint8_t order, uint8_t search_attrib,
 	return sorted;
 }
 
-static void cache_ready_notify(void *user_data)
+static int generate_response(void *user_data)
 {
 	struct pbap_session *pbap = user_data;
 	GSList *sorted;
@@ -398,7 +398,8 @@ static void cache_ready_notify(void *user_data)
 		memcpy(hdr->val, &size, sizeof(size));
 
 		pbap->obj->buffer = g_string_new_len(aparam, sizeof(aparam));
-		goto done;
+
+		return 0;
 	}
 
 	/*
@@ -409,11 +410,8 @@ static void cache_ready_notify(void *user_data)
 			pbap->params->searchattrib,
 			(const char *) pbap->params->searchval);
 
-	if (sorted == NULL) {
-		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
-		return;
-	}
+	if (sorted == NULL)
+		return -ENOENT;
 
 	/* Computing offset considering first entry of the phonebook */
 	l = g_slist_nth(sorted, pbap->params->liststartoffset);
@@ -430,11 +428,25 @@ static void cache_ready_notify(void *user_data)
 
 	g_slist_free(sorted);
 
-done:
-	if (!pbap->cache.valid) {
-		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
+	return 0;
+}
+
+static void cache_ready_notify(void *user_data)
+{
+	struct pbap_session *pbap = user_data;
+	int err;
+
+	DBG("");
+
+	pbap->cache.valid = TRUE;
+
+	err = generate_response(pbap);
+	if (err < 0) {
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, err);
+		return;
 	}
+
+	obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 }
 
 static void cache_entry_done(void *user_data)
@@ -746,10 +758,28 @@ fail:
 	return NULL;
 }
 
+static int vobject_close(void *object)
+{
+	struct pbap_object *obj = object;
+
+	DBG("");
+
+	if (obj->session)
+		obj->session->obj = NULL;
+
+	if (obj->buffer)
+		g_string_free(obj->buffer, TRUE);
+
+	g_free(obj);
+
+	return 0;
+}
+
 static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 				void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
+	struct pbap_object *obj = NULL;
 	int ret;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
@@ -766,30 +796,25 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 
 	/* PullvCardListing always get the contacts from the cache */
 
-	if (pbap->cache.valid) {
-		/*
-		 * Valid cache and empty buffer mean that cache was already
-		 * created within a single session, but no data is available.
-		 */
-		if (!pbap->obj->buffer) {
-			ret = -ENOENT;
-			goto fail;
-		}
-
-		cache_ready_notify(pbap);
-		goto done;
-	}
-
-	ret = phonebook_create_cache(name,
-		cache_entry_notify, cache_ready_notify, pbap);
+	obj = vobject_create(pbap);
 
+	if (pbap->cache.valid)
+		ret = generate_response(pbap);
+	else
+		ret = phonebook_create_cache(name, cache_entry_notify,
+						cache_ready_notify, pbap);
 	if (ret < 0)
 		goto fail;
 
-done:
-	return vobject_create(pbap);
+	if (err)
+		*err = 0;
+
+	return obj;
 
 fail:
+	if (obj)
+		vobject_close(obj);
+
 	if (err)
 		*err = ret;
 
@@ -902,21 +927,6 @@ static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
 	return string_read(obj->buffer, buf, count);
 }
 
-static int vobject_close(void *object)
-{
-	struct pbap_object *obj = object;
-
-	if (obj->session)
-		obj->session->obj = NULL;
-
-	if (obj->buffer)
-		g_string_free(obj->buffer, TRUE);
-
-	g_free(obj);
-
-	return 0;
-}
-
 static struct obex_mime_type_driver mime_pull = {
 	.target = PBAP_TARGET,
 	.target_size = TARGET_SIZE,
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 0/1 v2] Fix regression causing crash in 3-way calling
From: Dmitriy Paliy @ 2010-12-08 11:57 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

More detailed commit message explaining the problem and
justification of changes, including refactoring of the code,
are written in this version of submitted earlier patch.

Also, label is removed in generate_response function.

Thanks,
Dmitriy


^ permalink raw reply

* [PATCH v2] Fix regression causing crash in 3-way calling
From: Dmitriy Paliy @ 2010-12-08 11:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1291808923-440-1-git-send-email-dmitriy.paliy@nokia.com>

Fix obexd crash in 3-way calling scenario. Crash happens when there
is redialed second incoming call. Cache for the PBAP session is
already created at that moment, but PBAP object is destroyed. Crash
happens when object is dereferenced in vobject_list_open.

Therefore, PBAP object has to be created before any attempt to write
cached data to buffer associated to this object.

However, cache_ready_notify function, which is invoked in
vobject_vcard_open for valid cache case, sends also PBAP object data
via callback function to obex.c and written to OBEX stream as GET
response in handle_async_io handler function.

A new response is sent to OBEX stream after cache_ready_notify exists
to vobject_list_open function, which is callback function for
obex_mime_type_driver. Such leads to undefined befavior. Therefore,
cache_ready_notify is splitted in two cache_ready_notify and
generate_response functions.

generate_response fills data to buffer and returns error, if any,
while cache_ready_notify notifies OBEX core to write this data to
stream.

In order to avoid writing to stream twice, cache_ready_notify is
replaced by generate_response in vobject_list_open. As a result,
PBAP buffer data is generated from existing cache and sent to
stream upon start of OBEX stream after vobject_list_open exits.
---
 plugins/pbap.c |   98 +++++++++++++++++++++++++++++++-------------------------
 1 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 1a7d001..4086227 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -378,7 +378,7 @@ static GSList *sort_entries(GSList *l, uint8_t order, uint8_t search_attrib,
 	return sorted;
 }
 
-static void cache_ready_notify(void *user_data)
+static int generate_response(void *user_data)
 {
 	struct pbap_session *pbap = user_data;
 	GSList *sorted;
@@ -398,7 +398,8 @@ static void cache_ready_notify(void *user_data)
 		memcpy(hdr->val, &size, sizeof(size));
 
 		pbap->obj->buffer = g_string_new_len(aparam, sizeof(aparam));
-		goto done;
+
+		return 0;
 	}
 
 	/*
@@ -409,11 +410,8 @@ static void cache_ready_notify(void *user_data)
 			pbap->params->searchattrib,
 			(const char *) pbap->params->searchval);
 
-	if (sorted == NULL) {
-		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
-		return;
-	}
+	if (sorted == NULL)
+		return -ENOENT;
 
 	/* Computing offset considering first entry of the phonebook */
 	l = g_slist_nth(sorted, pbap->params->liststartoffset);
@@ -430,11 +428,25 @@ static void cache_ready_notify(void *user_data)
 
 	g_slist_free(sorted);
 
-done:
-	if (!pbap->cache.valid) {
-		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
+	return 0;
+}
+
+static void cache_ready_notify(void *user_data)
+{
+	struct pbap_session *pbap = user_data;
+	int err;
+
+	DBG("");
+
+	pbap->cache.valid = TRUE;
+
+	err = generate_response(pbap);
+	if (err < 0) {
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, err);
+		return;
 	}
+
+	obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 }
 
 static void cache_entry_done(void *user_data)
@@ -746,10 +758,28 @@ fail:
 	return NULL;
 }
 
+static int vobject_close(void *object)
+{
+	struct pbap_object *obj = object;
+
+	DBG("");
+
+	if (obj->session)
+		obj->session->obj = NULL;
+
+	if (obj->buffer)
+		g_string_free(obj->buffer, TRUE);
+
+	g_free(obj);
+
+	return 0;
+}
+
 static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 				void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
+	struct pbap_object *obj = NULL;
 	int ret;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
@@ -766,30 +796,25 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 
 	/* PullvCardListing always get the contacts from the cache */
 
-	if (pbap->cache.valid) {
-		/*
-		 * Valid cache and empty buffer mean that cache was already
-		 * created within a single session, but no data is available.
-		 */
-		if (!pbap->obj->buffer) {
-			ret = -ENOENT;
-			goto fail;
-		}
-
-		cache_ready_notify(pbap);
-		goto done;
-	}
-
-	ret = phonebook_create_cache(name,
-		cache_entry_notify, cache_ready_notify, pbap);
+	obj = vobject_create(pbap);
 
+	if (pbap->cache.valid)
+		ret = generate_response(pbap);
+	else
+		ret = phonebook_create_cache(name, cache_entry_notify,
+						cache_ready_notify, pbap);
 	if (ret < 0)
 		goto fail;
 
-done:
-	return vobject_create(pbap);
+	if (err)
+		*err = 0;
+
+	return obj;
 
 fail:
+	if (obj)
+		vobject_close(obj);
+
 	if (err)
 		*err = ret;
 
@@ -902,21 +927,6 @@ static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
 	return string_read(obj->buffer, buf, count);
 }
 
-static int vobject_close(void *object)
-{
-	struct pbap_object *obj = object;
-
-	if (obj->session)
-		obj->session->obj = NULL;
-
-	if (obj->buffer)
-		g_string_free(obj->buffer, TRUE);
-
-	g_free(obj);
-
-	return 0;
-}
-
 static struct obex_mime_type_driver mime_pull = {
 	.target = PBAP_TARGET,
 	.target_size = TARGET_SIZE,
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 0/1 v2] Fix regression causing crash in 3-way calling
From: Dmitriy Paliy @ 2010-12-08 11:48 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

More detailed commit message explaining the problem and
justification of changes, including refactoring of the code,
are written in this version of submitted earlier patch.

Also, label is removed in generate_response function.

Thanks,
Dmitriy


^ permalink raw reply

* Re: [PATCH 1/1] bluetooth: Fix NULL pointer dereference issue
From: Yuri Ershov @ 2010-12-08 10:52 UTC (permalink / raw)
  To: ext Gustavo F. Padovan
  Cc: marcel, davem, jprvita, ville.tervo, andrei.emeltchenko,
	linux-bluetooth@vger.kernel.org
In-Reply-To: <20101207155037.GA2944@vigoh>

[-- Attachment #1: Type: text/plain, Size: 2240 bytes --]

Hi Gustavo,

ext Gustavo F. Padovan wrote:
> Hi Yuri,
>
> * Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-12-07 16:12:00 +0300]:
>
>   
>> Hello Gustavo,
>>
>> ext Gustavo F. Padovan wrote:
>>     
>>> Hi Yuri,
>>>
>>> * Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-11-25 12:55:33 +0300]:
>>>
>>>   
>>>       
>>>> This patch is an addition to my previous patch for this issue.
>>>> The problem is in resynchronization between two loops:
>>>> 1. Main controlling loop (l2cap_connect_req, l2cap_config_req,
>>>> l2cap_config_rsp, l2cap_disconnect_req, etc.)
>>>> 2. Loop waiting of BT_CONNECTED state of socket (l2cap_sock_accept,
>>>> bt_accept_dequeue, etc.).
>>>> In case of fast sequence of connect/disconnect operations the loop #1
>>>> makes several cycles, while the loop #2 only has time to make one
>>>> cycle and it results crash.
>>>> The aim of the patch is to skeep handling of sockets queued for
>>>> deletion.
>>>>
>>>> Signed-off-by: Yuri Ershov <ext-yuri.ershov@nokia.com>
>>>> ---
>>>>  net/bluetooth/af_bluetooth.c |    2 ++
>>>>  net/bluetooth/l2cap.c        |    6 ++++--
>>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
>>>> index c4cf3f5..f9389da 100644
>>>> --- a/net/bluetooth/af_bluetooth.c
>>>> +++ b/net/bluetooth/af_bluetooth.c
>>>> @@ -200,6 +200,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
>>>>  	BT_DBG("parent %p", parent);
>>>>  
>>>>  	list_for_each_safe(p, n, &bt_sk(parent)->accept_q) {
>>>> +		if (n == p)
>>>> +			break;
>>>>     
>>>>         
>>> So in which situations (n == p), or (p == p->next)? That should happen only
>>> when p is the only element in the list, then p == head, right?
>>>   
>>>       
>> The (n == p) is in situation, when sk is unlinked by task responsible 
>> for handling connect/disconnect requests while the "bt_accept_dequeue". 
>> This condition is indirect checking of sk validity.
>>     
>
> Why not using a list lock here instead? Fits a way better.
>
>   
Yes, it's better. I tried to use the locks in this function, but it 
slows down the task handling connect/disconnect/etc. events and the task 
skips some events from fast clients.


[-- Attachment #2: Type: text/html, Size: 2898 bytes --]

^ permalink raw reply

* [PATCH] Fix regression causing crash in 3-way calling
From: Dmitriy Paliy @ 2010-12-08 10:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy

Fix possible obexd crash in 3-way calling scenario. Callback
function cache_ready_notify is splitted in two cache_ready_notify
and generate_response functions.

generate_response fills data to buffer and returns error, if any.
---
 plugins/pbap.c |   94 +++++++++++++++++++++++++++++++-------------------------
 1 files changed, 52 insertions(+), 42 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 1a7d001..83c8ff4 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -378,7 +378,7 @@ static GSList *sort_entries(GSList *l, uint8_t order, uint8_t search_attrib,
 	return sorted;
 }
 
-static void cache_ready_notify(void *user_data)
+static int generate_response(void *user_data)
 {
 	struct pbap_session *pbap = user_data;
 	GSList *sorted;
@@ -409,11 +409,8 @@ static void cache_ready_notify(void *user_data)
 			pbap->params->searchattrib,
 			(const char *) pbap->params->searchval);
 
-	if (sorted == NULL) {
-		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
-		return;
-	}
+	if (sorted == NULL)
+		return -ENOENT;
 
 	/* Computing offset considering first entry of the phonebook */
 	l = g_slist_nth(sorted, pbap->params->liststartoffset);
@@ -431,10 +428,25 @@ static void cache_ready_notify(void *user_data)
 	g_slist_free(sorted);
 
 done:
-	if (!pbap->cache.valid) {
-		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
+	return 0;
+}
+
+static void cache_ready_notify(void *user_data)
+{
+	struct pbap_session *pbap = user_data;
+	int err;
+
+	DBG("");
+
+	pbap->cache.valid = TRUE;
+
+	err = generate_response(pbap);
+	if (err < 0) {
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, err);
+		return;
 	}
+
+	obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 }
 
 static void cache_entry_done(void *user_data)
@@ -746,10 +758,28 @@ fail:
 	return NULL;
 }
 
+static int vobject_close(void *object)
+{
+	struct pbap_object *obj = object;
+
+	DBG("");
+
+	if (obj->session)
+		obj->session->obj = NULL;
+
+	if (obj->buffer)
+		g_string_free(obj->buffer, TRUE);
+
+	g_free(obj);
+
+	return 0;
+}
+
 static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 				void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
+	struct pbap_object *obj = NULL;
 	int ret;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
@@ -766,30 +796,25 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 
 	/* PullvCardListing always get the contacts from the cache */
 
-	if (pbap->cache.valid) {
-		/*
-		 * Valid cache and empty buffer mean that cache was already
-		 * created within a single session, but no data is available.
-		 */
-		if (!pbap->obj->buffer) {
-			ret = -ENOENT;
-			goto fail;
-		}
-
-		cache_ready_notify(pbap);
-		goto done;
-	}
-
-	ret = phonebook_create_cache(name,
-		cache_entry_notify, cache_ready_notify, pbap);
+	obj = vobject_create(pbap);
 
+	if (pbap->cache.valid)
+		ret = generate_response(pbap);
+	else
+		ret = phonebook_create_cache(name, cache_entry_notify,
+						cache_ready_notify, pbap);
 	if (ret < 0)
 		goto fail;
 
-done:
-	return vobject_create(pbap);
+	if (err)
+		*err = 0;
+
+	return obj;
 
 fail:
+	if (obj)
+		vobject_close(obj);
+
 	if (err)
 		*err = ret;
 
@@ -902,21 +927,6 @@ static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
 	return string_read(obj->buffer, buf, count);
 }
 
-static int vobject_close(void *object)
-{
-	struct pbap_object *obj = object;
-
-	if (obj->session)
-		obj->session->obj = NULL;
-
-	if (obj->buffer)
-		g_string_free(obj->buffer, TRUE);
-
-	g_free(obj);
-
-	return 0;
-}
-
 static struct obex_mime_type_driver mime_pull = {
 	.target = PBAP_TARGET,
 	.target_size = TARGET_SIZE,
-- 
1.7.0.4


^ permalink raw reply related

* LE Slave
From: Koustuv Ghosh @ 2010-12-08  8:19 UTC (permalink / raw)
  To: linux-bluetooth

Hello ,

dose anybody know whether any commercial LE slave(core 4.0 compatible)
is available where slave side Security Manager Protocol is up and
running?

rgds
Koustuv

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Pavan Savoy @ 2010-12-08  7:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Vitaly Wool, Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij,
	linux-bluetooth, linux-kernel, Marcel Holtmann
In-Reply-To: <201012070007.19239.arnd@arndb.de>

On Tue, Dec 7, 2010 at 4:37 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 06 December 2010 22:24:34 Vitaly Wool wrote:
>> On Mon, Dec 6, 2010 at 5:54 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> >> But I was trying to make a different point here. On a basic level,
>> >> there's this cg2000 chip from STE that does BT, FM and GPS. There's
>> >> the chip from TI that does BT, FM and GPS, and there's the Broadcom
>> >> chip that does BT+FM. They all use HCI to access the other functions
>> >> of the combo chip and they do it in a really simiar way, with the
>> >> differences mostly in power management techniques. So I think it's
>> >> quite sensible to have some kind of framework that is suitable for
>> >> such devices.
>> >
>> > Yes, I agree 100% in principle. I could not find the code that
>> > Broadcom/TI FM and GPS stuff so far, can you point us to that?
>>
>> Sure, the TI "shared transport" code is mostly at drivers/misc/ti-st.
>>
>> Some Broadcom BCM43xx chips work in a similar way AFAIK but I'm not
>> sure about the mainline support for those.
>
> Ah, it had not occured to me to look in drivers/misc. Looking at the
> code over there, it becomes much clearer what your point is. Evidently
> the ti-st driver implements a line discipline similar to, but conflicting
> with hci_ldisc, and it has its own copy of the hci_ll code.
>
> I guess this is exactly what we're trying to avoid having with the
> cg2900 code ;-).
>
>> > The cg2900 solution for this was to use MFD (plus another layer
>> > in the posted version, but that will go away I assume). Using
>> > MFD is not the only possibility here, but I could not see anything
>> > wrong with it either. Do you think we can move them all over to
>> > use MFD infrastructure?
>>
>> I guess so but it's probably more of a detail than what I'm up to now :)
>
> Agreed.
>
>> >> But generally speaking, isn't a line discipline/driver attached to a
>> >> tty? We can use dumb tty for e. g. SPI and still be able to use
>> >> hci_ll, right?
>> >
>> > I suggested that as well, but the point was made that this would
>> > add an unnecessary indirection for the SPI case, which is not
>> > really much like a serial port. It's certainly possible to do it
>> > like you say, but if we add a way to register the high-level
>> > protocols with an HCI-like multi-function device, we could
>> > also do it in a way that does not rely on tty-ldisc but keeps it
>> > as one of the options.
>>
>> I actually don't think it's such a big indirection, I prefer to think
>> of it more as of the abstraction layer. If not use this, are we going
>> to have direct SPI device drivers? I'm afraid we might end up with a
>> huge duplication of code in that case.
>
> The question is how it should be modeled in a better way than today.
>
> I believe we already agree it should be something like
>
> =C2=A0 bt =C2=A0 ti-radio =C2=A0 =C2=A0st-e-radio =C2=A0 =C2=A0ti-gps =C2=
=A0 st-e-gps =C2=A0broadcom-radio ...
> =C2=A0 | =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
| =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 |
> =C2=A0 +---------+----------+---------+--+----------+----------+---------=
+
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0common-hci-module
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0+-----------+-----------+
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=
=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0seri=
al =C2=A0 =C2=A0spi =C2=A0 =C2=A0 i2c =C2=A0 =C2=A0...

Yes, this sort of architecture would certainly help...
However, I suppose the most common question as one of you stated above
was how can the channel -ID and other factors such as offset-header
packet format be generalized?

As in over here, will the common-hci-module work fine, If say ti-radio
says he is expecting packets starting from id-8 which is of length 10?
and also work for st-e-radio which would say expecting data from id-6
with 15 max bytes?
Answering this I suppose would solve a lot of our problems....

Marcel did previously suggest a bus-driver sort of a structure, where
the transport are sort of adapter drivers, the data interpretation
logic like splitting HCI-events, FM CH-8 packets all fall into the
algos drivers and the actual Bluetooth driver or GPS or FM-V4L2
drivers falling into the client/protocol drivers....



> Any objections to this?
>
> If you agree, I think we should discuss the alternatives for the common
> module. I think you are saying we should make the common module a single
> ldisc doing the multiplexing and have all transports register as ttys int=
o
> it, right?
>
> What we discussed before was to split it into multiple modules, one for
> the tty ldisc and one for the common code, and then have the spi/i2c/...
> drivers feed into the common multiplexer without going through tty.
>
> I don't currently see a significant advantage or disadvantage with either
> approach.
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0Arnd
> --
> 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 =C2=A0http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH] Revert use of new error function for Blocked
From: Johan Hedberg @ 2010-12-08  7:03 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1291770126-2726-1-git-send-email-padovan@profusion.mobi>

Hi Gustavo,

On Tue, Dec 07, 2010, Gustavo F. Padovan wrote:
> The error message says a lot about what the user need to do, I'm reverting
> this change and will fix in the next patch series about the DBus error
> handling.
> ---
>  src/device.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

Applied, thanks.

Johan

^ permalink raw reply

* csr initialization
From: Brad Midgley @ 2010-12-08  6:55 UTC (permalink / raw)
  To: linux-bluetooth

Hey,

Is anyone on this list using the csr dev board and csr tools? It would
be great to know if we might be able to make a bluecore4-rom chip
change baud rate and reroute sco using some initialization and a soft
reboot. If we can figure something, it could be submitted as a patch
to bccmd. I'm just not sure it's possible.

-- 
Brad Midgley

^ permalink raw reply

* Re: [RFC v2 7/9] Bluetooth: Add support for SMP confirmation checks
From: Brian Gix @ 2010-12-08  6:33 UTC (permalink / raw)
  To: Koustuv Ghosh; +Cc: Vinicius Costa Gomes, linux-bluetooth
In-Reply-To: <AANLkTikkMK6gXigQ49iMa25c2XZ1Dx9hp7DvWR=8_3XQ@mail.gmail.com>

Hi Koustuv,

On 12/07/2010 9:48 PM, Koustuv Ghosh wrote:
>   Hello Vinicus,
>   regarding the below routine,
>   is it not good to check the IO capabilities and authentication
> rquirement of the remote device from the skb inside this routine and
> based on that we can trigger JustWorks or PassKey paring method.
> Though I know you are implementing  only JustWorks
>   but I feel it will be good approach to parse the response and then
> trigger the paring method.

The way LE pairing is suppose to work is that each of the two devices
declares what it wants from the process ((Permanent)Pairing and/or 
Man-In-The-Middle (MITM) protection) and what kind of I/O it has to
perform the required steps.

If *either* side wants MITM, then it will be attempted, but if the
I/O capabilities are incompatible with MITM, then JUST_WORKS must be
done instead. The only special case here is if both side *Don't* want
MITM, and at least one side does not have OOB capabilities. In that
case, JUST_WORKS is always used, regardless of the I/O Capabilities.

This creates the situation that going into the process, neither side
knows for sure whether MITM protection is provided. So at "completion"
of the pairing process, the level of protection provided must be 
advertised to the upper layers.  This gives the individual profiles
the protection level provided, so that they may reject any transactions 
that do not conform to their required security level.

In other words, you can request MITM (highest) security, but you cannot 
require it during pairing. You will get MITM only if possible. It *will 
not* prevent pairing from happening. however you will always know at 
completion what level has been achieved.

>
>
>    static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
>    {
> +       struct smp_cmd_pairing *rp = (void *) skb->data;
>          struct smp_cmd_pairing_confirm cp;
>   +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
>   +       int ret;
>   +       u8 k[16], res[16];
>
>   -       BT_DBG("");
>   +       /* Just Works */
>   +       memset(k, 0, sizeof(k));
>   +
>   +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
>   +       memcpy(&conn->pres[1], rp, sizeof(*rp));
>   +       skb_pull(skb, sizeof(*rp));
>
>   -       memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
>   +       ret = smp_rand(conn->prnd);
>   +       if (ret)
>   +               return;
>   +
>   +       ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
>   +                       conn->src, 0, conn->dst, res);
>   +       if (ret)
>   +               return;
>
>          smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp),&cp);
>    }
>
> On Tue, Dec 7, 2010 at 3:13 AM, Vinicius Costa Gomes
> <vinicius.gomes@openbossa.org>  wrote:
>> This adds supports for verifying the confirmation value that the
>> remote side has sent. This includes support for generating and sending
>> the random value used to produce the confirmation value.
>>
>> Signed-off-by: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
>> ---
>>   include/net/bluetooth/l2cap.h |    5 ++
>>   net/bluetooth/smp.c           |  121 ++++++++++++++++++++++++++++++++---------
>>   2 files changed, 101 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index a3cb1ab..bcda2aa 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -290,6 +290,11 @@ struct l2cap_conn {
>>
>>         __u8            disc_reason;
>>
>> +       __u8            preq[7];
>> +       __u8            pres[7];
>> +       __u8            prnd[16];
>> +       __u8            pcnf[16];
>> +
>>         struct l2cap_chan_list chan_list;
>>   };
>>
>> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
>> index b62160e..7d7e8ad 100644
>> --- a/net/bluetooth/smp.c
>> +++ b/net/bluetooth/smp.c
>> @@ -203,7 +203,9 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
>>
>>         BT_DBG("");
>>
>> -       skb_pull(skb, sizeof(struct smp_cmd_pairing));
>> +       conn->preq[0] = SMP_CMD_PAIRING_REQ;
>> +       memcpy(&conn->preq[1], rp, sizeof(*rp));
>> +       skb_pull(skb, sizeof(*rp));
>>
>>         rp->io_capability = 0x00;
>>         rp->oob_flag = 0x00;
>> @@ -212,64 +214,125 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
>>         rp->resp_key_dist = 0x00;
>>         rp->auth_req&= 0x05;
>>
>> +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
>> +       memcpy(&conn->pres[1], rp, sizeof(rp));
>> +
>>         smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
>>   }
>>
>>   static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
>>   {
>> +       struct smp_cmd_pairing *rp = (void *) skb->data;
>>         struct smp_cmd_pairing_confirm cp;
>> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
>> +       int ret;
>> +       u8 k[16], res[16];
>>
>> -       BT_DBG("");
>> +       /* Just Works */
>> +       memset(k, 0, sizeof(k));
>> +
>> +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
>> +       memcpy(&conn->pres[1], rp, sizeof(*rp));
>> +       skb_pull(skb, sizeof(*rp));
>> +
>> +       ret = smp_rand(conn->prnd);
>> +       if (ret)
>> +               return;
>>
>> -       memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
>> +       ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
>> +                       conn->src, 0, conn->dst, res);
>> +       if (ret)
>> +               return;
>> +
>> +       swap128(res, cp.confirm_val);
>>
>>         smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp),&cp);
>>   }
>>
>>   static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
>>   {
>> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
>> +
>>         BT_DBG("");
>>
>> -       if (conn->hcon->out) {
>> -               struct smp_cmd_pairing_random random;
>> +       memcpy(conn->pcnf, skb->data, 16);
>> +       skb_pull(skb, 16);
>>
>> -               BT_DBG("master");
>> +       if (conn->hcon->out) {
>> +               u8 random[16];
>>
>> -               memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
>> +               swap128(conn->prnd, random);
>>
>> -               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
>> -&random);
>> +               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, random);
>>         } else {
>> -               struct smp_cmd_pairing_confirm confirm;
>> +               struct smp_cmd_pairing_confirm cp;
>> +               int ret;
>> +               u8 k[16], res[16];
>> +
>> +               /* Just Works */
>> +               memset(k, 0, sizeof(k));
>>
>> -               BT_DBG("slave");
>> +               ret = smp_rand(conn->prnd);
>> +               if (ret)
>> +                       return;
>>
>> -               memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
>> +               ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
>> +                               conn->dst, 0, conn->src, res);
>> +               if (ret)
>> +                       return;
>>
>> -               smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
>> -&confirm);
>> +               swap128(res, cp.confirm_val);
>> +
>> +               smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp),&cp);
>>         }
>>   }
>>
>>   static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
>>   {
>> -       struct smp_cmd_pairing_random cp;
>> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
>> +       int ret;
>> +       u8 k[16], key[16], res[16], random[16], confirm[16], buf[128];
>> +
>> +       swap128(skb->data, random);
>> +       skb_pull(skb, 16);
>> +
>> +       memset(k, 0, sizeof(k));
>> +
>> +       if (conn->hcon->out)
>> +               ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
>> +                               conn->src, 0, conn->dst, res);
>> +       else
>> +               ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
>> +                               conn->dst, 0, conn->src, res);
>> +       if (ret)
>> +               return;
>>
>> -       BT_DBG("");
>> +       swap128(res, confirm);
>>
>> -       skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
>> +       if (memcmp(conn->pcnf, confirm, 16) != 0) {
>> +               struct smp_cmd_pairing_fail cp;
>>
>> -       /* FIXME: check if random matches */
>> +               BT_ERR("Pairing failed (confirmation values mismatch)");
>> +               cp.reason = SMP_CONFIRM_FAILED;
>> +               smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, sizeof(cp),&cp);
>> +               return;
>> +       }
>>
>>         if (conn->hcon->out) {
>> -               BT_DBG("master");
>> -               /* FIXME: start encryption */
>> +               smp_s1(tfm, k, random, conn->prnd, key);
>> +
>> +               hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
>> +               BT_DBG("key %s", buf);
>>         } else {
>> -               BT_DBG("slave");
>> +               u8 r[16];
>>
>> -               memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
>> +               swap128(conn->prnd, r);
>> +               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, r);
>>
>> -               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp),&cp);
>> +               smp_s1(tfm, k, conn->prnd, random, key);
>> +
>> +               hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
>> +               BT_DBG("key %s", buf);
>>         }
>>   }
>>
>> @@ -280,8 +343,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
>>
>>         BT_DBG("");
>>
>> -       skb_pull(skb, sizeof(struct smp_cmd_security_req));
>> -       memset(&cp, 0, sizeof(struct smp_cmd_pairing));
>> +       skb_pull(skb, sizeof(*rp));
>> +
>> +       memset(&cp, 0, sizeof(cp));
>>
>>         cp.io_capability = 0x00;
>>         cp.oob_flag = 0x00;
>> @@ -290,6 +354,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
>>         cp.resp_key_dist = 0x00;
>>         cp.auth_req = rp->auth_req&  0x05;
>>
>> +       conn->preq[0] = SMP_CMD_PAIRING_REQ;
>> +       memcpy(&conn->preq[1],&cp, sizeof(cp));
>> +
>>         smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp),&cp);
>>   }
>>
>> @@ -323,6 +390,10 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
>>                 cp.init_key_dist = 0x00;
>>                 cp.resp_key_dist = 0x00;
>>                 cp.auth_req = authreq;
>> +
>> +               conn->preq[0] = SMP_CMD_PAIRING_REQ;
>> +               memcpy(&conn->preq[1],&cp, sizeof(cp));
>> +
>>                 smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp),&cp);
>>         } else {
>>                 struct smp_cmd_security_req cp;
>> --
>> 1.7.3.2
>>
>> --
>> 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


-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* Re: [RFC v2 7/9] Bluetooth: Add support for SMP confirmation checks
From: Koustuv Ghosh @ 2010-12-08  6:19 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1291671832-13435-8-git-send-email-vinicius.gomes@openbossa.org>

Hello ,
pls I apologise for top posting. So I request to ignore my first mail.

On Tue, Dec 7, 2010 at 3:13 AM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> This adds supports for verifying the confirmation value that the
> remote side has sent. This includes support for generating and sending
> the random value used to produce the confirmation value.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  include/net/bluetooth/l2cap.h |    5 ++
>  net/bluetooth/smp.c           |  121 ++++++++++++++++++++++++++++++++---------
>  2 files changed, 101 insertions(+), 25 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index a3cb1ab..bcda2aa 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -290,6 +290,11 @@ struct l2cap_conn {
>
>        __u8            disc_reason;
>
> +       __u8            preq[7];
> +       __u8            pres[7];
> +       __u8            prnd[16];
> +       __u8            pcnf[16];
> +
>        struct l2cap_chan_list chan_list;
>  };
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index b62160e..7d7e8ad 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -203,7 +203,9 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
>
>        BT_DBG("");
>
> -       skb_pull(skb, sizeof(struct smp_cmd_pairing));
> +       conn->preq[0] = SMP_CMD_PAIRING_REQ;
> +       memcpy(&conn->preq[1], rp, sizeof(*rp));
> +       skb_pull(skb, sizeof(*rp));
>
>        rp->io_capability = 0x00;
>        rp->oob_flag = 0x00;
> @@ -212,64 +214,125 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
>        rp->resp_key_dist = 0x00;
>        rp->auth_req &= 0x05;
>
> +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
> +       memcpy(&conn->pres[1], rp, sizeof(rp));
> +
>        smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
>  }
>
>  static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> +       struct smp_cmd_pairing *rp = (void *) skb->data;
>        struct smp_cmd_pairing_confirm cp;
> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +       int ret;
> +       u8 k[16], res[16];
>
> -       BT_DBG("");
> +       /* Just Works */
> +       memset(k, 0, sizeof(k));
> +
> +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
> +       memcpy(&conn->pres[1], rp, sizeof(*rp));
> +       skb_pull(skb, sizeof(*rp));
> +
> +       ret = smp_rand(conn->prnd);
> +       if (ret)
> +               return;
>
> -       memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
> +       ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
> +                       conn->src, 0, conn->dst, res);
> +       if (ret)
> +               return;
> +
> +       swap128(res, cp.confirm_val);
>
>        smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
>  }
>
>  static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +
>        BT_DBG("");
>
> -       if (conn->hcon->out) {
> -               struct smp_cmd_pairing_random random;
> +       memcpy(conn->pcnf, skb->data, 16);
> +       skb_pull(skb, 16);
>
> -               BT_DBG("master");
> +       if (conn->hcon->out) {
> +               u8 random[16];
>
> -               memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
> +               swap128(conn->prnd, random);
>
> -               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
> -                                                               &random);
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, random);
>        } else {
> -               struct smp_cmd_pairing_confirm confirm;
> +               struct smp_cmd_pairing_confirm cp;
> +               int ret;
> +               u8 k[16], res[16];
> +
> +               /* Just Works */
> +               memset(k, 0, sizeof(k));
>
> -               BT_DBG("slave");
> +               ret = smp_rand(conn->prnd);
> +               if (ret)
> +                       return;
>
> -               memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
> +               ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
> +                               conn->dst, 0, conn->src, res);
> +               if (ret)
> +                       return;
>
> -               smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
> -                                                               &confirm);
> +               swap128(res, cp.confirm_val);
> +
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
>        }
>  }
>
>  static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> -       struct smp_cmd_pairing_random cp;
> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +       int ret;
> +       u8 k[16], key[16], res[16], random[16], confirm[16], buf[128];
> +
> +       swap128(skb->data, random);
> +       skb_pull(skb, 16);
> +
> +       memset(k, 0, sizeof(k));
> +
> +       if (conn->hcon->out)
> +               ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
> +                               conn->src, 0, conn->dst, res);
> +       else
> +               ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
> +                               conn->dst, 0, conn->src, res);
> +       if (ret)
> +               return;
>
> -       BT_DBG("");
> +       swap128(res, confirm);
>
> -       skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
> +       if (memcmp(conn->pcnf, confirm, 16) != 0) {
> +               struct smp_cmd_pairing_fail cp;
>
> -       /* FIXME: check if random matches */
> +               BT_ERR("Pairing failed (confirmation values mismatch)");
> +               cp.reason = SMP_CONFIRM_FAILED;
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, sizeof(cp), &cp);
> +               return;
> +       }
>
>        if (conn->hcon->out) {
> -               BT_DBG("master");
> -               /* FIXME: start encryption */
> +               smp_s1(tfm, k, random, conn->prnd, key);
> +
> +               hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
> +               BT_DBG("key %s", buf);
>        } else {
> -               BT_DBG("slave");
> +               u8 r[16];
>
> -               memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
> +               swap128(conn->prnd, r);
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, r);
>
> -               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp), &cp);
> +               smp_s1(tfm, k, conn->prnd, random, key);
> +
> +               hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
> +               BT_DBG("key %s", buf);
>        }
>  }
>
> @@ -280,8 +343,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
>
>        BT_DBG("");
>
> -       skb_pull(skb, sizeof(struct smp_cmd_security_req));
> -       memset(&cp, 0, sizeof(struct smp_cmd_pairing));
> +       skb_pull(skb, sizeof(*rp));
> +
> +       memset(&cp, 0, sizeof(cp));
>
>        cp.io_capability = 0x00;
>        cp.oob_flag = 0x00;
> @@ -290,6 +354,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
>        cp.resp_key_dist = 0x00;
>        cp.auth_req = rp->auth_req & 0x05;
>
> +       conn->preq[0] = SMP_CMD_PAIRING_REQ;
> +       memcpy(&conn->preq[1], &cp, sizeof(cp));
> +
>        smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
>  }
>
> @@ -323,6 +390,10 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
>                cp.init_key_dist = 0x00;
>                cp.resp_key_dist = 0x00;
>                cp.auth_req = authreq;
> +
> +               conn->preq[0] = SMP_CMD_PAIRING_REQ;
> +               memcpy(&conn->preq[1], &cp, sizeof(cp));
> +
>                smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
>        } else {
>                struct smp_cmd_security_req cp;
> --
> 1.7.3.2
>
> --
> 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
>

Hello Vinicus,
 regarding the below routine,
 is it not good to check the IO capabilities and authentication
rquirement of the remote device from the skb inside this routine and
based on that we can trigger JustWorks or PassKey paring method.
Though I know you are implementing  only JustWorks
 but I feel it will be good approach to parse the response and then
trigger the paring method.



 static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 {
+       struct smp_cmd_pairing *rp = (void *) skb->data;
       struct smp_cmd_pairing_confirm cp;
 +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
 +       int ret;
 +       u8 k[16], res[16];

 -       BT_DBG("");
 +       /* Just Works */
 +       memset(k, 0, sizeof(k));
 +
 +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
 +       memcpy(&conn->pres[1], rp, sizeof(*rp));
 +       skb_pull(skb, sizeof(*rp));


 -       memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
 +       ret = smp_rand(conn->prnd);
 +       if (ret)
 +               return;
 +
 +       ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,

 +                       conn->src, 0, conn->dst, res);
 +       if (ret)
 +               return;


       smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
 }

^ permalink raw reply

* Re: [RFC v2 7/9] Bluetooth: Add support for SMP confirmation checks
From: Koustuv Ghosh @ 2010-12-08  5:48 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1291671832-13435-8-git-send-email-vinicius.gomes@openbossa.org>

 Hello Vinicus,
 regarding the below routine,
 is it not good to check the IO capabilities and authentication
rquirement of the remote device from the skb inside this routine and
based on that we can trigger JustWorks or PassKey paring method.
Though I know you are implementing  only JustWorks
 but I feel it will be good approach to parse the response and then
trigger the paring method.


  static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
  {
+       struct smp_cmd_pairing *rp = (void *) skb->data;
        struct smp_cmd_pairing_confirm cp;
 +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
 +       int ret;
 +       u8 k[16], res[16];

 -       BT_DBG("");
 +       /* Just Works */
 +       memset(k, 0, sizeof(k));
 +
 +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
 +       memcpy(&conn->pres[1], rp, sizeof(*rp));
 +       skb_pull(skb, sizeof(*rp));

 -       memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
 +       ret = smp_rand(conn->prnd);
 +       if (ret)
 +               return;
 +
 +       ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
 +                       conn->src, 0, conn->dst, res);
 +       if (ret)
 +               return;

        smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
  }

On Tue, Dec 7, 2010 at 3:13 AM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> This adds supports for verifying the confirmation value that the
> remote side has sent. This includes support for generating and sending
> the random value used to produce the confirmation value.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  include/net/bluetooth/l2cap.h |    5 ++
>  net/bluetooth/smp.c           |  121 ++++++++++++++++++++++++++++++++---------
>  2 files changed, 101 insertions(+), 25 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index a3cb1ab..bcda2aa 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -290,6 +290,11 @@ struct l2cap_conn {
>
>        __u8            disc_reason;
>
> +       __u8            preq[7];
> +       __u8            pres[7];
> +       __u8            prnd[16];
> +       __u8            pcnf[16];
> +
>        struct l2cap_chan_list chan_list;
>  };
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index b62160e..7d7e8ad 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -203,7 +203,9 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
>
>        BT_DBG("");
>
> -       skb_pull(skb, sizeof(struct smp_cmd_pairing));
> +       conn->preq[0] = SMP_CMD_PAIRING_REQ;
> +       memcpy(&conn->preq[1], rp, sizeof(*rp));
> +       skb_pull(skb, sizeof(*rp));
>
>        rp->io_capability = 0x00;
>        rp->oob_flag = 0x00;
> @@ -212,64 +214,125 @@ static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
>        rp->resp_key_dist = 0x00;
>        rp->auth_req &= 0x05;
>
> +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
> +       memcpy(&conn->pres[1], rp, sizeof(rp));
> +
>        smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
>  }
>
>  static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> +       struct smp_cmd_pairing *rp = (void *) skb->data;
>        struct smp_cmd_pairing_confirm cp;
> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +       int ret;
> +       u8 k[16], res[16];
>
> -       BT_DBG("");
> +       /* Just Works */
> +       memset(k, 0, sizeof(k));
> +
> +       conn->pres[0] = SMP_CMD_PAIRING_RSP;
> +       memcpy(&conn->pres[1], rp, sizeof(*rp));
> +       skb_pull(skb, sizeof(*rp));
> +
> +       ret = smp_rand(conn->prnd);
> +       if (ret)
> +               return;
>
> -       memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
> +       ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
> +                       conn->src, 0, conn->dst, res);
> +       if (ret)
> +               return;
> +
> +       swap128(res, cp.confirm_val);
>
>        smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
>  }
>
>  static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +
>        BT_DBG("");
>
> -       if (conn->hcon->out) {
> -               struct smp_cmd_pairing_random random;
> +       memcpy(conn->pcnf, skb->data, 16);
> +       skb_pull(skb, 16);
>
> -               BT_DBG("master");
> +       if (conn->hcon->out) {
> +               u8 random[16];
>
> -               memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
> +               swap128(conn->prnd, random);
>
> -               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
> -                                                               &random);
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, random);
>        } else {
> -               struct smp_cmd_pairing_confirm confirm;
> +               struct smp_cmd_pairing_confirm cp;
> +               int ret;
> +               u8 k[16], res[16];
> +
> +               /* Just Works */
> +               memset(k, 0, sizeof(k));
>
> -               BT_DBG("slave");
> +               ret = smp_rand(conn->prnd);
> +               if (ret)
> +                       return;
>
> -               memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
> +               ret = smp_c1(tfm, k, conn->prnd, conn->preq, conn->pres, 0,
> +                               conn->dst, 0, conn->src, res);
> +               if (ret)
> +                       return;
>
> -               smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
> -                                                               &confirm);
> +               swap128(res, cp.confirm_val);
> +
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
>        }
>  }
>
>  static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
>  {
> -       struct smp_cmd_pairing_random cp;
> +       struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm;
> +       int ret;
> +       u8 k[16], key[16], res[16], random[16], confirm[16], buf[128];
> +
> +       swap128(skb->data, random);
> +       skb_pull(skb, 16);
> +
> +       memset(k, 0, sizeof(k));
> +
> +       if (conn->hcon->out)
> +               ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
> +                               conn->src, 0, conn->dst, res);
> +       else
> +               ret = smp_c1(tfm, k, random, conn->preq, conn->pres, 0,
> +                               conn->dst, 0, conn->src, res);
> +       if (ret)
> +               return;
>
> -       BT_DBG("");
> +       swap128(res, confirm);
>
> -       skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
> +       if (memcmp(conn->pcnf, confirm, 16) != 0) {
> +               struct smp_cmd_pairing_fail cp;
>
> -       /* FIXME: check if random matches */
> +               BT_ERR("Pairing failed (confirmation values mismatch)");
> +               cp.reason = SMP_CONFIRM_FAILED;
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, sizeof(cp), &cp);
> +               return;
> +       }
>
>        if (conn->hcon->out) {
> -               BT_DBG("master");
> -               /* FIXME: start encryption */
> +               smp_s1(tfm, k, random, conn->prnd, key);
> +
> +               hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
> +               BT_DBG("key %s", buf);
>        } else {
> -               BT_DBG("slave");
> +               u8 r[16];
>
> -               memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
> +               swap128(conn->prnd, r);
> +               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, 16, r);
>
> -               smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp), &cp);
> +               smp_s1(tfm, k, conn->prnd, random, key);
> +
> +               hex_dump_to_buffer(key, sizeof(key), 16, 1, buf, sizeof(buf), 0);
> +               BT_DBG("key %s", buf);
>        }
>  }
>
> @@ -280,8 +343,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
>
>        BT_DBG("");
>
> -       skb_pull(skb, sizeof(struct smp_cmd_security_req));
> -       memset(&cp, 0, sizeof(struct smp_cmd_pairing));
> +       skb_pull(skb, sizeof(*rp));
> +
> +       memset(&cp, 0, sizeof(cp));
>
>        cp.io_capability = 0x00;
>        cp.oob_flag = 0x00;
> @@ -290,6 +354,9 @@ static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
>        cp.resp_key_dist = 0x00;
>        cp.auth_req = rp->auth_req & 0x05;
>
> +       conn->preq[0] = SMP_CMD_PAIRING_REQ;
> +       memcpy(&conn->preq[1], &cp, sizeof(cp));
> +
>        smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
>  }
>
> @@ -323,6 +390,10 @@ int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
>                cp.init_key_dist = 0x00;
>                cp.resp_key_dist = 0x00;
>                cp.auth_req = authreq;
> +
> +               conn->preq[0] = SMP_CMD_PAIRING_REQ;
> +               memcpy(&conn->preq[1], &cp, sizeof(cp));
> +
>                smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
>        } else {
>                struct smp_cmd_security_req cp;
> --
> 1.7.3.2
>
> --
> 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 1/1] bluetooth: add NULL pointer check in hci
From: Jun Nie @ 2010-12-08  5:46 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: Vinicius Costa Gomes, Brian Gix, linux-bluetooth
In-Reply-To: <20101207150608.GA1867@vigoh>

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

2010/12/7 Gustavo F. Padovan <padovan@profusion.mobi>:
> Hi Jun,
>
> * Jun Nie <niej0001@gmail.com> [2010-12-07 15:01:21 +0800]:
>
>> Resend it to fix checkpatch.pl warning.
>
>> From 75dc111b5d9f62619bbeec803b15e84412ae050e Mon Sep 17 00:00:00 2001
>> From: Jun Nie <njun@marvell.com>
>> Date: Tue, 7 Dec 2010 14:03:38 +0800
>> Subject: [PATCH] bluetooth: add NULL pointer check in hci
>
> Clearly a bug fix, but can you add a commit message to your patch. Thanks.
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
>

Add commit message for understanding in a glance.

Jun

[-- Attachment #2: 0001-bluetooth-add-NULL-pointer-check-in-hci.patch --]
[-- Type: text/x-diff, Size: 939 bytes --]

From e729eda3b2cfae501c704e2eb39e07aa1b8607f0 Mon Sep 17 00:00:00 2001
From: Jun Nie <njun@marvell.com>
Date: Tue, 7 Dec 2010 14:03:38 +0800
Subject: [PATCH] bluetooth: add NULL pointer check in hci

If we fail to find a hci device pointer in hci_uart, don't try
to deref the NULL one we do have.

Signed-off-by: Jun Nie <njun@marvell.com>
---
 drivers/bluetooth/hci_ldisc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 7201482..3c6cabc 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -311,8 +311,10 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 
 		if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
 			hu->proto->close(hu);
-			hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
+			if (hdev) {
+				hci_unregister_dev(hdev);
+				hci_free_dev(hdev);
+			}
 		}
 	}
 }
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 3/3] Bluetooth: Make hci_send_to_sock usable for management control sockets
From: Gustavo F. Padovan @ 2010-12-08  1:09 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1291760467-1569-4-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2010-12-08 00:21:07 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> In order to send data to management control sockets the function should:
> 
>   - skip checks intended for raw HCI data and stack internal events
>   - make sure RAW HCI data or stack internal events don't go to
>     management control sockets
> 
> In order to accomplish this the patch adds a new member to the bluetooth
> skb private data to flag skb's that are destined for management control
> sockets.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>

All 3 patches have been applied. Thanks.

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

^ permalink raw reply

* [PATCH] Revert use of new error function for Blocked
From: Gustavo F. Padovan @ 2010-12-08  1:02 UTC (permalink / raw)
  To: linux-bluetooth

The error message says a lot about what the user need to do, I'm reverting
this change and will fix in the next patch series about the DBus error
handling.
---
 src/device.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index cfe00c5..b9341a2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -532,7 +532,9 @@ static DBusMessage *set_blocked(DBusConnection *conn, DBusMessage *msg,
 	case 0:
 		return dbus_message_new_method_return(msg);
 	case EINVAL:
-		return btd_error_not_supported(msg);
+		return g_dbus_create_error(msg,
+					ERROR_INTERFACE ".NotSupported",
+					"Kernel lacks blacklist support");
 	default:
 		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
 						"%s", strerror(-err));
-- 
1.7.3.2


^ permalink raw reply related

* Re: [RFC v2 4/9] Bluetooth: simple SMP pairing negotiation
From: Vinicius Costa Gomes @ 2010-12-07 22:27 UTC (permalink / raw)
  To: Brian Gix; +Cc: linux-bluetooth, 'Anderson Briglia'
In-Reply-To: <002e01cb963c$560392b0$020ab810$@org>

Hi Brian,

On 10:26 Tue 07 Dec, Brian Gix wrote:
> 
> 
> Hi Vinicius,
> 
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Vinicius Costa Gomes
> > Sent: 06 December, 2010 1:44 PM
> > To: linux-bluetooth@vger.kernel.org
> > Cc: Anderson Briglia; Vinicius Costa Gomes
> > Subject: [RFC v2 4/9] Bluetooth: simple SMP pairing negotiation
> > 
> > From: Anderson Briglia <anderson.briglia@openbossa.org>
> > 
> > This implementation only exchanges SMP messages between the Host and
> > the
> > Remote. No keys are being generated. TK and STK generation will be
> > provided in further patches.
> > 
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> > ---
> >  net/bluetooth/l2cap_core.c |    3 +-
> >  net/bluetooth/smp.c        |  114
> > ++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 112 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 674799c..da4f13d 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4630,7 +4630,8 @@ static void l2cap_recv_frame(struct l2cap_conn
> > *conn, struct sk_buff *skb)
> >  		break;
> > 
> >  	case L2CAP_CID_SMP:
> > -		smp_sig_channel(conn, skb);
> > +		if (smp_sig_channel(conn, skb))
> > +			l2cap_conn_del(conn->hcon, 0x05);
> >  		break;
> > 
> >  	default:
> > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> > index e9dde5f..b25010f 100644
> > --- a/net/bluetooth/smp.c
> > +++ b/net/bluetooth/smp.c
> > @@ -64,6 +64,102 @@ static void smp_send_cmd(struct l2cap_conn *conn,
> > u8 code, u16 len, void *data)
> >  	hci_send_acl(conn->hcon, skb, 0);
> >  }
> > 
> > +static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct
> > sk_buff *skb)
> > +{
> > +	struct smp_cmd_pairing *rp = (void *) skb->data;
> > +
> > +	BT_DBG("");
> > +
> > +	skb_pull(skb, sizeof(struct smp_cmd_pairing));
> > +
> > +	rp->io_capability = 0x00;
> > +	rp->oob_flag = 0x00;
> > +	rp->max_key_size = 16;
> > +	rp->init_key_dist = 0x00;
> > +	rp->resp_key_dist = 0x00;
> > +	rp->auth_req &= 0x05;
> > +
> > +	smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
> > +}
> 
> As a "placeholder" I understand that there is a fair amount of fleshing
> out that these changes need.  However, as you have an conn->hcon->out
> flag that indicates direction (which hopefully is based on Link Master),
> I would like to see checking in this function and next, that the
> correct role has received these SMP packets, with a rejection if they
> were received by the incorrect role. Also, although the placeholder is
> requesting no key distribution, in the fleshed out version, the responder
> should be returning the subset (logical AND) of the requesters and the
> responders key_dist masks, which in this case is still of course Zero.
> 

Yeah, that kind of protocol checking is something that is really lacking.

This RFC is just the implementation of the Just Works pairing procedure,
without any support for key distribution. And as you noted, many things were
implemented using this assumption.

> I'm sorry if this is to many comments for this starting point.

Keep them coming :-) they are being very helpful.

> 
> > +
> > +static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct
> > sk_buff *skb)
> > +{
> > +	struct smp_cmd_pairing_confirm cp;
> > +
> > +	BT_DBG("");
> > +
> > +	memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
> > +
> > +	smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
> > +}
> > +
> > +static void smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct
> > sk_buff *skb)
> > +{
> > +	BT_DBG("");
> > +
> > +	if (conn->hcon->out) {
> > +		struct smp_cmd_pairing_random random;
> > +
> > +		BT_DBG("master");
> > +
> > +		memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
> > +
> > +		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
> > +								&random);
> > +	} else {
> > +		struct smp_cmd_pairing_confirm confirm;
> > +
> > +		BT_DBG("slave");
> > +
> > +		memset(&confirm, 0, sizeof(struct
> > smp_cmd_pairing_confirm));
> > +
> > +		smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM,
> > sizeof(confirm),
> > +								&confirm);
> > +	}
> > +}
> > +
> > +static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct
> > sk_buff *skb)
> > +{
> > +	struct smp_cmd_pairing_random cp;
> > +
> > +	BT_DBG("");
> > +
> > +	skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
> > +
> > +	/* FIXME: check if random matches */
> 
> The random numbers will not match. The correct check will be that
> when the encryption with p1, p2, k, and the remote's random number,
> is performed, that it matches the confirm previously received
> via smp_cmd_pairing_confirm.

The comment is wrong. Will fix.

> 
> > +
> > +	if (conn->hcon->out) {
> > +		BT_DBG("master");
> > +		/* FIXME: start encryption */
> > +	} else {
> > +		BT_DBG("slave");
> > +
> > +		memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
> > +
> > +		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp),
> > &cp);
> > +	}
> > +}
> > +
> > +static void smp_cmd_security_req(struct l2cap_conn *conn, struct
> > sk_buff *skb)
> > +{
> > +	struct smp_cmd_security_req *rp = (void *) skb->data;
> > +	struct smp_cmd_pairing cp;
> > +
> > +	BT_DBG("");
> > +
> > +	skb_pull(skb, sizeof(struct smp_cmd_security_req));
> > +	memset(&cp, 0, sizeof(struct smp_cmd_pairing));
> > +
> > +	cp.io_capability = 0x00;
> > +	cp.oob_flag = 0x00;
> > +	cp.max_key_size = 16;
> > +	cp.init_key_dist = 0x00;
> > +	cp.resp_key_dist = 0x00;
> > +	cp.auth_req = rp->auth_req & 0x05;
> > +
> > +	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
> > +}
> > +
> 
> This function may need to be overloaded, such that if an existing
> set of keys already exist (from an earlier pairing) that they are
> used by simply encrypting the link, or signing the WRITE_CMD pkt
> as needed.  Should the link encryption fail due to remote rejection,
> we might then request security, subject to the same limitations
> used by BR/EDR's SSP. 

This particular function is just for the actual SMP Security Request Command.
But yeah, we need to have a single starting point for both when we have the
keys or not. How signing will be implemented is still an open point on my
mind.

> 
> But I do not know where the division lies between the key storage dB,
> the kernel mode code and the user mode code.
> 
> 
> >  int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
> >  {
> >  	__u8 authreq;
> > @@ -114,23 +210,33 @@ int smp_sig_channel(struct l2cap_conn *conn,
> > struct sk_buff *skb)
> > 
> >  	switch (code) {
> >  	case SMP_CMD_PAIRING_REQ:
> > -		reason = SMP_PAIRING_NOTSUPP;
> > -		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
> > -		err = -1;
> > +		smp_cmd_pairing_req(conn, skb);
> >  		break;
> > 
> >  	case SMP_CMD_PAIRING_FAIL:
> >  		break;
> > 
> >  	case SMP_CMD_PAIRING_RSP:
> > +		smp_cmd_pairing_rsp(conn, skb);
> > +		break;
> > +
> > +	case SMP_CMD_SECURITY_REQ:
> > +		smp_cmd_security_req(conn, skb);
> > +		break;
> > +
> >  	case SMP_CMD_PAIRING_CONFIRM:
> > +		smp_cmd_pairing_confirm(conn, skb);
> > +		break;
> > +
> >  	case SMP_CMD_PAIRING_RANDOM:
> > +		smp_cmd_pairing_random(conn, skb);
> > +		break;
> > +
> >  	case SMP_CMD_ENCRYPT_INFO:
> >  	case SMP_CMD_MASTER_IDENT:
> >  	case SMP_CMD_IDENT_INFO:
> >  	case SMP_CMD_IDENT_ADDR_INFO:
> >  	case SMP_CMD_SIGN_INFO:
> > -	case SMP_CMD_SECURITY_REQ:
> >  	default:
> >  		BT_DBG("Unknown command code 0x%2.2x", code);
> > 
> > --
> > 1.7.3.2
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > bluetooth" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Cheers,
-- 
Vinicius

^ permalink raw reply

* Re: [PATCH] Remove fixed item from TODO
From: Johan Hedberg @ 2010-12-07 22:24 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1291760437-32216-1-git-send-email-claudio.takahasi@openbossa.org>

Hi Claudio,

On Tue, Dec 07, 2010, Claudio Takahasi wrote:
> Read by UUID is already supported by attribute server and gatttool.
> ---
>  TODO |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)

Pushed upstream. Thanks.

Johan

^ 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