[pulseaudio-discuss] [PATCH 00/13] Fix reading of freed memory
Tanu Kaskinen
tanuk at iki.fi
Tue Feb 12 11:36:50 PST 2013
Luiz reported today a bug revealed by Valgrind:
==16165== Invalid read of size 1
==16165== at 0x5170C2E: pa_idxset_string_hash_func (idxset.c:67)
==16165== by 0x516FC77: remove_entry (hashmap.c:93)
==16165== by 0x516FE0B: pa_hashmap_free (hashmap.c:110)
==16165== by 0x4C77B75: device_port_free (device-port.c:60)
==16165== by 0x4C43C7F: pa_object_unref (object.c:64)
==16165== by 0x4C7785E: pa_device_port_unref (device-port.h:61)
==16165== by 0x4C77D9C: pa_device_port_hashmap_free (device-port.c:96)
==16165== by 0x4C860BE: source_free (source.c:680)
==16165== by 0x4C43C7F: pa_object_unref (object.c:64)
==16165== by 0x155AD610: pa_source_unref (source.h:237)
==16165== by 0x155B1965: pa_dbusiface_stream_free (iface-stream.c:917)
==16165== by 0x1559CE89: subscription_cb (iface-core.c:1746)
==16165== Address 0x10dc1ec0 is 0 bytes inside a block of size 12 free'd
==16165== at 0x4A077A6: free (vg_replace_malloc.c:446)
==16165== by 0x4F31E3D: pa_xfree (xmalloc.c:131)
==16165== by 0x4C376A8: pa_card_profile_free (card.c:62)
==16165== by 0x4C38788: pa_card_free (card.c:254)
==16165== by 0x159D6E6C: module_bluetooth_device_LTX_pa__done (module-bluetooth-device.c:2627)
==16165== by 0x4C41C02: pa_module_free (module.c:162)
==16165== by 0x4C41DE8: pa_module_unload (module.c:188)
==16165== by 0x159D6391: discovery_hook_cb (module-bluetooth-device.c:2439)
==16165== by 0x4C3EF55: pa_hook_fire (hook-list.c:106)
==16165== by 0x13528C21: run_callback (bluetooth-util.c:668)
==16165== by 0x1352E504: endpoint_clear_configuration (bluetooth-util.c:1859)
==16165== by 0x1352F23B: endpoint_handler (bluetooth-util.c:2077)
The keys of the profiles hashmap of pa_device_port were getting freed
too early. The reason was that module-dbus-protocol held a reference
to a bluetooth source, and that source in turn held a reference to a
port. When the associated card was freed, the port was not. When the
port then eventually was freed, it was referencing profile memory that
was already freed.
I started by modifying pa_card_free(), and I found myself wanting
a pa_hashmap_remove_all() function, so I started writing that. I
wanted the free_cb of that function to match the free_cb type of
pa_hashmap_free(), but pa_hashmap_free() used pa_free2_cb_t, which was
stupid, so I changed that first. Then I made the same changes to
idxset. Patches 1-7 implement this refactoring.
Patches 8-13 then fix the actual problem by removing refcounting from
ports and making sure that there won't be any other reference holders
when the sink or source implementor calls pa_sink/source_unref(). This
ensures that the ports and profiles are freed in the expected order
when a card is removed.
FWIW, three of the patches are from an older patch set that I haven't
finished (the intention has been to completely fix
https://bugs.freedesktop.org/show_bug.cgi?id=45817 before submitting
the patch set, but I haven't worked on that lately).
Tanu Kaskinen (13):
gconf: Add userdata pointer to struct module_info
gconf: Remove needless userdata function arguments
hashmap: Use pa_free_cb_t instead of pa_free2_cb_t
device-port: Remove pa_device_port_hashmap_free()
idxset: Use pa_free_cb_t instead of pa_free2_cb_t
hashmap: Add pa_hashmap_remove_all()
idxset: Add pa_idxset_remove_all()
core: Add hooks for default device changes
dbus: Track the default sink and source with hooks
dbus: Track streams with hooks
dbus: Track stream moves with hooks
bluetooth: Fix thread teardown code ordering
Remove refcounting from ports, sinks and sources
src/modules/alsa/alsa-mixer.c | 68 +---
src/modules/alsa/alsa-sink.c | 4 +-
src/modules/alsa/alsa-ucm.c | 12 +-
src/modules/alsa/module-alsa-card.c | 18 +-
src/modules/bluetooth/bluetooth-util.c | 4 +-
src/modules/bluetooth/module-bluetooth-device.c | 22 +-
src/modules/bluetooth/module-bluetooth-discover.c | 2 +-
src/modules/bluetooth/module-bluetooth-proximity.c | 10 +-
src/modules/dbus/iface-card.c | 10 +-
src/modules/dbus/iface-core.c | 416 +++++++++-----------
src/modules/dbus/iface-device.c | 23 +-
src/modules/dbus/iface-stream.c | 96 ++---
src/modules/dbus/module-dbus-protocol.c | 9 +-
src/modules/gconf/module-gconf.c | 37 +-
src/modules/macosx/module-bonjour-publish.c | 2 +-
src/modules/module-augment-properties.c | 10 +-
src/modules/module-card-restore.c | 4 +-
src/modules/module-combine-sink.c | 11 +-
src/modules/module-console-kit.c | 9 +-
src/modules/module-device-manager.c | 10 +-
src/modules/module-device-restore.c | 6 +-
src/modules/module-filter-apply.c | 2 +-
src/modules/module-role-cork.c | 18 +-
src/modules/module-role-ducking.c | 19 +-
src/modules/module-stream-restore.c | 14 +-
src/modules/module-suspend-on-idle.c | 15 +-
src/modules/module-systemd-login.c | 14 +-
src/modules/module-udev-detect.c | 10 +-
src/modules/module-zeroconf-discover.c | 2 +-
src/modules/module-zeroconf-publish.c | 18 +-
src/modules/raop/module-raop-discover.c | 2 +-
src/modules/rtp/headerlist.c | 7 +-
src/modules/rtp/module-rtp-recv.c | 17 +-
src/pulse/context.c | 4 +-
src/pulse/format.c | 4 -
src/pulse/internal.h | 2 -
src/pulse/proplist.c | 7 +-
src/pulsecore/card.c | 38 +-
src/pulsecore/client.c | 4 +-
src/pulsecore/core-scache.c | 5 +-
src/pulsecore/core.c | 20 +-
src/pulsecore/core.h | 2 +
src/pulsecore/database-simple.c | 7 +-
src/pulsecore/device-port.c | 30 +-
src/pulsecore/device-port.h | 7 +-
src/pulsecore/hashmap.c | 25 +-
src/pulsecore/hashmap.h | 7 +-
src/pulsecore/idxset.c | 25 +-
src/pulsecore/idxset.h | 10 +-
src/pulsecore/memblock.c | 4 +-
src/pulsecore/modargs.c | 6 +-
src/pulsecore/module.c | 4 +-
src/pulsecore/mutex-win32.c | 2 +-
src/pulsecore/namereg.c | 2 +
src/pulsecore/protocol-cli.c | 2 +-
src/pulsecore/protocol-dbus.c | 66 +---
src/pulsecore/protocol-esound.c | 2 +-
src/pulsecore/protocol-http.c | 2 +-
src/pulsecore/protocol-native.c | 18 +-
src/pulsecore/protocol-simple.c | 2 +-
src/pulsecore/sink-input.c | 27 +-
src/pulsecore/sink.c | 33 +-
src/pulsecore/source-output.c | 10 +-
src/pulsecore/source.c | 17 +-
64 files changed, 521 insertions(+), 794 deletions(-)
--
1.7.10.4
More information about the pulseaudio-discuss
mailing list