Input Validation: High
The Apple MIDI command parser in module-rtp-session had two issues:
1. Insufficient minimum length check: the caller validated len >= 12,
but struct rtp_apple_midi is 16 bytes (cmd + protocol + initiator +
ssrc). Accessing hdr->ssrc on a 12-byte packet reads 4 bytes past
the received data.
2. Missing null-termination check: the name field (flexible array
member) from the network packet was passed directly to pw_log_info
with %s format and to find_session_by_addr_name for string
comparison, without verifying it contains a null terminator within
the received data. This could read past the receive buffer into
uninitialized stack memory, potentially leaking data into logs.
Fix by adding a sizeof check in parse_apple_midi_cmd and by validating
that the name is null-terminated within the received data in
parse_apple_midi_cmd_in.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of using timerfd, use the context timer-queue to schedule
timeouts. This saves fds and removes some redundant code.
Make the rtp-source timeout and standby code a bit better by using
atomic operations.
The state_changed callbacks fulfill multiple roles, which is both a problem
regarding separation of concerns and regarding code clarity. De facto,
these callbacks cover error reporting, opening connections, and closing
connection, all in one, depending on a state that is arguably an internal
stream detail. The code in these callbacks tie these internal states to
assumptions that opening/closing callbacks is directly tied to specific
state changes in a common way, which is not always true. For example,
stopping the stream may not _actually_ stop it if a background send timer
is still running.
The notion of a "state_changed" callback is also problematic because the
pw_streams that are used in rtp-sink and rtp-source also have a callback
for state changes, causing confusion.
Solve this by replacing state_changed with three new callbacks:
1. report_error : Used for reporting nonrecoverable errors to the caller.
Note that currently, no one does such error reporting, but the feature
does exist, so this callback is introduced to preserve said feature.
2. open_connection : Used for opening a connection. Its optional return
value informs about success or failure.
3. close_connection : Used for opening a connection. Its optional return
value informs about success or failure.
Importantly, these callbacks do not export any internal stream state. This
improves encapsulation, and also makes it possible to invoke these
callbacks in situations that may not neatly map to a state change. One
example could be to close the connection as part of a stream_start call
to close any connection(s) left over from a previous run. (Followup commits
will in fact introduce such measures.)
When we simply need to change some state for the code executed in the
loop, we can use locked() instead of invoke(). This is more efficient
and avoids some context switches in the normal case.
This makes it possible to discover a local RAOP, pulse or RTP services
and connect to them.
IPv6 addresses need the interface appended to local addresses to
make the connection work.
Expose the acquire_loop/release_loop functions and use them in the
modules.
Make sure the nodes created from the module use the same data loop as
the module. We need to ensure this because otherwise, the nodes might
be scheduled on different data loops and the invoke or timer logic will
fail.
Use `getaddrinfo` in `parse_address` instead of `inet_pton`.
Display Ipv6 addresses with scope identifiers correctly in `get_ip`
functions using `if_indextoname`.
