[pulseaudio-discuss] [PATCH] sink-input, source-output: Don't crash if format negotiation fails

[Tanu Kaskinen]

A segfault was reported on this line:

pa_return_val_if_fail(PA_SINK_IS_LINKED(pa_sink_get_state(data->sink)), -PA_ERR_BADSTATE);

After expanding the pa_sink_get_state() macro, the line looks like
this:

pa_return_val_if_fail(PA_SINK_IS_LINKED(data->sink->state), -PA_ERR_BADSTATE);

So data->sink was apparently NULL. That could happen if we try to fall
back to the default sink, but format negotiation fails.

This bug was introduced in commit
71816ecb7f81fe7a124cefa2258cfa64046908a1.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=74646
---
 src/pulsecore/sink-input.c    | 14 +++++++-------
 src/pulsecore/source-output.c | 10 +++++-----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 97a55a6..9ae95e2 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -334,13 +334,6 @@ int pa_sink_input_new(
         pa_sink_input_new_data_set_sink(data, sink, false);
     }
 
-    pa_return_val_if_fail(PA_SINK_IS_LINKED(pa_sink_get_state(data->sink)), -PA_ERR_BADSTATE);
-    pa_return_val_if_fail(!data->sync_base || (data->sync_base->sink == data->sink
-                                               && pa_sink_input_get_state(data->sync_base) == PA_SINK_INPUT_CORKED),
-                          -PA_ERR_INVALID);
-
-    /* Routing's done, we have a sink. Now let's fix the format. */
-
     /* If something didn't pick a format for us, pick the top-most format since
      * we assume this is sorted in priority order */
     if (!data->format && data->nego_formats && !pa_idxset_isempty(data->nego_formats))
@@ -359,6 +352,13 @@ int pa_sink_input_new(
         return -PA_ERR_NOTSUPPORTED;
     }
 
+    pa_return_val_if_fail(PA_SINK_IS_LINKED(pa_sink_get_state(data->sink)), -PA_ERR_BADSTATE);
+    pa_return_val_if_fail(!data->sync_base || (data->sync_base->sink == data->sink
+                                               && pa_sink_input_get_state(data->sync_base) == PA_SINK_INPUT_CORKED),
+                          -PA_ERR_INVALID);
+
+    /* Routing is done. We have a sink and a format. */
+
     if (data->volume_is_set && pa_format_info_is_pcm(data->format)) {
         /* If volume is set, we need to save the original data->channel_map,
          * so that we can remap the volume from the original channel map to the
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 7b08b7e..67331a7 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -276,11 +276,6 @@ int pa_source_output_new(
         pa_source_output_new_data_set_source(data, source, false);
     }
 
-    pa_return_val_if_fail(PA_SOURCE_IS_LINKED(pa_source_get_state(data->source)), -PA_ERR_BADSTATE);
-    pa_return_val_if_fail(!data->direct_on_input || data->direct_on_input->sink == data->source->monitor_of, -PA_ERR_INVALID);
-
-    /* Routing's done, we have a source. Now let's fix the format. */
-
     /* If something didn't pick a format for us, pick the top-most format since
      * we assume this is sorted in priority order */
     if (!data->format && data->nego_formats && !pa_idxset_isempty(data->nego_formats))
@@ -299,6 +294,11 @@ int pa_source_output_new(
         return -PA_ERR_NOTSUPPORTED;
     }
 
+    pa_return_val_if_fail(PA_SOURCE_IS_LINKED(pa_source_get_state(data->source)), -PA_ERR_BADSTATE);
+    pa_return_val_if_fail(!data->direct_on_input || data->direct_on_input->sink == data->source->monitor_of, -PA_ERR_INVALID);
+
+    /* Routing is done. We have a source and a format. */
+
     if (data->volume_is_set && pa_format_info_is_pcm(data->format)) {
         /* If volume is set, we need to save the original data->channel_map,
          * so that we can remap the volume from the original channel map to the
-- 
1.8.3.1