As Coverity points out, `ensure_size()` is not fully correct.
Let us assume that the message already has some allocated storage,
and the `realloc()` call in the next `ensure_size()` invocation fails.
In that case `message::data` is freed, but the pointer is left behind.
If another `ensure_size()` call is made, then `realloc()` will be called
(since the previous call left `message::allocated` as zero), but the
first argument of the `realloc()` call will be a dangling pointer.
In order to avoid the above, first of all, clear `message::data` after
a failed `realloc()` call, and immediately return `-ENOMEM` if
`message::length` is greater than `message::allocated` since
that signals if the message has even run into an out-of-memory
situation.
Store a pointer to the owner `impl` object instead of
the (embedded) `stat` object. This way `message_free()`
can be simplified since the owner `impl` does not need
to be passed explicitly.
The `impl::servers` list contains all servers, and they are all
destroyed in `impl_clear()`. However, by that time, modules were
not freed previously, so if there were any instances of
*module-protocol-native-tcp* loaded, then the unload() method
of those would call `server_free()` on already freed servers,
resulting in use-after-frees. Fix that by unloading modules
before destroying the servers.
==451490==ERROR: AddressSanitizer: heap-use-after-free on address 0x612000006050 ...
READ of size 8 at 0x612000006050 thread T0
#0 0x7f45edb19a0c in server_free ../src/modules/module-protocol-pulse/server.c:1022
#1 0x7f45edb46c7d in module_native_protocol_tcp_unload ../src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c:66
#2 0x7f45eda893c7 in module_unload ../src/modules/module-protocol-pulse/module.c:128
#3 0x7f45edaf7269 in impl_unload_module ../src/modules/module-protocol-pulse/pulse-server.c:5336
#4 0x7f45edaa1583 in pw_map_for_each ../src/pipewire/map.h:238
#5 0x7f45edaf79c5 in impl_clear ../src/modules/module-protocol-pulse/pulse-server.c:5358
...
0x612000006050 is located 16 bytes inside of 264-byte region [0x612000006040,0x612000006148)
freed by thread T0 here:
#0 0x7f45f30be672 in __interceptor_free /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x7f45edb1a48a in server_free ../src/modules/module-protocol-pulse/server.c:1040
#2 0x7f45edaf730b in impl_clear ../src/modules/module-protocol-pulse/pulse-server.c:5347
...
Fixes: f181210b29 ("pulse-server: properly unload modules")
As Coverity correctly points out, the `if (blocklist)` condition
is never true after the `out` label, so this commit makes some
changes to remove the dead code.
First of all, the `regex_t` object is directly embedded in the
module's data struct, so the `malloc()` call can be removed,
and thus there is no need for the cleanup code anymore, so everything
after the `out` label is also removed.
Furthermore, two NULL checks are removed which check `d->blocklist`
from `module_switch_on_connect_unload()` and `manager_added()`
because both of those functions can only ever run if the `d->blocklist`
regex object has been successfully initialized in `module_switch_on_connect_prepare()`.
Those checks were not strictly needed to begin with.
It is already logged by the module loading code when a module is
successfully loaded, so there is no need to print it on a per-module
basis at the same log level.
Check all pending streams to see if the new link completes their
creation.
It is possible that the link linked 2 pending streams and then we
want to complete them both.
All modules need to manually create a `module` object and check
if it was successfully created. The same with argument parsing.
To simplify modules, move the module object creation and argument
parsing into `module_create()`, and pass the already initialized
module to `module_info::create()`.
The semantics of `module_info::create()` are kept, that is,
if it fails, `module_info::unload()` will not be called.
Now that the module's properties are served from `module_info::properties`,
there is no need for modules to put their static properties into
the `module::props` dictionary.
Make sure to never send less than the negotiated fragsize to a client.
Also make sure we don't send too much data in one go. This is more in
line with what pulseaudio does.
Fixes capture from multiple tabs in chrome.
Fixes#2418
Previously, `module_x11_bell_data::module` was not initialized
properly, which led to a NULL pointer dereference in
`module_schedule_unload()` when called from `module_destroy()`
in case the native pipewire module was unloaded.
See #2392
When we move a message to the free list, reset the length to 0.
Otherwise the previous length plus the new length will be used to
allocated the message size, which would overallocate.
Make MOVE_SINK_INPUT/MOVE_SOURCE_OUTPUT change the linked peer
immediately in subsequent GET_SINK_INPUT_INFO/GET_SOURCE_OUTPUT_INFO
commands. Do this by keeping track of the sink/source where the client
moved the stream to, and temporarily replying so in future GET_INFO (but
only in messages for that client).
We discard the temporary override when we either get an update event for
the stream (i.e. SM moved the stream), or a 1sec timer runs out. If the
timer runs out, we emit a sink-input/source-output change event, as in
that case what we claimed in the earlier GET_INFO messages might not be
true, so clients need to update their information.
This gets rid of race conditions where an application moves a stream,
and expects the move to be visible in future GET_INFO replies, which may
fail to happen because it takes some time for the session manager to
re-link the streams.
Fixes pasystray behavior.
Add functions for setting timer-based lifetimes for object data.
Having the timers in the object data themselves simplifies life cycle
management, as client/manager disconnects are handled without further
work.
Add the auth-anonymous option to module-native-protocol-tcp.
It does not exactly do what pulseaudio does but it will enable
unrestricted access instead.
Fixes#1899
This prevents the ringbuffer from advancing the read pointer more than
the size of data actually available. It prevents the "avail" value from
keeping to drop when no audio is being played by the client.
Applying this patch seems to prevent the "randomly playing music after
a couple hours of silence" issue
Fixes#2366
Guess the expected latency with the stream info we have and use that as
the node.latency. This way, the graph can attempt to start with some
sort of latency setting.
After we know the exact format, we can calculate the real latency and we
will update the node latency accordingly.
So that all nodes attach to some driver to be scheduled.
For the virtual sink/sources this is a normal thing and we can remove
the custom settings.
For normal stream, this now makes it possible to link pw-play directly
to pw-record and have it transport data.
Fixes#1761
Move all module methods into the `module_info` struct, and place
all such structs into the "pw_mod_pulse_modules" section of
the executable. This way there is no need for an explicit
module registry, and all information about a module can
be declared in the module's source file in a single place.
For the modules that require a driver, don't add ourselves to
the pipewire.dummy group but instead just use the NODE_WANT_DRIVER
property to be assigned to a driver.
This makes it possible for the nodes to move to another driver than the
dummy driver (which has very high priority) and it avoids resampling in
cases where the nodes are linked to an audio source or sink.
Deprecate pw_stream_get_time() in favour of _get_time_n() that contains
the size of the pw_time structure. Make the old one fill in the fields
up to the buffered field. Make the new one use the size to decide how
much info to fill in.
Add a new buffered field in pw_time that contains the buffered data
inside the converter/resampler. This leaves the queued field with
purely the user provided size in the buffers.
Use get_time_n() in places.
And use this in spa_json_format_float() where we also avoid invalid
json floats.
Use json float format in some places where we serialize json floats.
Add a unit test.
See #2223
