linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bluez-devel] bug in sdp_gen_pd
@ 2006-06-06 10:25 Rafael Espíndola
  2006-06-07 21:10 ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-06 10:25 UTC (permalink / raw)
  To: bluez-devel

In sdp_gen_pd when handling SDP_TEXT_STR{8,16,32}, data_size is assumed to be
"d->unitSize - sizeof(uint8_t)". This is false.

In sdp_data_alloc_with_length, d->unitSize is defined to be
sizeof(unit8_t) + length + sizeof(uint8_t) if length <= UCHAR_MAX
or
sizeof(unit8_t) + length + sizeof(uint16_t) if length > UCHAR_MAX

The attached patch fixes sdp_gen_pdu to correctly compute data_size.

Another strange thing in sdp_data_alloc_with_length: after adjusting
unitSize, the dtd variable is changed from SDP_*_STR8 to SDP_*_STR16
or the other way around. But this code is dead, since the dtd variable
is no longer used in this function.

Best Regards,
Rafael


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bluez-devel] bug in sdp_gen_pd
@ 2006-06-06 14:14 Rafael Espíndola
  0 siblings, 0 replies; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-06 14:14 UTC (permalink / raw)
  To: bluez-devel, ulissesf@gmail.com

In sdp_gen_pd when handling SDP_TEXT_STR{8,16,32}, data_size is assumed to be
"d->unitSize - sizeof(uint8_t)". This is false.

In sdp_data_alloc_with_length, d->unitSize is defined to be
sizeof(unit8_t) + length + sizeof(uint8_t) if length <= UCHAR_MAX
or
sizeof(unit8_t) + length + sizeof(uint16_t) if length > UCHAR_MAX

The attached patch fixes sdp_gen_pdu to correctly compute data_size.

Another strange thing in sdp_data_alloc_with_length: after adjusting
unitSize, the dtd variable is changed from SDP_*_STR8 to SDP_*_STR16
or the other way around. But this code is dead, since the dtd variable
is no longer used in this function.

Best Regards,
Rafael


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-06 10:25 [Bluez-devel] bug in sdp_gen_pd Rafael Espíndola
@ 2006-06-07 21:10 ` Marcel Holtmann
  2006-06-07 21:59   ` Rafael Espíndola
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-07 21:10 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> In sdp_gen_pd when handling SDP_TEXT_STR{8,16,32}, data_size is assumed to be
> "d->unitSize - sizeof(uint8_t)". This is false.
> 
> In sdp_data_alloc_with_length, d->unitSize is defined to be
> sizeof(unit8_t) + length + sizeof(uint8_t) if length <= UCHAR_MAX
> or
> sizeof(unit8_t) + length + sizeof(uint16_t) if length > UCHAR_MAX
> 
> The attached patch fixes sdp_gen_pdu to correctly compute data_size.

first you forgot the patch and second, are you sure. The SDP code is
actually messy and some stuff is in there that actually works, but is
not quite obvious. Can you provide a simple test program?

> Another strange thing in sdp_data_alloc_with_length: after adjusting
> unitSize, the dtd variable is changed from SDP_*_STR8 to SDP_*_STR16
> or the other way around. But this code is dead, since the dtd variable
> is no longer used in this function.

Never looked at it actually, but you might be right. However again,
there might be a really strange reason for it.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-07 21:10 ` Marcel Holtmann
@ 2006-06-07 21:59   ` Rafael Espíndola
  2006-06-07 22:11     ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-07 21:59 UTC (permalink / raw)
  To: BlueZ development

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

> first you forgot the patch
Sorry, the first email was lost. I forgot to attach when I resent the email.

> and second, are you sure. The SDP code is
> actually messy and some stuff is in there that actually works, but is
> not quite obvious. Can you provide a simple test program?

I found the bug while running a mid sized program in valgrind. The
relevant part of the log is
==26360== Invalid read of size 1
==26360==    at 0x4A1BAA0: memcpy (mac_replace_strmem.c:394)
==26360==    by 0x523124D: sdp_gen_pdu (in /usr/lib/libbluetooth.so.1.0.24)
==26360==    by 0x5231D18: sdp_append_to_pdu (in
/usr/lib/libbluetooth.so.1.0.24)
==26360==    by 0x522E46B: sdp_gen_record_pdu (in
/usr/lib/libbluetooth.so.1.0.24)
==26360==    by 0x52305D2: sdp_device_record_register (in
/usr/lib/libbluetooth.so.1.0.24)
.....
==26360==    at 0x4A19A16: malloc (vg_replace_malloc.c:149)
==26360==    by 0x522DE07: sdp_data_alloc_with_length (in
/usr/lib/libbluetooth.so.1.0.24)
==26360==    by 0x523078D: sdp_attr_add_new (in /usr/lib/libbluetooth.so.1.0.24)
==26360==    by 0x52309A2: sdp_set_info_attr (in
/usr/lib/libbluetooth.so.1.0.24)
--------------------------------------------------

If it is really necessary I can try to build a "small" test program.

> Never looked at it actually, but you might be right. However again,
> there might be a really strange reason for it.
For dead code?

> Regards
>
> Marcel

Best Regards,
Rafael

[-- Attachment #2: bluez-libs.patch --]
[-- Type: application/octet-stream, Size: 431 bytes --]

--- src/sdp.c	2005-12-24 14:03:38.000000000 -0300
+++ /home/rafael/sdp.c	2006-06-05 17:48:33.000000000 -0300
@@ -740,6 +740,10 @@
 	case SDP_TEXT_STR32:
 		src = (unsigned char *)d->val.str;
 		data_size = d->unitSize - sizeof(uint8_t);
+		if (data_size - sizeof(uint8_t) <= UCHAR_MAX)
+			data_size -= sizeof(uint8_t);
+		else
+			data_size -= sizeof(uint16_t);
 		sdp_set_seq_len(seqp, data_size);
 		break;
 	case SDP_URL_STR8:

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-07 21:59   ` Rafael Espíndola
@ 2006-06-07 22:11     ` Marcel Holtmann
  2006-06-08 17:07       ` Rafael Espíndola
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-07 22:11 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> > and second, are you sure. The SDP code is
> > actually messy and some stuff is in there that actually works, but is
> > not quite obvious. Can you provide a simple test program?
> 
> I found the bug while running a mid sized program in valgrind. The
> relevant part of the log is
> ==26360== Invalid read of size 1
> ==26360==    at 0x4A1BAA0: memcpy (mac_replace_strmem.c:394)
> ==26360==    by 0x523124D: sdp_gen_pdu (in /usr/lib/libbluetooth.so.1.0.24)
> ==26360==    by 0x5231D18: sdp_append_to_pdu (in
> /usr/lib/libbluetooth.so.1.0.24)
> ==26360==    by 0x522E46B: sdp_gen_record_pdu (in
> /usr/lib/libbluetooth.so.1.0.24)
> ==26360==    by 0x52305D2: sdp_device_record_register (in
> /usr/lib/libbluetooth.so.1.0.24)
> .....
> ==26360==    at 0x4A19A16: malloc (vg_replace_malloc.c:149)
> ==26360==    by 0x522DE07: sdp_data_alloc_with_length (in
> /usr/lib/libbluetooth.so.1.0.24)
> ==26360==    by 0x523078D: sdp_attr_add_new (in /usr/lib/libbluetooth.so.1.0.24)
> ==26360==    by 0x52309A2: sdp_set_info_attr (in
> /usr/lib/libbluetooth.so.1.0.24)
> --------------------------------------------------
> 
> If it is really necessary I can try to build a "small" test program.

it would help a lot. I am a little bit worried of such off-by-one
errors, because as I said, the SDP code is a strange code. You might
break some other part of the code that rely on this being wrong. Don't
get me wrong, it is still a bug, but we need to take care to fix both
places. And I have seen such stuff in the SDP code before. This is why I
am extra careful.

> > Never looked at it actually, but you might be right. However again,
> > there might be a really strange reason for it.
> For dead code?

Send in a patch. I will check it.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-07 22:11     ` Marcel Holtmann
@ 2006-06-08 17:07       ` Rafael Espíndola
  2006-06-09 20:14         ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-08 17:07 UTC (permalink / raw)
  To: BlueZ development

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

> it would help a lot. I am a little bit worried of such off-by-one
> errors, because as I said, the SDP code is a strange code. You might
> break some other part of the code that rely on this being wrong. Don't
> get me wrong, it is still a bug, but we need to take care to fix both
> places. And I have seen such stuff in the SDP code before. This is why I
> am extra careful.
ok. This is a simple program that has the bug:
----------------------------------------------------------
#include <sdp.h>
#include <sdp_lib.h>

int main() {
        uint8_t     dtd = SDP_URL_STR8;
        char     *value = "abc";

        sdp_data_t *d = sdp_data_alloc(dtd, value);

        sdp_buf_t buf;
        uint8_t   data[1000];
        buf.data      = data;
        buf.buf_size  = sizeof(data);
        buf.data_size = 0;

        sdp_gen_pdu(&buf, d);
        return 0;
}
-------------------------------------------------------------------

> Send in a patch. I will check it.
Attached.

> Regards
>
> Marcel

Best Regards,
Rafael

[-- Attachment #2: sdp.patch --]
[-- Type: application/octet-stream, Size: 593 bytes --]

--- temp/bluez-libs-2.24/src/sdp.c	2005-12-24 14:03:38.000000000 -0300
+++ sdp.c	2006-06-08 14:04:11.000000000 -0300
@@ -433,18 +433,8 @@
 
 			if (length <= UCHAR_MAX) {
 				d->unitSize += sizeof(uint8_t);
-				if (dtd != SDP_URL_STR8 && dtd != SDP_TEXT_STR8) {
-					if (dtd == SDP_URL_STR16)
-						dtd = SDP_URL_STR8;
-					else
-						dtd = SDP_TEXT_STR8;
-				}
 			} else {
 				d->unitSize += sizeof(uint16_t);
-				if (dtd == SDP_TEXT_STR8)
-					dtd = SDP_TEXT_STR16;
-				else
-					dtd = SDP_URL_STR16;
 			}
 		} else {
 			SDPERR("Strings of size > USHRT_MAX not supported\n");

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-08 17:07       ` Rafael Espíndola
@ 2006-06-09 20:14         ` Marcel Holtmann
  2006-06-12  8:01           ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-09 20:14 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> > it would help a lot. I am a little bit worried of such off-by-one
> > errors, because as I said, the SDP code is a strange code. You might
> > break some other part of the code that rely on this being wrong. Don't
> > get me wrong, it is still a bug, but we need to take care to fix both
> > places. And I have seen such stuff in the SDP code before. This is why I
> > am extra careful.
> ok. This is a simple program that has the bug:
> ----------------------------------------------------------
> #include <sdp.h>
> #include <sdp_lib.h>
> 
> int main() {
>         uint8_t     dtd = SDP_URL_STR8;
>         char     *value = "abc";
> 
>         sdp_data_t *d = sdp_data_alloc(dtd, value);
> 
>         sdp_buf_t buf;
>         uint8_t   data[1000];
>         buf.data      = data;
>         buf.buf_size  = sizeof(data);
>         buf.data_size = 0;
> 
>         sdp_gen_pdu(&buf, d);
>         return 0;
> }
> -------------------------------------------------------------------

I applied the patch. Thanks.

> > Send in a patch. I will check it.
> Attached.

Will look into that after the 3.0 release. If I forget it, please remind
me.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-09 20:14         ` Marcel Holtmann
@ 2006-06-12  8:01           ` Marcel Holtmann
  2006-06-12 11:52             ` Rafael Espíndola
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-12  8:01 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> > > Send in a patch. I will check it.
> > Attached.
> 
> Will look into that after the 3.0 release. If I forget it, please remind
> me.

please use the latest bluez-utils-3.0 release and call this three
commands:

	sdptool add sp
	sdptool browse local
	sdptool browse --raw local

You will see that the string values are wrong for the third commands.
Can you please check what happens here.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-12  8:01           ` Marcel Holtmann
@ 2006-06-12 11:52             ` Rafael Espíndola
  2006-06-19 11:07               ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-12 11:52 UTC (permalink / raw)
  To: BlueZ development

> please use the latest bluez-utils-3.0 release and call this three
> commands:
>
>         sdptool add sp
>         sdptool browse local
>         sdptool browse --raw local
>
> You will see that the string values are wrong for the third commands.
> Can you please check what happens here.
I am unable to reproduce. I have just compiled bluez-libs-3.0 and
bluz-utils-3.0 (using the just compiled lib) and the output is equal
to the installed 2.25.

> Regards
>
> Marcel

Best Regards,

--
Rafael,
INdT


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-12 11:52             ` Rafael Espíndola
@ 2006-06-19 11:07               ` Marcel Holtmann
  2006-06-19 12:13                 ` Rafael Espíndola
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-19 11:07 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> > please use the latest bluez-utils-3.0 release and call this three
> > commands:
> >
> >         sdptool add sp
> >         sdptool browse local
> >         sdptool browse --raw local
> >
> > You will see that the string values are wrong for the third commands.
> > Can you please check what happens here.
> I am unable to reproduce. I have just compiled bluez-libs-3.0 and
> bluz-utils-3.0 (using the just compiled lib) and the output is equal
> to the installed 2.25.

please use bluez-libs-3.1 and bluez-utils-3.1 and make sure that the
sdpd from this release is running and you use its sdptool. After that
issues the following commands:

# sdptool add sp
Serial Port service registered
# sdptool browse local
Browsing FF:FF:FF:00:00:00 ...
Service Name: Serial Po
Service Description: COM Po
Service RecHandle: 0x10000
Service Class ID List:
  "Serial Port" (0x1101)
Protocol Descriptor List:
  "L2CAP" (0x0100)
  "RFCOMM" (0x0003)
    Channel: 1
Language Base Attr List:
  code_ISO639: 0x656e
  encoding:    0x6a
  base_offset: 0x100
Profile Descriptor List:
  "Serial Port" (0x1101)
    Version: 0x0100

# sdptool browse --tree local
Browsing FF:FF:FF:00:00:00 ...
Attribute Identifier : 0x0 - ServiceRecordHandle
  Integer : 0x10000
Attribute Identifier : 0x1 - ServiceClassIDList
  Data Sequence
    UUID16 : 0x1101 - SerialPort
Attribute Identifier : 0x4 - ProtocolDescriptorList
  Data Sequence
    Data Sequence
      UUID16 : 0x0100 - L2CAP
    Data Sequence
      UUID16 : 0x0003 - RFCOMM
      Channel/Port (Integer) : 0x1
Attribute Identifier : 0x5 - BrowseGroupList
  Data Sequence
    UUID16 : 0x1002 - PublicBrowseGroup
Attribute Identifier : 0x6 - LanguageBaseAttributeIDList
  Data Sequence
    Code ISO639 (Integer) : 0x656e
    Encoding (Integer) : 0x6a
    Base Offset (Integer) : 0x100
Attribute Identifier : 0x9 - BluetoothProfileDescriptorList
  Data Sequence
    Data Sequence
      UUID16 : 0x1101 - SerialPort
      Version (Integer) : 0x100
Attribute Identifier : 0x100
  Text : "Serial Po"
Attribute Identifier : 0x101
  Text : "COM Po"

# sdptool browse --raw local
Sequence
        Attribute 0x0000 - ServiceRecordHandle
                UINT32 0x00010000
        Attribute 0x0001 - ServiceClassIDList
                Sequence
                        UUID16 0x1101 - SerialPort
        Attribute 0x0004 - ProtocolDescriptorList
                Sequence
                        Sequence
                                UUID16 0x0100 - L2CAP
                        Sequence
                                UUID16 0x0003 - RFCOMM
                                UINT8 0x01
        Attribute 0x0005 - BrowseGroupList
                Sequence
                        UUID16 0x1002 - PublicBrowseGroup
        Attribute 0x0006 - LanguageBaseAttributeIDList
                Sequence
                        UINT16 0x656e
                        UINT16 0x006a
                        UINT16 0x0100
        Attribute 0x0009 - BluetoothProfileDescriptorList
                Sequence
                        Sequence
                                UUID16 0x1101 - SerialPort
                                UINT16 0x0100
        Attribute 0x0100
                String Serial Po
        Attribute 0x0101
                String COM Po

You will see that attributes 0x100 and 0x101 are not matching.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 11:07               ` Marcel Holtmann
@ 2006-06-19 12:13                 ` Rafael Espíndola
  2006-06-19 12:17                   ` Rafael Espíndola
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-19 12:13 UTC (permalink / raw)
  To: BlueZ development

On 6/19/06, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Rafael,
Hi Marcel,

>
> please use bluez-libs-3.1 and bluez-utils-3.1 and make sure that the
> sdpd from this release is running and you use its sdptool. After that
> issues the following commands:
.....

> You will see that attributes 0x100 and 0x101 are not matching.
I am sorry, but due to my lack of experience with bluetooth I don't
know what is wrong with the output.

I have run the commands in vanilla 2.24-0ubuntu1 packages and with my
patch attached. The output was the same.

> Regards
>
> Marcel

Best Regards,
Rafael


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 12:13                 ` Rafael Espíndola
@ 2006-06-19 12:17                   ` Rafael Espíndola
  2006-06-19 17:38                     ` Rafael Espíndola
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-19 12:17 UTC (permalink / raw)
  To: BlueZ development

> I have run the commands in vanilla 2.24-0ubuntu1 packages and with my
> patch attached. The output was the same.

Sorry again :-(
restarting the services I notice the Port -> Po.

I will investigate.

Rafael


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 12:17                   ` Rafael Espíndola
@ 2006-06-19 17:38                     ` Rafael Espíndola
  2006-06-19 18:07                       ` Marcel Holtmann
  2006-06-26 12:49                       ` Marcel Holtmann
  0 siblings, 2 replies; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-19 17:38 UTC (permalink / raw)
  To: BlueZ development

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

> Sorry again :-(
> restarting the services I notice the Port -> Po.
>
> I will investigate.

A patch is attached. It creates the following invariant when handling strings:

d->unitSize == strlen(d->val.str) + sizeof(uint8_t)

Different parts of the code had different "opinions".

Rafael,
INdT

[-- Attachment #2: sdp-3.1.patch --]
[-- Type: application/octet-stream, Size: 1111 bytes --]

--- src/sdp.c	2006-06-09 12:45:08.000000000 -0300
+++ /home/rafael/sdp-3.1.c	2006-06-19 14:17:41.000000000 -0300
@@ -432,22 +432,6 @@
 			}
 
 			memcpy(d->val.str, value, length);
-
-			if (length <= UCHAR_MAX) {
-				d->unitSize += sizeof(uint8_t);
-				if (dtd != SDP_URL_STR8 && dtd != SDP_TEXT_STR8) {
-					if (dtd == SDP_URL_STR16)
-						dtd = SDP_URL_STR8;
-					else
-						dtd = SDP_TEXT_STR8;
-				}
-			} else {
-				d->unitSize += sizeof(uint16_t);
-				if (dtd == SDP_TEXT_STR8)
-					dtd = SDP_TEXT_STR16;
-				else
-					dtd = SDP_URL_STR16;
-			}
 		} else {
 			SDPERR("Strings of size > USHRT_MAX not supported\n");
 			free(d);
@@ -742,10 +726,6 @@
 	case SDP_TEXT_STR32:
 		src = (unsigned char *)d->val.str;
 		data_size = d->unitSize - sizeof(uint8_t);
-		if (data_size - sizeof(uint8_t) <= UCHAR_MAX)
-			data_size -= sizeof(uint8_t);
-		else
-			data_size -= sizeof(uint16_t);
 		sdp_set_seq_len(seqp, data_size);
 		break;
 	case SDP_URL_STR8:
@@ -973,7 +953,7 @@
 	SDPDBG("Str : %s\n", s);
 
 	d->val.str = s;
-	d->unitSize = n;
+	d->unitSize = n + sizeof(uint8_t);
 	return d;
 }
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 17:38                     ` Rafael Espíndola
@ 2006-06-19 18:07                       ` Marcel Holtmann
  2006-06-19 20:21                         ` Rafael Espíndola
  2006-06-26 12:49                       ` Marcel Holtmann
  1 sibling, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-19 18:07 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> A patch is attached. It creates the following invariant when handling strings:
> 
> d->unitSize == strlen(d->val.str) + sizeof(uint8_t)
> 
> Different parts of the code had different "opinions".

please also check hidd, because the HID report descriptor is placed in a
string and we need to know the exact string length.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 18:07                       ` Marcel Holtmann
@ 2006-06-19 20:21                         ` Rafael Espíndola
  2006-06-20  7:17                           ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2006-06-19 20:21 UTC (permalink / raw)
  To: BlueZ development

On 6/19/06, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Rafael,
Hi Marcel,

> please also check hidd, because the HID report descriptor is placed in a
> string and we need to know the exact string length.

I tested with a Nokia bluetooth keyboard. I am not sure what the "HID
report descriptor" is.


> Regards
>
> Marcel

Best Regards,
Rafael
INdT


_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 20:21                         ` Rafael Espíndola
@ 2006-06-20  7:17                           ` Marcel Holtmann
  0 siblings, 0 replies; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-20  7:17 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> > please also check hidd, because the HID report descriptor is placed in a
> > string and we need to know the exact string length.
> 
> I tested with a Nokia bluetooth keyboard. I am not sure what the "HID
> report descriptor" is.

if you run a "sdptool search --bdaddr <bdaddr> --raw hid" you will see
an attribute with a byte stream. That is the HID report descriptor.
However you need to test this with the latest -mh kernel and you need to
make sure that /var/lib/bluetooth/<bdaddr>/hidd is empty.

Regards

Marcel




_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Bluez-devel] bug in sdp_gen_pd
  2006-06-19 17:38                     ` Rafael Espíndola
  2006-06-19 18:07                       ` Marcel Holtmann
@ 2006-06-26 12:49                       ` Marcel Holtmann
  1 sibling, 0 replies; 17+ messages in thread
From: Marcel Holtmann @ 2006-06-26 12:49 UTC (permalink / raw)
  To: BlueZ development

Hi Rafael,

> A patch is attached. It creates the following invariant when handling strings:
> 
> d->unitSize == strlen(d->val.str) + sizeof(uint8_t)
> 
> Different parts of the code had different "opinions".

the patch is in the CVE nos and it makes it better, but I am still not
fully happy. When creating strings in SDP you don't need to include the
terminating NUL character, because every string is prefixed with a
length.

# sdptool add sp
Serial Port service registered

# sdptool browse local
Browsing FF:FF:FF:00:00:00 ...
Service Name: Serial Port
Service Description: COM Port
Service RecHandle: 0x10000
Service Class ID List:
  "Serial Port" (0x1101)
Protocol Descriptor List:
  "L2CAP" (0x0100)
  "RFCOMM" (0x0003)
    Channel: 1
Language Base Attr List:
  code_ISO639: 0x656e
  encoding:    0x6a
  base_offset: 0x100
Profile Descriptor List:
  "Serial Port" (0x1101)
    Version: 0x0100

# sdptool browse --tree local
Browsing FF:FF:FF:00:00:00 ...
Attribute Identifier : 0x0 - ServiceRecordHandle
  Integer : 0x10000
Attribute Identifier : 0x1 - ServiceClassIDList
  Data Sequence
    UUID16 : 0x1101 - SerialPort
Attribute Identifier : 0x4 - ProtocolDescriptorList
  Data Sequence
    Data Sequence
      UUID16 : 0x0100 - L2CAP
    Data Sequence
      UUID16 : 0x0003 - RFCOMM
      Channel/Port (Integer) : 0x1
Attribute Identifier : 0x5 - BrowseGroupList
  Data Sequence
    UUID16 : 0x1002 - PublicBrowseGroup
Attribute Identifier : 0x6 - LanguageBaseAttributeIDList
  Data Sequence
    Code ISO639 (Integer) : 0x656e
    Encoding (Integer) : 0x6a
    Base Offset (Integer) : 0x100
Attribute Identifier : 0x9 - BluetoothProfileDescriptorList
  Data Sequence
    Data Sequence
      UUID16 : 0x1101 - SerialPort
      Version (Integer) : 0x100
Attribute Identifier : 0x100
  Data : 53 65 72 69 61 6c 20 50 6f 72 74 00
Attribute Identifier : 0x101
  Data : 43 4f 4d 20 50 6f 72 74 00

# sdptool browse --raw local
Sequence
        Attribute 0x0000 - ServiceRecordHandle
                UINT32 0x00010000
        Attribute 0x0001 - ServiceClassIDList
                Sequence
                        UUID16 0x1101 - SerialPort
        Attribute 0x0004 - ProtocolDescriptorList
                Sequence
                        Sequence
                                UUID16 0x0100 - L2CAP
                        Sequence
                                UUID16 0x0003 - RFCOMM
                                UINT8 0x01
        Attribute 0x0005 - BrowseGroupList
                Sequence
                        UUID16 0x1002 - PublicBrowseGroup
        Attribute 0x0006 - LanguageBaseAttributeIDList
                Sequence
                        UINT16 0x656e
                        UINT16 0x006a
                        UINT16 0x0100
        Attribute 0x0009 - BluetoothProfileDescriptorList
                Sequence
                        Sequence
                                UUID16 0x1101 - SerialPort
                                UINT16 0x0100
        Attribute 0x0100
                String Serial Port
        Attribute 0x0101
                String COM Port

As you see, the --tree option is too dumb to detect that it is a string.
This has been left there on purpose to find stupid SDP records that
include a NUL character at the end.

Please try to fix this, too.

Regards

Marcel



Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2006-06-26 12:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-06 10:25 [Bluez-devel] bug in sdp_gen_pd Rafael Espíndola
2006-06-07 21:10 ` Marcel Holtmann
2006-06-07 21:59   ` Rafael Espíndola
2006-06-07 22:11     ` Marcel Holtmann
2006-06-08 17:07       ` Rafael Espíndola
2006-06-09 20:14         ` Marcel Holtmann
2006-06-12  8:01           ` Marcel Holtmann
2006-06-12 11:52             ` Rafael Espíndola
2006-06-19 11:07               ` Marcel Holtmann
2006-06-19 12:13                 ` Rafael Espíndola
2006-06-19 12:17                   ` Rafael Espíndola
2006-06-19 17:38                     ` Rafael Espíndola
2006-06-19 18:07                       ` Marcel Holtmann
2006-06-19 20:21                         ` Rafael Espíndola
2006-06-20  7:17                           ` Marcel Holtmann
2006-06-26 12:49                       ` Marcel Holtmann
  -- strict thread matches above, loose matches on Subject: below --
2006-06-06 14:14 Rafael Espíndola

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).