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