From 48dbb4da3c166159cb1828ef9d2c7d76536c32f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sun, 27 Jun 2021 01:14:32 +0200 Subject: [PATCH] spa: bluez: native: do not use spa_aprintf() Instead of spa_aprintf(), convert `rfcomm_send_{cmd,query}` to take a printf-style format string. Furthermore, handle overflows and return errors from `rfcomm_send_{cmd,reply}`. And make those functions take an rfcomm as argument instead of any spa_source. And match conversion specifiers to the actual types in format strings. --- spa/plugins/bluez5/backend-native.c | 184 ++++++++++++++++------------ 1 file changed, 107 insertions(+), 77 deletions(-) diff --git a/spa/plugins/bluez5/backend-native.c b/spa/plugins/bluez5/backend-native.c index 5dac1be13..3e3bc2491 100644 --- a/spa/plugins/bluez5/backend-native.c +++ b/spa/plugins/bluez5/backend-native.c @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -245,36 +246,76 @@ static void rfcomm_free(struct rfcomm *rfcomm) free(rfcomm); } -static void rfcomm_send_cmd(struct spa_source *source, char *data) -{ - struct rfcomm *rfcomm = source->data; - struct impl *backend = rfcomm->backend; - char message[256]; - ssize_t len; +#define RFCOMM_MESSAGE_MAX_LENGTH 256 - spa_log_debug(backend->log, NAME": RFCOMM >> %s", data); - sprintf(message, "%s\n", data); - len = write(source->fd, message, strlen(message)); +SPA_PRINTF_FUNC(2, 3) +static ssize_t rfcomm_send_cmd(const struct rfcomm *rfcomm, const char *format, ...) +{ + struct impl *backend = rfcomm->backend; + char message[RFCOMM_MESSAGE_MAX_LENGTH + 1]; + ssize_t len; + va_list args; + + va_start(args, format); + len = vsnprintf(message, RFCOMM_MESSAGE_MAX_LENGTH + 1, format, args); + va_end(args); + + if (len < 0) + return -EINVAL; + + if (len > RFCOMM_MESSAGE_MAX_LENGTH) + return -E2BIG; + + spa_log_debug(backend->log, NAME": RFCOMM >> %s", message); + + message[len] = '\n'; + /* `message` is no longer null-terminated */ + + len = write(rfcomm->source.fd, message, len + 1); /* we ignore any errors, it's not critical and real errors should * be caught with the HANGUP and ERROR events handled above */ - if (len < 0) + if (len < 0) { + len = -errno; spa_log_error(backend->log, NAME": RFCOMM write error: %s", strerror(errno)); + } + + return len; } -static int rfcomm_send_reply(struct spa_source *source, const char *data) +SPA_PRINTF_FUNC(2, 3) +static ssize_t rfcomm_send_reply(const struct rfcomm *rfcomm, const char *format, ...) { - struct rfcomm *rfcomm = source->data; struct impl *backend = rfcomm->backend; - char message[256]; + char message[RFCOMM_MESSAGE_MAX_LENGTH + 4]; ssize_t len; + va_list args; - spa_log_debug(backend->log, NAME": RFCOMM >> %s", data); - sprintf(message, "\r\n%s\r\n", data); - len = write(source->fd, message, strlen(message)); + va_start(args, format); + len = vsnprintf(&message[2], RFCOMM_MESSAGE_MAX_LENGTH + 1, format, args); + va_end(args); + + if (len < 0) + return -EINVAL; + + if (len > RFCOMM_MESSAGE_MAX_LENGTH) + return -E2BIG; + + spa_log_debug(backend->log, NAME": RFCOMM >> %s", &message[2]); + + message[0] = '\r'; + message[1] = '\n'; + message[len + 2] = '\r'; + message[len + 3] = '\n'; + /* `message` is no longer null-terminated */ + + len = write(rfcomm->source.fd, message, len + 4); /* we ignore any errors, it's not critical and real errors should * be caught with the HANGUP and ERROR events handled above */ - if (len < 0) + if (len < 0) { + len = -errno; spa_log_error(backend->log, NAME": RFCOMM write error: %s", strerror(errno)); + } + return len; } @@ -325,22 +366,22 @@ static bool rfcomm_hsp_ag(struct spa_source *source, char* buf) if (sscanf(buf, "AT+VGS=%d", &gain) == 1) { if (gain <= SPA_BT_VOLUME_HS_MAX) { rfcomm_emit_volume_changed(rfcomm, SPA_BT_VOLUME_ID_TX, gain); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else { spa_log_debug(backend->log, NAME": RFCOMM receive unsupported VGS gain: %s", buf); - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); } } else if (sscanf(buf, "AT+VGM=%d", &gain) == 1) { if (gain <= SPA_BT_VOLUME_HS_MAX) { if (!rfcomm->broken_mic_hw_volume) rfcomm_emit_volume_changed(rfcomm, SPA_BT_VOLUME_ID_RX, gain); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else { - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); spa_log_debug(backend->log, NAME": RFCOMM receive unsupported VGM gain: %s", buf); } } else if (sscanf(buf, "AT+CKPD=%d", &dummy) == 1) { - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else { return false; } @@ -352,7 +393,7 @@ static bool rfcomm_send_volume_cmd(struct spa_source *source, int id) { struct rfcomm *rfcomm = source->data; struct spa_bt_transport_volume *t_volume; - char *cmd; + const char *format; int hw_volume; if (!rfcomm_volume_enabled(rfcomm)) @@ -367,14 +408,14 @@ static bool rfcomm_send_volume_cmd(struct spa_source *source, int id) rfcomm->volumes[id].hw_volume = hw_volume; if (id == SPA_BT_VOLUME_ID_TX) - cmd = spa_aprintf("AT+VGM=%u", hw_volume); + format = "AT+VGM=%d"; else if (id == SPA_BT_VOLUME_ID_RX) - cmd = spa_aprintf("AT+VGS=%u", hw_volume); + format = "AT+VGS=%d"; else spa_assert_not_reached(); - rfcomm_send_cmd(source, cmd); - free(cmd); + rfcomm_send_cmd(rfcomm, format, hw_volume); + return true; } @@ -616,7 +657,6 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) if (sscanf(buf, "AT+BRSF=%u", &features) == 1) { unsigned int ag_features = SPA_BT_HFP_AG_FEATURE_NONE; - char *cmd; /* * Determine device volume control. Some headsets only support control of @@ -652,10 +692,8 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) } /* send reply to HF with the features supported by Audio Gateway (=computer) */ - cmd = spa_aprintf("+BRSF: %d", ag_features); - rfcomm_send_reply(source, cmd); - free(cmd); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "+BRSF: %u", ag_features); + rfcomm_send_reply(rfcomm, "OK"); } else if (strncmp(buf, "AT+BAC=", 7) == 0) { /* retrieve supported codecs */ /* response has the form AT+BAC=,, @@ -681,22 +719,22 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) cntr++; } - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else if (strncmp(buf, "AT+CIND=?", 9) == 0) { - rfcomm_send_reply(source, "+CIND:(\"service\",(0-1)),(\"call\",(0-1)),(\"callsetup\",(0-3)),(\"callheld\",(0-2))"); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "+CIND:(\"service\",(0-1)),(\"call\",(0-1)),(\"callsetup\",(0-3)),(\"callheld\",(0-2))"); + rfcomm_send_reply(rfcomm, "OK"); } else if (strncmp(buf, "AT+CIND?", 8) == 0) { - rfcomm_send_reply(source, "+CIND: 0,0,0,0"); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "+CIND: 0,0,0,0"); + rfcomm_send_reply(rfcomm, "OK"); } else if (strncmp(buf, "AT+CMER", 7) == 0) { rfcomm->slc_configured = true; - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); /* switch codec to mSBC by sending unsolicited +BCS message */ if (rfcomm->codec_negotiation_supported && rfcomm->msbc_supported_by_hfp) { spa_log_debug(backend->log, NAME": RFCOMM initial codec setup"); rfcomm->hfp_ag_initial_codec_setup = HFP_AG_INITIAL_CODEC_SETUP_SEND; - rfcomm_send_reply(&rfcomm->source, "+BCS: 2"); + rfcomm_send_reply(rfcomm, "+BCS: 2"); codec_switch_start_timer(rfcomm, HFP_CODEC_SWITCH_INITIAL_TIMEOUT_MSEC); } else { rfcomm->transport = _transport_create(rfcomm); @@ -712,7 +750,7 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) } else if (!rfcomm->slc_configured) { spa_log_warn(backend->log, NAME": RFCOMM receive command before SLC completed: %s", buf); - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); return false; } else if (sscanf(buf, "AT+BCS=%u", &selected_codec) == 1) { /* parse BCS(=Bluetooth Codec Selection) reply */ @@ -723,7 +761,7 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) if (selected_codec != HFP_AUDIO_CODEC_CVSD && selected_codec != HFP_AUDIO_CODEC_MSBC) { spa_log_warn(backend->log, NAME": unsupported codec negotiation: %d", selected_codec); - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); if (was_switching_codec) spa_bt_device_emit_codec_switched(rfcomm->device, -EIO); return true; @@ -741,7 +779,7 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) if (rfcomm->transport == NULL) { spa_log_warn(backend->log, NAME": can't create transport: %m"); // TODO: We should manage the missing transport - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); if (was_switching_codec) spa_bt_device_emit_codec_switched(rfcomm->device, -ENOMEM); return true; @@ -750,30 +788,30 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) spa_bt_device_connect_profile(rfcomm->device, rfcomm->profile); rfcomm_emit_volume_changed(rfcomm, -1, SPA_BT_VOLUME_INVALID); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); if (was_switching_codec) spa_bt_device_emit_codec_switched(rfcomm->device, 0); } else if (sscanf(buf, "AT+VGM=%u", &gain) == 1) { if (gain <= SPA_BT_VOLUME_HS_MAX) { if (!rfcomm->broken_mic_hw_volume) rfcomm_emit_volume_changed(rfcomm, SPA_BT_VOLUME_ID_RX, gain); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else { spa_log_debug(backend->log, NAME": RFCOMM receive unsupported VGM gain: %s", buf); - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); } } else if (sscanf(buf, "AT+VGS=%u", &gain) == 1) { if (gain <= SPA_BT_VOLUME_HS_MAX) { rfcomm_emit_volume_changed(rfcomm, SPA_BT_VOLUME_ID_TX, gain); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else { spa_log_debug(backend->log, NAME": RFCOMM receive unsupported VGS gain: %s", buf); - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); } } else if (strncmp(buf, "AT+XAPL=", 8) == 0) { // We expect battery status only (bitmask 10) - rfcomm_send_reply(source, "+XAPL: iPhone,2"); - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "+XAPL: iPhone,2"); + rfcomm_send_reply(rfcomm, "OK"); } else if (sscanf(buf, "AT+IPHONEACCEV=%u", &len) == 1) { unsigned int i; int k, v; @@ -807,7 +845,7 @@ static bool rfcomm_hfp_ag(struct spa_source *source, char* buf) } } else if (strncmp(buf, "AT+APLSIRI?", 11) == 0) { // This command is sent when we activate Apple extensions - rfcomm_send_reply(source, "OK"); + rfcomm_send_reply(rfcomm, "OK"); } else { return false; } @@ -836,8 +874,6 @@ static bool rfcomm_hfp_hf(struct spa_source *source, char* buf) rfcomm->msbc_supported_by_hfp) rfcomm->codec_negotiation_supported = true; } else if (strncmp(token, "+BCS", 4) == 0 && rfcomm->codec_negotiation_supported) { - char *cmd; - /* get next token */ token = strtok(NULL, separators); selected_codec = atoi(token); @@ -848,9 +884,8 @@ static bool rfcomm_hfp_hf(struct spa_source *source, char* buf) spa_log_debug(backend->log, NAME": RFCOMM selected_codec = %i", selected_codec); /* send codec selection to AG */ - cmd = spa_aprintf("AT+BCS=%u", selected_codec); - rfcomm_send_cmd(source, cmd); - free(cmd); + rfcomm_send_cmd(rfcomm, "AT+BCS=%u", selected_codec); + rfcomm->hf_state = hfp_hf_bcs; if (!rfcomm->transport || (rfcomm->transport->codec != selected_codec) ) { @@ -894,23 +929,23 @@ static bool rfcomm_hfp_hf(struct spa_source *source, char* buf) switch(rfcomm->hf_state) { case hfp_hf_brsf: if (rfcomm->codec_negotiation_supported) { - rfcomm_send_cmd(source, "AT+BAC=1,2"); + rfcomm_send_cmd(rfcomm, "AT+BAC=1,2"); rfcomm->hf_state = hfp_hf_bac; } else { - rfcomm_send_cmd(source, "AT+CIND=?"); + rfcomm_send_cmd(rfcomm, "AT+CIND=?"); rfcomm->hf_state = hfp_hf_cind1; } break; case hfp_hf_bac: - rfcomm_send_cmd(source, "AT+CIND=?"); + rfcomm_send_cmd(rfcomm, "AT+CIND=?"); rfcomm->hf_state = hfp_hf_cind1; break; case hfp_hf_cind1: - rfcomm_send_cmd(source, "AT+CIND?"); + rfcomm_send_cmd(rfcomm, "AT+CIND?"); rfcomm->hf_state = hfp_hf_cind2; break; case hfp_hf_cind2: - rfcomm_send_cmd(source, "AT+CMER=3,0,0,0"); + rfcomm_send_cmd(rfcomm, "AT+CMER=3,0,0,0"); rfcomm->hf_state = hfp_hf_cmer; break; case hfp_hf_cmer: @@ -990,7 +1025,7 @@ static void rfcomm_event(struct spa_source *source) if (!res) { spa_log_debug(backend->log, NAME": RFCOMM receive unsupported command: %s", buf); - rfcomm_send_reply(source, "ERROR"); + rfcomm_send_reply(rfcomm, "ERROR"); } } return; @@ -1332,7 +1367,7 @@ static int sco_set_volume_cb(void *data, int id, float volume) struct spa_bt_transport_volume *t_volume = &t->volumes[id]; struct transport_data *td = t->user_data; struct rfcomm *rfcomm = td->rfcomm; - char *msg; + const char *format; int value; if (!rfcomm_volume_enabled(rfcomm) @@ -1349,21 +1384,20 @@ static int sco_set_volume_cb(void *data, int id, float volume) if (id == SPA_BT_VOLUME_ID_RX) if (rfcomm->profile & SPA_BT_PROFILE_HFP_HF) - msg = spa_aprintf("+VGM: %d", value); + format = "+VGM: %d"; else - msg = spa_aprintf("+VGM=%d", value); + format = "+VGM=%d"; else if (id == SPA_BT_VOLUME_ID_TX) if (rfcomm->profile & SPA_BT_PROFILE_HFP_HF) - msg = spa_aprintf("+VGS: %d", value); + format = "+VGS: %d"; else - msg = spa_aprintf("+VGS=%d", value); + format = "+VGS=%d"; else spa_assert_not_reached(); if (rfcomm->transport) - rfcomm_send_reply(&rfcomm->source, msg); + rfcomm_send_reply(rfcomm, format, value); - free(msg); return 0; } @@ -1443,7 +1477,7 @@ static void codec_switch_timer_event(struct spa_source *source) case HFP_AG_INITIAL_CODEC_SETUP_SEND: /* Retry codec selection */ rfcomm->hfp_ag_initial_codec_setup = HFP_AG_INITIAL_CODEC_SETUP_WAIT; - rfcomm_send_reply(&rfcomm->source, "+BCS: 2"); + rfcomm_send_reply(rfcomm, "+BCS: 2"); codec_switch_start_timer(rfcomm, HFP_CODEC_SWITCH_TIMEOUT_MSEC); return; case HFP_AG_INITIAL_CODEC_SETUP_WAIT: @@ -1458,7 +1492,7 @@ static void codec_switch_timer_event(struct spa_source *source) spa_bt_device_connect_profile(rfcomm->device, rfcomm->profile); } } - rfcomm_send_reply(&rfcomm->source, "+BCS: 1"); + rfcomm_send_reply(rfcomm, "+BCS: 1"); return; default: break; @@ -1500,7 +1534,6 @@ static int backend_native_ensure_codec(void *data, struct spa_bt_device *device, struct impl *backend = data; struct rfcomm *rfcomm; int res; - char msg[16]; res = backend_native_supports_codec(data, device, codec); if (res <= 0) @@ -1518,8 +1551,7 @@ static int backend_native_ensure_codec(void *data, struct spa_bt_device *device, return 0; } - snprintf(msg, sizeof(msg), "+BCS: %d", codec); - if ((res = rfcomm_send_reply(&rfcomm->source, msg)) < 0) + if ((res = rfcomm_send_reply(rfcomm, "+BCS: %u", codec)) < 0) return res; rfcomm->hfp_ag_switching_codec = true; @@ -1636,7 +1668,6 @@ static DBusHandlerResult profile_new_connection(DBusConnection *conn, DBusMessag } else if (profile == SPA_BT_PROFILE_HFP_AG) { /* Start SLC connection */ unsigned int hf_features = SPA_BT_HFP_HF_FEATURE_NONE; - char *cmd; /* Decide if we want to signal that the HF supports mSBC negotiation This should be done when the bluetooth adapter supports the necessary transport mode */ @@ -1656,9 +1687,8 @@ static DBusHandlerResult profile_new_connection(DBusConnection *conn, DBusMessag } /* send command to AG with the features supported by Hands-Free */ - cmd = spa_aprintf("AT+BRSF=%u", hf_features); - rfcomm_send_cmd(&rfcomm->source, cmd); - free(cmd); + rfcomm_send_cmd(rfcomm, "AT+BRSF=%u", hf_features); + rfcomm->hf_state = hfp_hf_brsf; }