I was able to trigger a crash with some unfinished code that caused
a protocol error. The error was handled while
pa_pstream_set_srbchannel() had not yet finished, so
pa_native_connection.srbpending and pa_pstream.srbpending were both
non-NULL. During the error handling, native_connection_unlink() freed
the srbchannel, and then pa_pstream_unlink() tried to free it for the
second time, causing an assertion error in mainloop_io_free().
---
src/pulsecore/protocol-native.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index 6ec65d6..012c41a 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -2639,13 +2639,25 @@ static void setup_srbchannel(pa_native_connection *c) {
static void command_enable_srbchannel(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata) {
pa_native_connection *c = PA_NATIVE_CONNECTION(userdata);
+ pa_srbchannel *tmp;
if (tag != (uint32_t) (size_t) c->srbpending)
protocol_error(c);
pa_log_debug("Client enabled srbchannel.");
- pa_pstream_set_srbchannel(c->pstream, c->srbpending);
+
+ /* We must set c->sbpending to NULL before calling
+ * pa_pstream_set_srbchannel(), because otherwise there is a time window
+ * when both pa_native_connection and pa_pstream think they own the
+ * srbchannel. pa_pstream_set_srbchannel() may read and dispatch arbitrary
+ * commands from the client, and if a protocol error occurs, the connection
+ * is torn down, and if c->sbpending has not yet been set to NULL here, the
+ * srbchannel will be double-freed. XXX: Can we somehow avoid reading and
+ * dispatching commands in pa_pstream_set_srbchannel()? Processing commands
+ * recursively seems prone to bugs. */
+ tmp = c->srbpending;
c->srbpending = NULL;
+ pa_pstream_set_srbchannel(c->pstream, c->srbpending);
}
static void command_auth(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata) {
--
1.9.3
