Fix issue 2516842 Device runtime restarts while pairing and connecting A2DP HS.
There are several problems in liba2dp.c:
1 There is a bug in bluetooth_parse_capabilities(): the capabilities field pointer increment
in the while loop is wrong and in case the first capabilities field is not the one we need
we end up reading undefined data.
2 In the same while loop there is no protection for infinite looping if the length of the
capabilities field is 0.
3 When start or configure fail, we just reset state to A2DP_STATE_INITIALIZED to force a new
configuration attempt. This is bad as we don't close the connection and configure expects a
closed connection. Subsequent configure attempts will fail reading sbc capabilites.
4 When there is a problem executing a command in a2dp_thread() loop, we do not reset current command
which prevents from executing the same command again.
Here is what happens in this issue:
There is an error in the A2DP start request because the headset has been disconnected and we go back to configure.
Because of problem 3, the connection is still opened when we run configure again and the 1st capabilitites
field lock indicator is set. We try to read next field and because of problems 1 & 2 we end up looping here for ever.
As data->mutex is locked, it cannot be acquired by pthread_cond_timedwait() trying to exit in
wait_for_start() and the timeout mechanism fails. We stay locked here with A2dpAudioInterface::A2dpAudioStreamOut::mLock also
locked as we are in A2dpAudioInterface::A2dpAudioStreamOut:write().
When system_server tries to disable bluetooth A2DP, A2dpAudioInterface::A2dpAudioStreamOut::setBluetoothEnabled()
tries to acquire mLock and system_server is also deadlocked.
Change-Id: I785250fe65651ec6fc2ae01a4250a61f2fd43908
diff --git a/audio/liba2dp.c b/audio/liba2dp.c
index 9dfcf8b..1e35cf6 100755
--- a/audio/liba2dp.c
+++ b/audio/liba2dp.c
@@ -96,7 +96,7 @@
#define WRITE_TIMEOUT 1000
/* timeout in seconds for command socket recv() */
-#define RECV_TIMEOUT 5
+#define RECV_TIMEOUT 5
typedef enum {
@@ -271,9 +271,9 @@
return 0;
error:
- /* set state to A2DP_STATE_INITIALIZED to force reconfiguration */
+ /* close bluetooth connection to force reinit and reconfiguration */
if (data->state == A2DP_STATE_STARTING)
- set_state(data, A2DP_STATE_INITIALIZED);
+ bluetooth_close(data);
return err;
}
@@ -734,7 +734,7 @@
length = msg->length ? msg->length : BT_SUGGESTED_BUFFER_SIZE;
- VDBG("sending %s", bt_audio_strmsg(msg->msg_type));
+ VDBG("sending %s", bt_audio_strtype(msg->type));
if (send(data->server.fd, msg, length,
MSG_NOSIGNAL) > 0)
err = 0;
@@ -823,17 +823,21 @@
static int bluetooth_init(struct bluetooth_data *data)
{
int sk, err;
- struct timeval tv = {.tv_sec = RECV_TIMEOUT};
+ struct timeval tv = {.tv_sec = RECV_TIMEOUT};
DBG("bluetooth_init");
sk = bt_audio_service_open();
- if (sk <= 0) {
+ if (sk < 0) {
ERR("bt_audio_service_open failed\n");
return -errno;
}
- setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+ err = setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+ if (err < 0) {
+ ERR("bluetooth_init setsockopt(SO_RCVTIMEO) failed %d", err);
+ return err;
+ }
data->server.fd = sk;
data->server.events = POLLIN;
@@ -849,15 +853,19 @@
codec_capabilities_t *codec = (void *) rsp->data;
if (codec->transport != BT_CAPABILITIES_TRANSPORT_A2DP)
- return 0;
+ return -EINVAL;
while (bytes_left > 0) {
if ((codec->type == BT_A2DP_SBC_SINK) &&
- !(codec->lock & BT_WRITE_LOCK))
+ !(codec->lock & BT_WRITE_LOCK))
break;
+ if (codec->length == 0) {
+ ERR("bluetooth_parse_capabilities() invalid codec capabilities length");
+ return -EINVAL;
+ }
bytes_left -= codec->length;
- codec = (void *) (codec + codec->length);
+ codec = (codec_capabilities_t *)((char *)codec + codec->length);
}
if (bytes_left <= 0 ||
@@ -902,19 +910,26 @@
goto error;
}
- bluetooth_parse_capabilities(data, getcaps_rsp);
+ err = bluetooth_parse_capabilities(data, getcaps_rsp);
+ if (err < 0) {
+ ERR("bluetooth_parse_capabilities failed err: %d", err);
+ goto error;
+ }
+
err = bluetooth_a2dp_hw_params(data);
if (err < 0) {
ERR("bluetooth_a2dp_hw_params failed err: %d", err);
goto error;
}
+
set_state(data, A2DP_STATE_CONFIGURED);
return 0;
error:
+
if (data->state == A2DP_STATE_CONFIGURING)
- set_state(data, A2DP_STATE_INITIALIZED);
+ bluetooth_close(data);
return err;
}
@@ -1013,6 +1028,7 @@
{
struct bluetooth_data* data = (struct bluetooth_data*)d;
a2dp_command_t command = A2DP_CMD_NONE;
+ int err = 0;
DBG("a2dp_thread started");
prctl(PR_SET_NAME, (int)"a2dp_thread", 0, 0, 0);
@@ -1029,8 +1045,8 @@
/* Initialization needed */
if (data->state == A2DP_STATE_NONE &&
- data->command != A2DP_CMD_QUIT) {
- bluetooth_init(data);
+ data->command != A2DP_CMD_QUIT) {
+ err = bluetooth_init(data);
}
/* New state command signaled */
@@ -1044,19 +1060,19 @@
case A2DP_CMD_CONFIGURE:
if (data->state != A2DP_STATE_INITIALIZED)
break;
- bluetooth_configure(data);
+ err = bluetooth_configure(data);
break;
case A2DP_CMD_START:
if (data->state != A2DP_STATE_CONFIGURED)
break;
- bluetooth_start(data);
+ err = bluetooth_start(data);
break;
case A2DP_CMD_STOP:
if (data->state != A2DP_STATE_STARTED)
break;
- bluetooth_stop(data);
+ err = bluetooth_stop(data);
break;
case A2DP_CMD_QUIT:
@@ -1070,6 +1086,11 @@
default:
break;
}
+ // reset last command in case of error to allow
+ // re-execution of the same command
+ if (err < 0) {
+ command = A2DP_CMD_NONE;
+ }
}
done:
