2023-07-07 22:45:16

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH v2 1/4] mgmt-tester: Fix tests that consider 31 bytes the max adv lenght

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

This fixes a couple of tests that consider 31 bytes the max advertising
length since in case of extended advertising that number is actually
251.
---
tools/mgmt-tester.c | 71 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index aec91fb41d3b..7dfd1b0c747d 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -5904,8 +5904,8 @@ static const char ext_adv_hci_params_valid[] = {
static const char ext_adv_params_mgmt_rsp_valid_50[] = {
0x01, /* instance */
0x00, /* tx_power defaults to 0 on BT5 platform*/
- 0x1f, /* max_adv_data_len */
- 0x1f, /* max_scan_rsp_len */
+ 0xfb, /* max_adv_data_len */
+ 0xfb, /* max_scan_rsp_len */
};

static const char ext_adv_params_mgmt_rsp_valid[] = {
@@ -7725,6 +7725,23 @@ static const uint8_t add_advertising_param_scrsp_data_only_too_long[] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00,
};

@@ -7778,6 +7795,29 @@ static const uint8_t add_advertising_param_scrsp_appear_data_too_long[] = {
/* scan rsp data: */
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
};

@@ -7991,6 +8031,29 @@ static const uint8_t add_advertising_param_name_data_inv[] = {
/* adv data: */
/* scan rsp data: */
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

@@ -8084,8 +8147,8 @@ static const struct generic_data set_appearance_success = {

static const uint8_t read_adv_features_rsp_3[] = {
0xff, 0xff, 0x01, 0x00, /* supported flags */
- 0x1f, /* max_adv_data_len */
- 0x1f, /* max_scan_rsp_len */
+ 0xfb, /* max_adv_data_len */
+ 0xfb, /* max_scan_rsp_len */
0x03, /* max_instances */
0x00, /* num_instances */
};
--
2.40.1



2023-07-07 22:57:26

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH v2 2/4] shared/ad: Fix hardcoding maximum data length

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

Instances shall not assume BT_AD_MAX_DATA_LEN is always the maximum
length as they could be used with EA which supports bigger length.
---
src/shared/ad.c | 56 ++++++++++++++++++++++++++++++-------------------
src/shared/ad.h | 2 ++
2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/shared/ad.c b/src/shared/ad.c
index 7350aa206d1e..8b3fde2b1dcd 100644
--- a/src/shared/ad.c
+++ b/src/shared/ad.c
@@ -22,6 +22,7 @@

struct bt_ad {
int ref_count;
+ uint8_t max_len;
char *name;
uint16_t appearance;
struct queue *service_uuids;
@@ -42,6 +43,7 @@ struct bt_ad *bt_ad_new(void)
struct bt_ad *ad;

ad = new0(struct bt_ad, 1);
+ ad->max_len = BT_AD_MAX_DATA_LEN;
ad->service_uuids = queue_new();
ad->manufacturer_data = queue_new();
ad->solicit_uuids = queue_new();
@@ -52,6 +54,16 @@ struct bt_ad *bt_ad_new(void)
return bt_ad_ref(ad);
}

+bool bt_ad_set_max_len(struct bt_ad *ad, uint8_t len)
+{
+ if (!ad || len < BT_AD_MAX_DATA_LEN)
+ return false;
+
+ ad->max_len = len;
+
+ return true;
+}
+
static bool ad_replace_data(struct bt_ad *ad, uint8_t type, const void *data,
size_t len);

@@ -298,17 +310,17 @@ static size_t uuid_data_length(struct queue *uuid_data)
return length;
}

-static size_t name_length(const char *name, size_t *pos)
+static size_t name_length(struct bt_ad *ad, size_t *pos)
{
size_t len;

- if (!name)
+ if (!ad->name)
return 0;

- len = 2 + strlen(name);
+ len = 2 + strlen(ad->name);

- if (len > BT_AD_MAX_DATA_LEN - *pos)
- len = BT_AD_MAX_DATA_LEN - *pos;
+ if (len > ad->max_len - (*pos))
+ len = ad->max_len - (*pos);

return len;
}
@@ -343,7 +355,7 @@ static size_t calculate_length(struct bt_ad *ad)

length += uuid_data_length(ad->service_data);

- length += name_length(ad->name, &length);
+ length += name_length(ad, &length);

length += ad->appearance != UINT16_MAX ? 4 : 0;

@@ -467,36 +479,36 @@ static void serialize_service_data(struct queue *service_data, uint8_t *buf,
}
}

-static void serialize_name(const char *name, uint8_t *buf, uint8_t *pos)
+static void serialize_name(struct bt_ad *ad, uint8_t *buf, uint8_t *pos)
{
int len;
uint8_t type = BT_AD_NAME_COMPLETE;

- if (!name)
+ if (!ad->name)
return;

- len = strlen(name);
- if (len > BT_AD_MAX_DATA_LEN - (*pos + 2)) {
+ len = strlen(ad->name);
+ if (len > ad->max_len - (*pos + 2)) {
type = BT_AD_NAME_SHORT;
- len = BT_AD_MAX_DATA_LEN - (*pos + 2);
+ len = ad->max_len - (*pos + 2);
}

buf[(*pos)++] = len + 1;
buf[(*pos)++] = type;

- memcpy(buf + *pos, name, len);
+ memcpy(buf + *pos, ad->name, len);
*pos += len;
}

-static void serialize_appearance(uint16_t value, uint8_t *buf, uint8_t *pos)
+static void serialize_appearance(struct bt_ad *ad, uint8_t *buf, uint8_t *pos)
{
- if (value == UINT16_MAX)
+ if (ad->appearance == UINT16_MAX)
return;

- buf[(*pos)++] = sizeof(value) + 1;
+ buf[(*pos)++] = sizeof(ad->appearance) + 1;
buf[(*pos)++] = BT_AD_GAP_APPEARANCE;

- bt_put_le16(value, buf + (*pos));
+ bt_put_le16(ad->appearance, buf + (*pos));
*pos += 2;
}

@@ -528,7 +540,7 @@ uint8_t *bt_ad_generate(struct bt_ad *ad, size_t *length)

*length = calculate_length(ad);

- if (*length > BT_AD_MAX_DATA_LEN)
+ if (*length > ad->max_len)
return NULL;

adv_data = malloc0(*length);
@@ -543,9 +555,9 @@ uint8_t *bt_ad_generate(struct bt_ad *ad, size_t *length)

serialize_service_data(ad->service_data, adv_data, &pos);

- serialize_name(ad->name, adv_data, &pos);
+ serialize_name(ad, adv_data, &pos);

- serialize_appearance(ad->appearance, adv_data, &pos);
+ serialize_appearance(ad, adv_data, &pos);

serialize_data(ad->data, adv_data, &pos);

@@ -653,7 +665,7 @@ bool bt_ad_add_manufacturer_data(struct bt_ad *ad, uint16_t manufacturer_id,
if (!ad)
return false;

- if (len > (BT_AD_MAX_DATA_LEN - 2 - sizeof(uint16_t)))
+ if (len > (ad->max_len - 2 - sizeof(uint16_t)))
return false;

new_data = queue_find(ad->manufacturer_data, manufacturer_id_data_match,
@@ -790,7 +802,7 @@ bool bt_ad_add_service_data(struct bt_ad *ad, const bt_uuid_t *uuid, void *data,
if (!ad)
return false;

- if (len > (BT_AD_MAX_DATA_LEN - 2 - (size_t)bt_uuid_len(uuid)))
+ if (len > (ad->max_len - 2 - (size_t)bt_uuid_len(uuid)))
return false;

new_data = queue_find(ad->service_data, service_uuid_match, uuid);
@@ -1009,7 +1021,7 @@ bool bt_ad_add_data(struct bt_ad *ad, uint8_t type, void *data, size_t len)
if (!ad)
return false;

- if (len > (BT_AD_MAX_DATA_LEN - 2))
+ if (len > (size_t)(ad->max_len - 2))
return false;

for (i = 0; i < sizeof(type_reject_list); i++) {
diff --git a/src/shared/ad.h b/src/shared/ad.h
index b100a6796109..93ba1b6cfa0b 100644
--- a/src/shared/ad.h
+++ b/src/shared/ad.h
@@ -98,6 +98,8 @@ struct bt_ad_pattern {

struct bt_ad *bt_ad_new(void);

+bool bt_ad_set_max_len(struct bt_ad *ad, uint8_t len);
+
struct bt_ad *bt_ad_new_with_data(size_t len, const uint8_t *data);

struct bt_ad *bt_ad_ref(struct bt_ad *ad);
--
2.40.1


2023-07-07 22:57:42

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH v2 3/4] advertising: Use bt_ad_set_max_len

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

This uses bt_ad_set_max_len to properly set the maximum data length of
the bt_ad instances based on what the kernel returns.
---
src/advertising.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/src/advertising.c b/src/advertising.c
index 0dceb14c3be4..6332ec8f9222 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -1556,10 +1556,14 @@ static struct btd_adv_client *client_create(struct btd_adv_manager *manager,
if (!client->data)
goto fail;

+ bt_ad_set_max_len(client->data, manager->max_adv_len);
+
client->scan = bt_ad_new();
if (!client->scan)
goto fail;

+ bt_ad_set_max_len(client->scan, manager->max_scan_rsp_len);
+
client->manager = manager;
client->appearance = UINT16_MAX;
client->tx_power = ADV_TX_POWER_NO_PREFERENCE;
--
2.40.1


2023-07-07 22:58:20

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH v2 4/4] client/advetising: Allow use of EA data length

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

The code was supporting a maximum of 25 bytes (31 - 6) to be entered as
advertising data, but in case of EA is used that allows up to 245 bytes
(251 - 6) to be entered.
---
client/advertising.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/advertising.c b/client/advertising.c
index 24852d93d1ec..a7474d6a2984 100644
--- a/client/advertising.c
+++ b/client/advertising.c
@@ -28,7 +28,7 @@
#define AD_IFACE "org.bluez.LEAdvertisement1"

struct ad_data {
- uint8_t data[25];
+ uint8_t data[245];
uint8_t len;
};

--
2.40.1


2023-07-08 01:13:05

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2,1/4] mgmt-tester: Fix tests that consider 31 bytes the max adv lenght

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=763577

---Test result---

Test Summary:
CheckPatch PASS 1.58 seconds
GitLint PASS 1.11 seconds
BuildEll PASS 34.28 seconds
BluezMake PASS 1015.70 seconds
MakeCheck PASS 13.60 seconds
MakeDistcheck PASS 188.77 seconds
CheckValgrind PASS 308.41 seconds
CheckSmatch PASS 404.99 seconds
bluezmakeextell PASS 124.11 seconds
IncrementalBuild PASS 3196.63 seconds
ScanBuild WARNING 1268.29 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
src/shared/ad.c:381:19: warning: Use of zero-allocated memory
buf[(*pos)++] = ad_type;
^
1 warning generated.



---
Regards,
Linux Bluetooth

2023-07-10 19:54:03

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] mgmt-tester: Fix tests that consider 31 bytes the max adv lenght

Hello:

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

On Fri, 7 Jul 2023 15:44:31 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This fixes a couple of tests that consider 31 bytes the max advertising
> length since in case of extended advertising that number is actually
> 251.
> ---
> tools/mgmt-tester.c | 71 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 67 insertions(+), 4 deletions(-)

Here is the summary with links:
- [v2,1/4] mgmt-tester: Fix tests that consider 31 bytes the max adv lenght
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=b9e7ca94d458
- [v2,2/4] shared/ad: Fix hardcoding maximum data length
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=2cb07aa669e5
- [v2,3/4] advertising: Use bt_ad_set_max_len
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=153aaeda2b21
- [v2,4/4] client/advetising: Allow use of EA data length
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=4578395b5370

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