On 03.05.2017 21:58, Tanu Kaskinen wrote:
On Tue, 2017-05-02 at 07:12 +0200, Georg Chini wrote:
On 01.05.2017 22:10, Tanu Kaskinen wrote:
On Mon, 2017-04-24 at 19:33 +0200, Georg Chini wrote:
There are several places in module-echo-cancel where a segfault is
possible when the master sink or source is invalid.
I don't think the rewind, volume and mute callbacks are ever called
during stream moves, at least with the current code base. However,
adding the extra checks does no harm either, so I don't mind, but I
think the commit message should be clarified on this point.
I do not mention a move at all in the commit message, I just
say there are several places where a segfault is possible.
The patch adds sink_input->sink and source_output->source checks, and
checking those pointers is only relevant during stream moves (or during
stream initialization before the routing has been decided, but the
callbacks aren't used that early). At all other times the sink and
source pointers are non-NULL.
I think the rewind callback is called during a move, the
PA_SINK_MESSAGE_START_MOVE calls request_rewind().
That happens before detaching the sink input, so the sink pointer is
not NULL.
This seems not correct. At least after fixing the first bug, the next
crash occurred in pa_sink_invalidate_requested_latency() during
PA_SINK_MESSAGE_START_MOVE. The request_rewind is just a few
lines below.
There must be (at least for echo-cancel) a possible situation
where the sink-input is moved from (null) to a real sink, otherwise
the crash I fixed could not happen at all.
Also the other checks are not pointless. In a situation where
a virtual sink is the only remaining sink, you can still try to
mute it or change the volume which will then crash PA.
A virtual sink can't exist without a master sink. The only time when a
virtual sink is not connected to a master sink is when the virtual sink
is being moved to a different master, and client commands (such as
volume and mute change commands) are not processed during moves.
Yes, I tested it, the modules get unloaded when no sinks are
available. I could not find anything on the first glance that did
it, so I assumed they would just "hang around".
Do you think I should remove the checks from the patch?
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
