* [PATCH] lib: Browsing services using sdptool can lead to writing to invalid heap locations
@ 2013-04-04 14:09 Arkadiusz Lichwa
2013-04-04 12:38 ` Johan Hedberg
0 siblings, 1 reply; 3+ messages in thread
From: Arkadiusz Lichwa @ 2013-04-04 14:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arkadiusz Lichwa
valgrind's output of exemplary call: sdptool browse local
==2203== HEAP SUMMARY:
==2203== in use at exit: 0 bytes in 0 blocks
==2203== total heap usage: 251 allocs, 251 frees, 140,156 bytes allocated
==2203==
==2203== All heap blocks were freed -- no leaks are possible
==2203==
==2203== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 0 from 0)
==2203==
==2203== 1 errors in context 1 of 2:
==2203== Invalid write of size 2
==2203== at 0x805B882: bt_put_be16 (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8062BD0: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== Address 0x4391359 is 7 bytes before a block of size 2,048 alloc'd
==2203== at 0x402B6A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2203== by 0x8062B4B: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203==
==2203==
==2203== 5 errors in context 2 of 2:
==2203== Invalid write of size 1
==2203== at 0x402D363: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2203== by 0x80613E7: gen_dataseq_pdu (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8061472: gen_attridseq_pdu (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8062C00: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== Address 0x439135b is 5 bytes before a block of size 2,048 alloc'd
==2203== at 0x402B6A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2203== by 0x8062B4B: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
==2203==
==2203== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 0 from 0)
---
lib/sdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index 6c73818..cc685b4 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -4429,6 +4429,11 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
/* add service class IDs for search */
seqlen = gen_searchseq_pdu(pdata, search);
+ if (seqlen < 0) {
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
SDPDBG("Data seq added : %d\n", seqlen);
--
1.8.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] lib: Browsing services using sdptool can lead to writing to invalid heap locations
2013-04-04 14:09 [PATCH] lib: Browsing services using sdptool can lead to writing to invalid heap locations Arkadiusz Lichwa
@ 2013-04-04 12:38 ` Johan Hedberg
2013-04-04 13:31 ` Arkadiusz Lichwa
0 siblings, 1 reply; 3+ messages in thread
From: Johan Hedberg @ 2013-04-04 12:38 UTC (permalink / raw)
To: Arkadiusz Lichwa; +Cc: linux-bluetooth, Arkadiusz Lichwa
Hi Arek,
On Thu, Apr 04, 2013, Arkadiusz Lichwa wrote:
> valgrind's output of exemplary call: sdptool browse local
>
> ==2203== HEAP SUMMARY:
> ==2203== in use at exit: 0 bytes in 0 blocks
> ==2203== total heap usage: 251 allocs, 251 frees, 140,156 bytes allocated
> ==2203==
> ==2203== All heap blocks were freed -- no leaks are possible
> ==2203==
> ==2203== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 0 from 0)
> ==2203==
> ==2203== 1 errors in context 1 of 2:
> ==2203== Invalid write of size 2
> ==2203== at 0x805B882: bt_put_be16 (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8062BD0: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== Address 0x4391359 is 7 bytes before a block of size 2,048 alloc'd
> ==2203== at 0x402B6A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
> ==2203== by 0x8062B4B: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203==
> ==2203==
> ==2203== 5 errors in context 2 of 2:
> ==2203== Invalid write of size 1
> ==2203== at 0x402D363: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
> ==2203== by 0x80613E7: gen_dataseq_pdu (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8061472: gen_attridseq_pdu (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8062C00: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== Address 0x439135b is 5 bytes before a block of size 2,048 alloc'd
> ==2203== at 0x402B6A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
> ==2203== by 0x8062B4B: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
> ==2203==
> ==2203== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 0 from 0)
> ---
> lib/sdp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/lib/sdp.c b/lib/sdp.c
> index 6c73818..cc685b4 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -4429,6 +4429,11 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
>
> /* add service class IDs for search */
> seqlen = gen_searchseq_pdu(pdata, search);
> + if (seqlen < 0) {
> + errno = EINVAL;
> + status = -1;
> + goto end;
> + }
>
> SDPDBG("Data seq added : %d\n", seqlen);
The patch looks correct and I've applied it, but could you explain how
exactly you reproduced the issue? I didn't get any valgrind warnings
when trying your suggested "sdptool browse local".
Johan
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] lib: Browsing services using sdptool can lead to writing to invalid heap locations
2013-04-04 12:38 ` Johan Hedberg
@ 2013-04-04 13:31 ` Arkadiusz Lichwa
0 siblings, 0 replies; 3+ messages in thread
From: Arkadiusz Lichwa @ 2013-04-04 13:31 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Arkadiusz Lichwa, linux-bluetooth@vger.kernel.org
Hi Johan
On 04/04/2013 02:38 PM, Johan Hedberg wrote:
> Hi Arek,
>
> On Thu, Apr 04, 2013, Arkadiusz Lichwa wrote:
>> valgrind's output of exemplary call: sdptool browse local
>>
>> ==2203== HEAP SUMMARY:
>> ==2203== in use at exit: 0 bytes in 0 blocks
>> ==2203== total heap usage: 251 allocs, 251 frees, 140,156 bytes allocated
>> ==2203==
>> ==2203== All heap blocks were freed -- no leaks are possible
>> ==2203==
>> ==2203== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 0 from 0)
>> ==2203==
>> ==2203== 1 errors in context 1 of 2:
>> ==2203== Invalid write of size 2
>> ==2203== at 0x805B882: bt_put_be16 (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8062BD0: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== Address 0x4391359 is 7 bytes before a block of size 2,048 alloc'd
>> ==2203== at 0x402B6A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
>> ==2203== by 0x8062B4B: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203==
>> ==2203==
>> ==2203== 5 errors in context 2 of 2:
>> ==2203== Invalid write of size 1
>> ==2203== at 0x402D363: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
>> ==2203== by 0x80613E7: gen_dataseq_pdu (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8061472: gen_attridseq_pdu (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8062C00: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== Address 0x439135b is 5 bytes before a block of size 2,048 alloc'd
>> ==2203== at 0x402B6A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
>> ==2203== by 0x8062B4B: sdp_service_search_attr_req (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8052457: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x80525AE: do_search (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x805277F: cmd_browse (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203== by 0x8053199: main (in /home/xpu/gits/bluez.bin/bin/sdptool)
>> ==2203==
>> ==2203== ERROR SUMMARY: 6 errors from 2 contexts (suppressed: 0 from 0)
>> ---
>> lib/sdp.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/lib/sdp.c b/lib/sdp.c
>> index 6c73818..cc685b4 100644
>> --- a/lib/sdp.c
>> +++ b/lib/sdp.c
>> @@ -4429,6 +4429,11 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
>>
>> /* add service class IDs for search */
>> seqlen = gen_searchseq_pdu(pdata, search);
>> + if (seqlen < 0) {
>> + errno = EINVAL;
>> + status = -1;
>> + goto end;
>> + }
>>
>> SDPDBG("Data seq added : %d\n", seqlen);
> The patch looks correct and I've applied it, but could you explain how
> exactly you reproduced the issue? I didn't get any valgrind warnings
> when trying your suggested "sdptool browse local".
>
> Johan
Just run latest bluetoothd with -dnC
Then start quering local sdp server using latest sdptool.
The problem is when there's a registered service PnP and contains sdp
attribute ID number 0x200, PnP version number.
Probably any other service that holds such attribute ID will trigger
that as well.
The issue is likely related to latest commit from David Herrmann that
maybe unveils some "misfunctionality" of sdp lib subsystem.
The wrong calculated offsets are related to internal sdp subcalls that
"gen_searchseq_pdu" call,
and that they returns sometimes -ENOMEM (12) as result.
That value can cause movement of "current pointer" of allocated
"sdprequest buffer" to invalid position before following writings to it.
/Arek
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-04 14:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-04 14:09 [PATCH] lib: Browsing services using sdptool can lead to writing to invalid heap locations Arkadiusz Lichwa
2013-04-04 12:38 ` Johan Hedberg
2013-04-04 13:31 ` Arkadiusz Lichwa
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).