[PATCH BlueZ v1 3/3] sdp: Fix memory leak when freeing alternates

Luiz Augusto von Dentz <[email protected]>
Newsgroups org.kernel.vger.linux-bluetooth
Message-ID <[email protected]>
From: Luiz Augusto von Dentz <[email protected]>

sdp_data_alloc_with_length() stores the members of SDP_ALT8, SDP_ALT16
and SDP_ALT32 in val.dataseq, exactly like it does for the SDP_SEQ8,
SDP_SEQ16 and SDP_SEQ32 sequences.

sdp_data_free() only calls data_seq_free() for the sequences though, so
freeing an alternate frees the alternate itself and leaks every one of
its members, along with anything they own in turn.

Free the members of alternates as well, and add a test parsing a record
containing an alternate, which leaks without this change:

209 (48 direct, 161 indirect) bytes in 1 blocks are definitely lost
   at calloc (vg_replace_malloc.c:1678)
   by sdp_data_alloc_with_length (sdp.c:350)
   by sdp_data_alloc (sdp.c:486)
   by sdp_xml_parse_int (sdp-xml.c:243)
   by sdp_xml_parse_datatype (sdp-xml.c:421)
   by element_start (sdp-xml.c:507)

Assisted-by: Claude:claude-opus-5
---
 Makefile.am                |  1 +
 lib/bluetooth/sdp.c        |  3 +++
 unit/sdp-xml/alternate.xml | 12 ++++++++++++
 unit/test-sdp-xml.c        |  1 +
 4 files changed, 17 insertions(+)
 create mode 100644 unit/sdp-xml/alternate.xml

diff --git a/Makefile.am b/Makefile.am
index 3c6cf92ab403..2754e1b7f2da 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -648,6 +648,7 @@ unit_test_sdp_xml_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS) -DTOP_SRCDIR=\""$(srcdir)
 unit_test_sdp_xml_CPPFLAGS = -I$(srcdir)/lib
 
 EXTRA_DIST += unit/sdp-xml/Bluetooth_HID-sdp_record.xml		\
+	unit/sdp-xml/alternate.xml				\
 	unit/sdp-xml/compute-seq-size-type-confusion.xml	\
 	unit/sdp-xml/duplicate-attribute.xml			\
 	unit/sdp-xml/qt-SerialPortSDPRecord.xml
diff --git a/lib/bluetooth/sdp.c b/lib/bluetooth/sdp.c
index 8c0865398519..1e027f9ebe6d 100644
--- a/lib/bluetooth/sdp.c
+++ b/lib/bluetooth/sdp.c
@@ -972,6 +972,9 @@ void sdp_data_free(sdp_data_t *d)
 	case SDP_SEQ8:
 	case SDP_SEQ16:
 	case SDP_SEQ32:
+	case SDP_ALT8:
+	case SDP_ALT16:
+	case SDP_ALT32:
 		data_seq_free(d);
 		break;
 	case SDP_URL_STR8:
diff --git a/unit/sdp-xml/alternate.xml b/unit/sdp-xml/alternate.xml
new file mode 100644
index 000000000000..a35ebccbc71f
--- /dev/null
+++ b/unit/sdp-xml/alternate.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<record>
+	<attribute id="0x0004">
+		<alternate>
+			<uint32 value="0x11223344" />
+			<text value="alternate-member" />
+			<sequence>
+				<uint16 value="0x0100" />
+			</sequence>
+		</alternate>
+	</attribute>
+</record>
diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
index 72e01f5f5806..069b064b7ae8 100644
--- a/unit/test-sdp-xml.c
+++ b/unit/test-sdp-xml.c
@@ -127,6 +127,7 @@ static struct test_data file_tests[] = {
 	DEFINE_TEST("compute-seq-size-type-confusion.xml", FALSE),
 	/* From https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc */
 	DEFINE_TEST("duplicate-attribute.xml", TRUE),
+	DEFINE_TEST("alternate.xml", TRUE),
 };
 
 int main(int argc, char *argv[])
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.