2011-11-30 16:36:16

by Santiago Carot

Subject: [PATCH] Fix byte order problems getting PSM through btio API

---
btio/btio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index a129bf9..f8c5cc7 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -882,7 +882,7 @@ static gboolean l2cap_get(int sock, GError **err, BtIOOption opt1,
break;
case BT_IO_OPT_PSM:
*(va_arg(args, uint16_t *)) = src.l2_psm ?
- src.l2_psm : dst.l2_psm;
+ btohs(src.l2_psm) : btohs(dst.l2_psm);
break;
case BT_IO_OPT_CID:
*(va_arg(args, uint16_t *)) = src.l2_cid ?
--
1.7.7.4

2011-11-30 21:16:00

by Johan Hedberg

Subject: Re: [PATCH] Fix byte order problems getting PSM through btio API

Hi Santiago,

On Wed, Nov 30, 2011, Santiago Carot wrote:
> 2011/11/30 Santiago Carot-Nemesio <[email protected]>:
> > ---
> > ?btio/btio.c | ? ?2 +-
> > ?1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/btio/btio.c b/btio/btio.c
> > index a129bf9..f8c5cc7 100644
> > --- a/btio/btio.c
> > +++ b/btio/btio.c
> > @@ -882,7 +882,7 @@ static gboolean l2cap_get(int sock, GError **err, BtIOOption opt1,
> > ? ? ? ? ? ? ? ? ? ? ? ?break;
> > ? ? ? ? ? ? ? ?case BT_IO_OPT_PSM:
> > ? ? ? ? ? ? ? ? ? ? ? ?*(va_arg(args, uint16_t *)) = src.l2_psm ?
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? src.l2_psm : dst.l2_psm;
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? btohs(src.l2_psm) : btohs(dst.l2_psm);
> > ? ? ? ? ? ? ? ? ? ? ? ?break;
> > ? ? ? ? ? ? ? ?case BT_IO_OPT_CID:
> > ? ? ? ? ? ? ? ? ? ? ? ?*(va_arg(args, uint16_t *)) = src.l2_cid ?
> > --
> > 1.7.7.4
> >
>
> At a first sight, It seems there may be the same problems retrieving
> CID, OMTU and IMTU on big endian architectures. I preffer waiting for
> a more experienced comments before sending any patch fixing that.

To my understanding the L2CAP PSM is the only parameter that uses
protocol endianess in the socket interface, so we should be safe with
the other values.

Johan

2011-11-30 16:44:21

by Santiago Carot

Subject: Re: [PATCH] Fix byte order problems getting PSM through btio API

Hello,

2011/11/30 Santiago Carot-Nemesio <[email protected]>:
> ---
> ?btio/btio.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index a129bf9..f8c5cc7 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -882,7 +882,7 @@ static gboolean l2cap_get(int sock, GError **err, BtIOOption opt1,
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case BT_IO_OPT_PSM:
> ? ? ? ? ? ? ? ? ? ? ? ?*(va_arg(args, uint16_t *)) = src.l2_psm ?
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? src.l2_psm : dst.l2_psm;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? btohs(src.l2_psm) : btohs(dst.l2_psm);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case BT_IO_OPT_CID:
> ? ? ? ? ? ? ? ? ? ? ? ?*(va_arg(args, uint16_t *)) = src.l2_cid ?
> --
> 1.7.7.4
>

At a first sight, It seems there may be the same problems retrieving
CID, OMTU and IMTU on big endian architectures. I preffer waiting for
a more experienced comments before sending any patch fixing that.

Regards.

2011-12-01 20:34:34

by Johan Hedberg

Subject: Re: [PATCH] Fix byte order problems getting PSM through btio API

Hi Santiago,

On Thu, Dec 01, 2011, Santiago Carot wrote:
> As far as I see, CID uses little endian as well, There are some parts
> in the code where the byte conversion is done when BT_IO_OPT_CID is
> provided, for example in l2cap_connect, Conversely, this attribute is
> being provided in bt_io_get without host endian conversion.
>
> Diving in the code I can see that the gatt-server is getting it in
> ./src/attrib-server.c:1039 so I guess that there is the same problem
> that we had in big endian architectures.

Please also take a look at the kernel side since that's the only way to
be sure. Last time I checked I saw byte-order conversion for CID but not
for PSM (i.e. PSM was expected in little endian whereas CID wasn't).

Johan

2011-12-01 19:31:58

by Santiago Carot

Subject: Re: [PATCH] Fix byte order problems getting PSM through btio API

Hello Johan,

2011/11/30 Johan Hedberg <[email protected]>:
> Hi Santiago,
>
> On Wed, Nov 30, 2011, Santiago Carot wrote:
>> 2011/11/30 Santiago Carot-Nemesio <[email protected]>:
>> > ---
>> > ?btio/btio.c | ? ?2 +-
>> > ?1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/btio/btio.c b/btio/btio.c
>> > index a129bf9..f8c5cc7 100644
>> > --- a/btio/btio.c
>> > +++ b/btio/btio.c
>> > @@ -882,7 +882,7 @@ static gboolean l2cap_get(int sock, GError **err, BtIOOption opt1,
>> > ? ? ? ? ? ? ? ? ? ? ? ?break;
>> > ? ? ? ? ? ? ? ?case BT_IO_OPT_PSM:
>> > ? ? ? ? ? ? ? ? ? ? ? ?*(va_arg(args, uint16_t *)) = src.l2_psm ?
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? src.l2_psm : dst.l2_psm;
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? btohs(src.l2_psm) : btohs(dst.l2_psm);
>> > ? ? ? ? ? ? ? ? ? ? ? ?break;
>> > ? ? ? ? ? ? ? ?case BT_IO_OPT_CID:
>> > ? ? ? ? ? ? ? ? ? ? ? ?*(va_arg(args, uint16_t *)) = src.l2_cid ?
>> > --
>> > 1.7.7.4
>> >
>>
>> At a first sight, It seems there may be the same problems retrieving
>> CID, OMTU and IMTU on big endian architectures. I preffer waiting for
>> a more experienced comments before sending any patch fixing that.
>
> To my understanding the L2CAP PSM is the only parameter that uses
> protocol endianess in the socket interface, so we should be safe with
> the other values.
>

As far as I see, CID uses little endian as well, There are some parts
in the code where the byte conversion is done when BT_IO_OPT_CID is
provided, for example in l2cap_connect, Conversely, this attribute is
being provided in bt_io_get without host endian conversion.

Diving in the code I can see that the gatt-server is getting it in
./src/attrib-server.c:1039 so I guess that there is the same problem
that we had in big endian architectures.

Regards

Santiago