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.
Cleanup controls instead of inserting new elements with id 0 into the
controls list every time there is an unsupported or invalid PropInfo in
an info_changed node event.
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.
Since gst_fd_allocator_alloc lazy mmap's the buffer to the assigned
file descriptor we can get downstream mmap failures if the pipewire
src(such as the v4l2 spa plugin) closes the file descriptor before
it gets mmap'd. To prevent the closed original file descriptor from
causing a mmap failure dup the file descriptor so that the original
being closed doesn't invalidate the descriptor passed to
gst_fd_allocator_alloc.
Add some more validation to dequeue_buffer as well.
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
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
When in driver mode (mode=provide), the process() function is never
called. It needs to be triggered manually every now and then.
This fixes starting a mode=provide sink, but it doesn't fix re-starting
it... if the client disconnects while streaming, all buffers are getting
filled up and the pool blocks in aqcuiring one more; when the client
connects again, even if we signal the cond to unblock the pool, it still
can't acquire any more buffers and deadlocks.
Relates to: #1980
