[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 09/17] bt: stop the sdp memory allocation craziness
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PULL 09/17] bt: stop the sdp memory allocation craziness |
Date: |
Tue, 1 Aug 2017 18:17:17 +0200 |
Clang static analyzer reports a memory leak. Actually, the allocated
memory escapes here:
record->attribute_list[record->attributes].pair = data;
but clang is correct that the memory might leak if len is zero. We
know it isn't; assert that it is the case.
The craziness doesn't end there. The memory is freed by
bt_l2cap_sdp_close_ch:
g_free(sdp->service_list[i].attribute_list->pair);
which actually should have been written like this:
g_free(sdp->service_list[i].attribute_list[0].pair);
The attribute_list is sorted with qsort; but indeed the first
entry of attribute_list should point to "data" even after the qsort,
because the first record has id SDP_ATTR_RECORD_HANDLE, whose
numeric value is zero.
But hang on. The qsort function is
static int sdp_attributeid_compare(
const struct sdp_service_attribute_s *a,
const struct sdp_service_attribute_s *b)
{
return (int) b->attribute_id - a->attribute_id;
}
but no one ever writes attribute_id. So it only works if qsort is
stable, and who knows what else is broken, but we can fix it by
setting attribute_id in the while loop.
Signed-off-by: Paolo Bonzini <address@hidden>
---
hw/bt/sdp.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/bt/sdp.c b/hw/bt/sdp.c
index f67b3b8..3cb60b9 100644
--- a/hw/bt/sdp.c
+++ b/hw/bt/sdp.c
@@ -580,7 +580,7 @@ static void bt_l2cap_sdp_close_ch(void *opaque)
int i;
for (i = 0; i < sdp->services; i ++) {
- g_free(sdp->service_list[i].attribute_list->pair);
+ g_free(sdp->service_list[i].attribute_list[0].pair);
g_free(sdp->service_list[i].attribute_list);
g_free(sdp->service_list[i].uuid);
}
@@ -720,6 +720,8 @@ static void sdp_service_record_build(struct
sdp_service_record_s *record,
len += sdp_attr_max_size(&def->attributes[record->attributes ++].data,
&record->uuids);
}
+
+ assert(len > 0);
record->uuids = pow2ceil(record->uuids);
record->attribute_list =
g_malloc0(record->attributes * sizeof(*record->attribute_list));
@@ -730,12 +732,14 @@ static void sdp_service_record_build(struct
sdp_service_record_s *record,
record->attributes = 0;
uuid = record->uuid;
while (def->attributes[record->attributes].data.type) {
+ int attribute_id = def->attributes[record->attributes].id;
record->attribute_list[record->attributes].pair = data;
+ record->attribute_list[record->attributes].attribute_id = attribute_id;
len = 0;
data[len ++] = SDP_DTYPE_UINT | SDP_DSIZE_2;
- data[len ++] = def->attributes[record->attributes].id >> 8;
- data[len ++] = def->attributes[record->attributes].id & 0xff;
+ data[len ++] = attribute_id >> 8;
+ data[len ++] = attribute_id & 0xff;
len += sdp_attr_write(data + len,
&def->attributes[record->attributes].data, &uuid);
@@ -749,10 +753,15 @@ static void sdp_service_record_build(struct
sdp_service_record_s *record,
data += len;
}
- /* Sort the attribute list by the AttributeID */
+ /* Sort the attribute list by the AttributeID. The first must be
+ * SDP_ATTR_RECORD_HANDLE so that bt_l2cap_sdp_close_ch can free
+ * the buffer.
+ */
qsort(record->attribute_list, record->attributes,
sizeof(*record->attribute_list),
(void *) sdp_attributeid_compare);
+ assert(record->attribute_list[0].pair == data);
+
/* Sort the searchable UUIDs list for bisection */
qsort(record->uuid, record->uuids,
sizeof(*record->uuid),
--
1.8.3.1
- Re: [Qemu-devel] [PULL 02/17] cpu_physical_memory_sync_dirty_bitmap: Fix alignment check, (continued)
[Qemu-devel] [PULL 01/17] vl.c/exit: pause cpus before closing block devices, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 05/17] char: don't exit on hmp 'chardev-add help', Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 04/17] char-fd: remove useless chr pointer, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 03/17] accel: cleanup error output, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 06/17] docs: document deprecation policy & deprecated features in appendix, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 08/17] exec: Add lock parameter to qemu_ram_ptr_length, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 09/17] bt: stop the sdp memory allocation craziness,
Paolo Bonzini <=
[Qemu-devel] [PULL 10/17] qemu-options: document existance of versioned machine types, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 11/17] migration: optimize the downtime, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 12/17] hw/scsi/vmw_pvscsi: Remove the dead error handling, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 13/17] hw/scsi/vmw_pvscsi: Convert to realize, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 14/17] rtc-test: cleanup register_b_set_flag test, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 15/17] rtc-test: introduce more update tests, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 16/17] mc146818rtc: simplify check_update_timer, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 17/17] mc146818rtc: implement UIP latching as intended, Paolo Bonzini, 2017/08/01
[Qemu-devel] [PULL 07/17] target-i386: kvm_get/put_vcpu_events don't handle sipi_vector, Paolo Bonzini, 2017/08/01
Re: [Qemu-devel] [PULL 00/17] Misc changes for QEMU 2.10-rc1 (?), no-reply, 2017/08/01