2024-01-18 21:33:26

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/2] player: Fix endpoint.config for broadcast

From: Luiz Augusto von Dentz <[email protected]>

endpoint.config where taking different arguments for broadcast which is
not recommended with shell as it doesn't support such a thing.

So instead of taking different arguments for broadcast both remote and
local endpoints shall be passed but in case of broadcast sync both the
remote and the local endpoint actually refer to the same endpoint
registered by bluetoothctl:

endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep2 16_2_1
---
client/player.c | 36 ++++++------------------------------
1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/client/player.c b/client/player.c
index 39d1be665c8f..1bc64af3c2ae 100644
--- a/client/player.c
+++ b/client/player.c
@@ -3642,10 +3642,6 @@ static void cmd_config_endpoint(int argc, char *argv[])
{
struct endpoint_config *cfg;
const struct codec_preset *preset;
- const struct capabilities *cap;
- char *uuid;
- uint8_t codec_id;
- bool broadcast = false;

cfg = new0(struct endpoint_config, 1);

@@ -3660,33 +3656,14 @@ static void cmd_config_endpoint(int argc, char *argv[])
/* Search for the local endpoint */
cfg->ep = endpoint_find(argv[2]);
if (!cfg->ep) {
-
- /* When the local endpoint was not found either we received
- * UUID, or the provided local endpoint is not available
- */
- uuid = argv[2];
- codec_id = strtol(argv[3], NULL, 0);
- cap = find_capabilities(uuid, codec_id);
- if (cap) {
- broadcast = true;
- cfg->ep = endpoint_new(cap);
- cfg->ep->preset = find_presets_name(uuid, argv[3]);
- if (!cfg->ep->preset)
- bt_shell_printf("Preset not found\n");
- } else {
- bt_shell_printf("Local Endpoint %s,"
- "or capabilities not found\n", uuid);
- goto fail;
- }
+ bt_shell_printf("Local Endpoint %s not found\n", argv[2]);
+ goto fail;
}

- if (((broadcast == false) && (argc > 3)) ||
- ((broadcast == true) && (argc > 4))) {
- char *preset_name = (broadcast == false)?argv[3]:argv[4];
-
- preset = preset_find_name(cfg->ep->preset, preset_name);
+ if (argc > 3) {
+ preset = preset_find_name(cfg->ep->preset, argv[3]);
if (!preset) {
- bt_shell_printf("Preset %s not found\n", preset_name);
+ bt_shell_printf("Preset %s not found\n", argv[3]);
goto fail;
}

@@ -4105,8 +4082,7 @@ static const struct bt_shell_menu endpoint_menu = {
{ "unregister", "<UUID/object>", cmd_unregister_endpoint,
"Register Endpoint",
local_endpoint_generator },
- { "config",
- "<endpoint> [local endpoint/UUID] [preset/codec id] [preset]",
+ { "config", "<endpoint> [local endpoint] [preset]",
cmd_config_endpoint,
"Configure Endpoint",
endpoint_generator },
--
2.43.0



2024-01-18 21:33:29

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ v2 2/2] bap: Fix crash when attempting to setup a broadcast source

From: Luiz Augusto von Dentz <[email protected]>

This fixes a crash which could be observed with the following commands:

Run bluetoothctl -e:
> endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep2 16_2_1
> transport.acquire /org/bluez/hci0/pac_bcast0/fd0
---
profiles/audio/bap.c | 127 ++++++++++++++++++++-----------------
profiles/audio/transport.c | 3 +-
2 files changed, 70 insertions(+), 60 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index b888764855ef..69e982832f89 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -66,6 +66,8 @@ struct bap_setup {
struct bap_ep *ep;
struct bt_bap_stream *stream;
struct bt_bap_qos qos;
+ int (*qos_parser)(struct bap_setup *setup, const char *key, int var,
+ DBusMessageIter *iter);
GIOChannel *io;
unsigned int io_id;
bool recreate;
@@ -75,6 +77,7 @@ struct bap_setup {
unsigned int id;
struct iovec *base;
DBusMessage *msg;
+ void (*destroy)(struct bap_setup *setup);
};

struct bap_ep {
@@ -514,9 +517,11 @@ static int parse_io_qos(const char *key, int var, DBusMessageIter *iter,
return 0;
}

-static int parse_ucast_qos(const char *key, int var, DBusMessageIter *iter,
- struct bt_bap_qos *qos)
+static int setup_parse_ucast_qos(struct bap_setup *setup, const char *key,
+ int var, DBusMessageIter *iter)
{
+ struct bt_bap_qos *qos = &setup->qos;
+
if (!strcasecmp(key, "CIG")) {
if (var != DBUS_TYPE_BYTE)
return -EINVAL;
@@ -553,9 +558,18 @@ static int parse_ucast_qos(const char *key, int var, DBusMessageIter *iter,
return 0;
}

-static int parse_bcast_qos(const char *key, int var, DBusMessageIter *iter,
- struct bt_bap_qos *qos)
+static void setup_bcast_destroy(struct bap_setup *setup)
{
+ struct bt_bap_qos *qos = &setup->qos;
+
+ util_iov_free(qos->bcast.bcode, 1);
+}
+
+static int setup_parse_bcast_qos(struct bap_setup *setup, const char *key,
+ int var, DBusMessageIter *iter)
+{
+ struct bt_bap_qos *qos = &setup->qos;
+
if (!strcasecmp(key, "Encryption")) {
if (var != DBUS_TYPE_BYTE)
return -EINVAL;
@@ -602,8 +616,15 @@ static int parse_bcast_qos(const char *key, int var, DBusMessageIter *iter,
if (var != DBUS_TYPE_ARRAY)
return -EINVAL;

+ memset(&iov, 0, sizeof(iov));
+
parse_array(iter, &iov);

+ if (iov.iov_len != 16) {
+ error("Invalid size for BCode: %zu != 16", iov.iov_len);
+ return -EINVAL;
+ }
+
util_iov_free(qos->bcast.bcode, 1);
qos->bcast.bcode = util_iov_dup(&iov, 1);
} else {
@@ -617,18 +638,10 @@ static int parse_bcast_qos(const char *key, int var, DBusMessageIter *iter,
return 0;
}

-static int parse_qos(DBusMessageIter *iter, struct bt_bap_qos *qos,
- struct iovec **base)
+static int setup_parse_qos(struct bap_setup *setup, DBusMessageIter *iter)
{
DBusMessageIter array;
const char *key;
- int (*parser)(const char *key, int var, DBusMessageIter *iter,
- struct bt_bap_qos *qos);
-
- if (*base)
- parser = parse_bcast_qos;
- else
- parser = parse_ucast_qos;

dbus_message_iter_recurse(iter, &array);

@@ -644,7 +657,7 @@ static int parse_qos(DBusMessageIter *iter, struct bt_bap_qos *qos,

var = dbus_message_iter_get_arg_type(&value);

- err = parser(key, var, &value, qos);
+ err = setup->qos_parser(setup, key, var, &value);
if (err) {
DBG("Failed parsing %s", key);
return err;
@@ -653,12 +666,23 @@ static int parse_qos(DBusMessageIter *iter, struct bt_bap_qos *qos,
dbus_message_iter_next(&array);
}

+ if (queue_find(setup->ep->data->bcast, NULL, setup->ep)) {
+ uint32_t presDelay;
+ uint8_t numSubgroups, numBis;
+ struct bt_bap_codec codec;
+
+ util_iov_free(setup->base, 1);
+ setup->base = util_iov_dup(setup->caps, 1);
+ parse_base(setup->base->iov_base, setup->base->iov_len,
+ bap_debug, &presDelay, &numSubgroups, &numBis,
+ &codec, &setup->caps, &setup->metadata);
+ }
+
return 0;
}

-static int parse_configuration(DBusMessageIter *props, struct iovec **caps,
- struct iovec **metadata, struct iovec **base,
- struct bt_bap_qos *qos)
+static int setup_parse_configuration(struct bap_setup *setup,
+ DBusMessageIter *props)
{
const char *key;
struct iovec iov;
@@ -684,8 +708,8 @@ static int parse_configuration(DBusMessageIter *props, struct iovec **caps,
if (parse_array(&value, &iov))
goto fail;

- util_iov_free(*caps, 1);
- *caps = util_iov_dup(&iov, 1);
+ util_iov_free(setup->caps, 1);
+ setup->caps = util_iov_dup(&iov, 1);
} else if (!strcasecmp(key, "Metadata")) {
if (var != DBUS_TYPE_ARRAY)
goto fail;
@@ -693,40 +717,24 @@ static int parse_configuration(DBusMessageIter *props, struct iovec **caps,
if (parse_array(&value, &iov))
goto fail;

- util_iov_free(*metadata, 1);
- *metadata = util_iov_dup(&iov, 1);
+ util_iov_free(setup->metadata, 1);
+ setup->metadata = util_iov_dup(&iov, 1);
} else if (!strcasecmp(key, "QoS")) {
if (var != DBUS_TYPE_ARRAY)
goto fail;

- if (parse_qos(&value, qos, base))
+ if (setup_parse_qos(setup, &value))
goto fail;
}

dbus_message_iter_next(props);
}

- if (*base) {
- uint32_t presDelay;
- uint8_t numSubgroups, numBis;
- struct bt_bap_codec codec;
-
- util_iov_memcpy(*base, (*caps)->iov_base, (*caps)->iov_len);
- parse_base((*caps)->iov_base, (*caps)->iov_len, bap_debug,
- &presDelay, &numSubgroups, &numBis, &codec,
- caps, NULL);
- }
-
return 0;

fail:
DBG("Failed parsing %s", key);

- if (*caps) {
- free(*caps);
- *caps = NULL;
- }
-
return -EINVAL;
}

@@ -819,6 +827,19 @@ static struct bap_setup *setup_new(struct bap_ep *ep)
setup = new0(struct bap_setup, 1);
setup->ep = ep;

+ if (queue_find(ep->data->bcast, NULL, ep)) {
+ /* Mark BIG and BIS to be auto assigned */
+ setup->qos.bcast.big = BT_ISO_QOS_BIG_UNSET;
+ setup->qos.bcast.bis = BT_ISO_QOS_BIS_UNSET;
+ setup->qos_parser = setup_parse_bcast_qos;
+ setup->destroy = setup_bcast_destroy;
+ } else {
+ /* Mark CIG and CIS to be auto assigned */
+ setup->qos.ucast.cig_id = BT_ISO_QOS_CIG_UNSET;
+ setup->qos.ucast.cis_id = BT_ISO_QOS_CIS_UNSET;
+ setup->qos_parser = setup_parse_ucast_qos;
+ }
+
if (!ep->setups)
ep->setups = queue_new();

@@ -844,8 +865,8 @@ static void setup_free(void *data)
util_iov_free(setup->metadata, 1);
util_iov_free(setup->base, 1);

- if (bt_bap_stream_get_type(setup->stream) == BT_BAP_STREAM_TYPE_BCAST)
- util_iov_free(setup->qos.bcast.bcode, 1);
+ if (setup->destroy)
+ setup->destroy(setup);

free(setup);
}
@@ -872,18 +893,7 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,

setup = setup_new(ep);

- if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE) {
- /* Mark BIG and BIS to be auto assigned */
- setup->qos.bcast.big = BT_ISO_QOS_BIG_UNSET;
- setup->qos.bcast.bis = BT_ISO_QOS_BIS_UNSET;
- } else {
- /* Mark CIG and CIS to be auto assigned */
- setup->qos.ucast.cig_id = BT_ISO_QOS_CIG_UNSET;
- setup->qos.ucast.cis_id = BT_ISO_QOS_CIS_UNSET;
- }
-
- if (parse_configuration(&props, &setup->caps, &setup->metadata,
- &setup->base, &setup->qos) < 0) {
+ if (setup_parse_configuration(setup, &props) < 0) {
DBG("Unable to parse configuration");
setup_free(setup);
return btd_error_invalid_args(msg);
@@ -943,10 +953,10 @@ static void update_bcast_qos(struct bt_iso_qos *qos,
bap_qos->bcast.io_qos.phy = qos->bcast.in.phy;
bap_qos->bcast.io_qos.sdu = qos->bcast.in.sdu;
bap_qos->bcast.io_qos.rtn = qos->bcast.in.rtn;
-
- bap_qos->bcast.bcode = new0(struct iovec, 1);
+ if (!bap_qos->bcast.bcode)
+ bap_qos->bcast.bcode = new0(struct iovec, 1);
util_iov_memcpy(bap_qos->bcast.bcode, qos->bcast.bcode,
- sizeof(qos->bcast.bcode));
+ sizeof(qos->bcast.bcode));
}

static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data)
@@ -1901,6 +1911,8 @@ static void setup_create_bcast_io(struct bap_data *data,
struct bap_setup *setup,
struct bt_bap_stream *stream, int defer)
{
+ struct bt_bap_qos *qos = &setup->qos;
+ struct iovec *bcode = qos->bcast.bcode;
struct bt_iso_qos iso_qos;

memset(&iso_qos, 0, sizeof(iso_qos));
@@ -1914,9 +1926,8 @@ static void setup_create_bcast_io(struct bap_data *data,
iso_qos.bcast.packing = setup->qos.bcast.packing;
iso_qos.bcast.framing = setup->qos.bcast.framing;
iso_qos.bcast.encryption = setup->qos.bcast.encryption;
- if (setup->qos.bcast.bcode)
- memcpy(iso_qos.bcast.bcode, setup->qos.bcast.bcode->iov_base,
- 16);
+ if (bcode && bcode->iov_base)
+ memcpy(iso_qos.bcast.bcode, bcode->iov_base, bcode->iov_len);
iso_qos.bcast.options = setup->qos.bcast.options;
iso_qos.bcast.skip = setup->qos.bcast.skip;
iso_qos.bcast.sync_timeout = setup->qos.bcast.sync_timeout;
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 5395985b990f..d89e75915787 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1798,8 +1798,7 @@ struct media_transport *media_transport_create(struct btd_device *device,
transport->adapter = media_endpoint_get_btd_adapter(endpoint);

transport->endpoint = endpoint;
- transport->configuration = g_new(uint8_t, size);
- memcpy(transport->configuration, configuration, size);
+ transport->configuration = util_memdup(configuration, size);
transport->size = size;
transport->remote_endpoint = remote_endpoint;

--
2.43.0


2024-01-18 23:18:02

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ,v2,1/2] player: Fix endpoint.config for broadcast

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=817943

---Test result---

Test Summary:
CheckPatch PASS 0.97 seconds
GitLint PASS 0.61 seconds
BuildEll PASS 23.76 seconds
BluezMake PASS 714.53 seconds
MakeCheck PASS 11.76 seconds
MakeDistcheck PASS 160.17 seconds
CheckValgrind PASS 222.21 seconds
CheckSmatch PASS 337.08 seconds
bluezmakeextell PASS 106.76 seconds
IncrementalBuild PASS 1343.92 seconds
ScanBuild PASS 927.03 seconds



---
Regards,
Linux Bluetooth

2024-01-19 20:40:50

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 1/2] player: Fix endpoint.config for broadcast

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Thu, 18 Jan 2024 16:33:13 -0500 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> endpoint.config where taking different arguments for broadcast which is
> not recommended with shell as it doesn't support such a thing.
>
> So instead of taking different arguments for broadcast both remote and
> local endpoints shall be passed but in case of broadcast sync both the
> remote and the local endpoint actually refer to the same endpoint
> registered by bluetoothctl:
>
> [...]

Here is the summary with links:
- [BlueZ,v2,1/2] player: Fix endpoint.config for broadcast
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=4908e58bd1a9
- [BlueZ,v2,2/2] bap: Fix crash when attempting to setup a broadcast source
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=2ef2f122e608

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html