The AVDTP spec allows for a race condition between remote and local
device when issuing an AVDTP_START cmd on a stream in the OPEN state.
However, the internal state must continue to be consistent. For example,
suppose that avdtp_start() has been called while in the OPEN state and
a AVDTP_START cmd is sent. Now before we have received a response (and
thus entered the STREAMING state), we *receive* a START cmd. Prior to
this fix, since the sep is still in the OPEN state, we would accept
the new START cmd. This will leads us to send both a Start_Ind and
Start_Cfm - not good.
Now, we track this transitional state (starting == TRUE).
NB - 'starting' is only in a valid state while the sep is in the
OPEN state. 'starting' is reset when we return to the OPEN state.
When handling the discover response, if all stream end points are
in use, then we must finalize discovery, as no GET_CAPABILITIES
command will be issued.
Changing stream state from STREAMING to IDLE can be associated with side
effects under some circumstances (such as terminating bluetoothd during
music is streamed). In this case, after connection is lost, stream state
changes from STREAMING to IDLE - "avdtp_sep_set_state" is triggered which
invokes callback called "stream_state_changed" which internally invokes
"avdtp_sep_set_state" (state of stream doesn't change and stays as IDLE)
second time and then stream object is freed by "stream_free"
at the end of "avdtp_sep_set_state". After returning from callback,
first triggered "avdtp_sep_set_state" attempts to free stream object
again ("if (state == AVDTP_STATE_IDLE)" condition is still satisfied)
and it leads to invalid read/write/free issues (reported by valgrind)
in "stream free" body, since "stream" is "alias" pointer to stream object
which is already out of date (memory for stream object has been already
freed).
This patch prevents from this special case by freeing stream object only
when it is present on streams list and removing from this list when
stream object would be freed.
Changing stream state from STREAMING to IDLE can be associated with side
effects under some circumstances (such as terminating bluetoothd during
music is streamed). In this case, after connection is lost, stream state
changes from STREAMING to IDLE - "avdtp_sep_set_state" is triggered which
invokes callback called "stream_state_changed" which internally invokes
"avdtp_sep_set_state" (state of stream doesn't change and stays as IDLE)
second time and then stream callbacks list is discarded by "stream_free"
("g_slist_free(stream->callbacks)"). After returning from callback,
"stream->callbacks" list (and "l" pointer as well) is already out of date,
so attempting to fetch "l->next" pointer (returned by "g_slist_next(l)"
to be prepared to next iteration of "for" loop) from node on discarded
list leads to invalid read issue (reported by valgrind).
This patch prevents from this issue by moving "l = g_slist_next(l)"
instruction just before invoking callback - loop has been modified and
"while" used instead of "for" loop variant.
Fixed incorrect update of server->sessions GSlist in avdtp_exit.
Previosly it was leading to invalid read of memory (l = l->next)
(and possible memory leaks) since after invoking avdtp_unref in
connection_lost, l pointer was not valid anymore (previously assignment
l = l->next was used after invoking connection_lost in for loop).
Disconnecting L2CAP before getting a response for AVDTP start cause a
crash while we try to abort it:
avdtp_sep_set_state (session=0x9c210, sep=0x9ada0, state=AVDTP_STATE_IDLE)
0x000256dc in connection_lost (session=0x9c210, err=-5)
0x00025d44 in cancel_request (session=0x9c210, err=-5)
0x00026a98 in avdtp_abort (session=0x9c210, stream=0x9bee8)
0x00020e74 in a2dp_cancel (dev=<value optimized out>, id=<value optimized out>)
0x0002d5b4 in acquire_request_free (req=0x9b1a0)
0x0002d638 in media_owner_remove (owner=0x9a4a0)
0x0002da94 in media_transport_free (data=<value optimized out>)
0x000115a0 in remove_interface (data=0xa1f88, name=<value optimized out>)
0x0002e2a0 in media_transport_remove (transport=0x8fad8)
0x0002c624 in media_endpoint_clear_configuration (endpoint=0x9b098)
0x000260c8 in avdtp_sep_set_state (session=0x9c210, sep=0x9ada0, state=AVDTP_STATE_IDLE)
To fix this callbacks are called after handling the state change, so
any pending request are properly removed before state is set to idle.
At least one headset with both A2DP and PBAP sends delayed responses
to AVDTP requests when it is doing PBAP queries (typically at the
beginning of the connection).
Increasing the timer by two seconds to give the headset more time to
reply, to avoid aborting the AVDTP connection.
It makes sense to try to reuse the same SEP whenever possible when
reconfiguring streams. In fact this is even necessary with a particular
BMW car kit which doesn't allow a new stream to be set up to any other
SEP.
Functions get_send_buffer_size and set_send_buffer_size are added to
avdpt.c.
get_send_buffer_size returns size of send buffer for a given socket
on success or error code on failure. set_send_buffer_size sets size
of send buffer for a given socket, and returns 0 on success or error
code on failure.
Size of send buffer for L2CAP socket for SRC AVDTP stream is verified
during establishment of a new transport channel. If the size is less
than twice of outgoing L2CAP MTU, then it is considered as being
insufficient to handle streaming data reliably.
In this case buffer size is increased to be twice of MTU size. Such
fixes some IOP problems with car-kits that use large MTU for music
playback.
Since the connections can be cancelled while still not complete the
GIOChannel must be stored in order to be properly closed when freeing
avdtp session or stream and thus avoid any callbacks to be called after
that.
The spec clearly suggested that abort can be send while on idle state:
"AVDTP_ABORT_CMD can be sent or received in IDLE state. In the event that
an AVDTP_ABORT_CMD is received in IDLE state, ACP or INT shall reply with
an AVDTP_ABORT_RSP, no state change is required."
In fact abort is the only way to cancel set_configuration and open
commands.
Remove starting the timer when setting the AVDTP state to idle. If
needed, the timer should probably already have been started in
avdtp_unref when the reference count goes to one.
Since reference counting is handled in avdtp_ref and avdtp_unref, it
seems reasonable that not to inspect the count outside of those
functions.
The issue was found when using Device.Disconnect to disconnect a
headset. It was revealed by commit
c72ce0f12a.
Before the commit, the timer was removed and then started again.
After applying it, the idle callback (disconnect_timeout) is called
twice, causing a crash.
When the user has requested a disconnection it doesn't make sense to
have the AVDTP state machine waiting before closing the signalling
channel. This is particularly important for Device.Disconnect since only
two seconds are available for a clean disconnect before a forced
HCI_Disconnect command is sent.
In order to forward and create avdtp error properly for setconf command
we need to have the category, to fix this avdtp_error_t now carries the
category as define in avdtp and a new category is used for errno
(AVDTP_ERRNO).
This is similar to a sink attempt to configure a stream with a local
source, so in case of devices hiding their records for some reason with
this we can sill register a proper interface to handle the profile.
This should guarantee that any outstanding command in the queue is
cancelled, accourding to the avdtp spec:
"6.16 Abort
Because AVDTP is typically used on unreliable channels, signaling messages
can be lost due to L2CAP Flush Timeouts. To react to inconsistencies
between the INT and ACP states, the Abort Command may be used."
It also says abort can be send regardless of the state even in case of
packet fragmantation:
"Note that it is permitted to breakdown a packet sequence (command or
response message) to send an Abort command message in the same direction.
The remaining packets consecutive to an aborted message shall be flushed by
the sender."
Make it consistent with the logic when abort timeout since the original
intent is to cancel a pending request there is no point in keeping the
connection if it is in an unrecoverable state.
The pending request might be freed twice when receiving an Abort
response, in handle_unanswered_req and session_cb. Avoid freeing
it in handle_unanswered_req.
If the connection is lost when there is an AVDTP request the remote has
not replied to and which has not timed out, then at least in some cases
the callback for the request is not called leading to no Error response
being sent on the Audio API.
This patch checks if there is an outstanding request when the stream
state goes to idle and in that case triggers the corresponding callback
with an error.
Since request_timeout() does callbacks into a2dp.c before sending the
ABORT command we need to make sure that none of the callbacks can
trigger sending any further AVDTP commands. This is easiest done by
setting the stream->abort_int flag early in the request_timeout function
and then checking for it in the send_request function.
