Skip to content

nimble/host: Fix ATT Read By Type and Find By Type Value responses limited by hardcoded buffer sizes #2171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nychyporuk wants to merge 2 commits into
base: master
Choose a base branch
from
+200 −15
Open

nimble/host: Fix ATT Read By Type and Find By Type Value responses limited by hardcoded buffer sizes #2171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c34aa22
nimble/host: Fix Read By Type and Find By Type Val hardcoded buff limits
nychyporuk Feb 9, 2026
ad54c58
nimble/host: Add UT for large value Read By Type and Find By Type Value
nychyporuk Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 61 additions & 13 deletions nimble/host/src/ble_att_svr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1089,8 +1089,9 @@ ble_att_svr_fill_type_value(uint16_t conn_handle,
uint16_t mtu, uint8_t *out_att_err)
{
struct ble_att_svr_entry *ha;
uint8_t buf[16];
struct os_mbuf *attr_om;
uint16_t attr_len;
uint16_t req_val_len;
uint16_t first;
uint16_t prev;
int any_entries;
Expand All @@ -1099,6 +1100,11 @@ ble_att_svr_fill_type_value(uint16_t conn_handle,
first = 0;
prev = 0;
rc = 0;
attr_om = NULL;

/* Length of the attribute value in the request. */
req_val_len = OS_MBUF_PKTLEN(rxom) -
sizeof(struct ble_att_find_type_value_req);

/* Iterate through the attribute list, keeping track of the current
* matching group. For each attribute entry, determine if data needs to be
Expand Down Expand Up @@ -1141,15 +1147,31 @@ ble_att_svr_fill_type_value(uint16_t conn_handle,
* determine if this attribute matches.
*/
if (ble_uuid_cmp(ha->ha_uuid, &attr_type.u) == 0) {
rc = ble_att_svr_read_flat(conn_handle, ha, 0, sizeof buf, buf,
&attr_len, out_att_err);
/* Lazily allocate a temporary mbuf for reading attribute values. */
if (attr_om == NULL) {
attr_om = ble_hs_mbuf_l2cap_pkt();
if (attr_om == NULL) {
*out_att_err = BLE_ATT_ERR_INSUFFICIENT_RES;
rc = BLE_HS_ENOMEM;
goto done;
}
} else {
os_mbuf_adj(attr_om, OS_MBUF_PKTLEN(attr_om));
}

/* Read attribute value into temporary mbuf. */
rc = ble_att_svr_read(conn_handle, ha, 0, attr_om, out_att_err);
if (rc != 0) {
goto done;
}
/* value is at the end of req */
rc = os_mbuf_cmpf(rxom, sizeof(struct ble_att_find_type_value_req),
buf, attr_len);
if (rc == 0) {

attr_len = OS_MBUF_PKTLEN(attr_om);

/* Compare attribute value with the value from the request. */
if (attr_len == req_val_len &&
os_mbuf_cmpm(rxom,
sizeof(struct ble_att_find_type_value_req),
attr_om, 0, attr_len) == 0) {
first = ha->ha_handle_id;
prev = ha->ha_handle_id;
}
Expand All @@ -1170,6 +1192,8 @@ ble_att_svr_fill_type_value(uint16_t conn_handle,
}

done:
os_mbuf_free_chain(attr_om);

any_entries = OS_MBUF_PKTHDR(txom)->omp_len >
BLE_ATT_FIND_TYPE_VALUE_RSP_BASE_SZ;
if (rc == 0 && !any_entries) {
Expand Down Expand Up @@ -1293,9 +1317,10 @@ ble_att_svr_build_read_type_rsp(uint16_t conn_handle, uint16_t cid,
struct ble_att_read_type_rsp *rsp;
struct ble_att_svr_entry *entry;
struct os_mbuf *txom;
struct os_mbuf *attr_om;
uint16_t attr_len;
uint16_t max_attr_len;
uint16_t mtu;
uint8_t buf[19];
int entry_written;
int txomlen;
int prev_attr_len;
Expand All @@ -1306,6 +1331,7 @@ ble_att_svr_build_read_type_rsp(uint16_t conn_handle, uint16_t cid,
*err_handle = start_handle;
entry_written = 0;
prev_attr_len = 0;
attr_om = NULL;

/* Just reuse the request buffer for the response. */
txom = *rxom;
Expand All @@ -1326,6 +1352,12 @@ ble_att_svr_build_read_type_rsp(uint16_t conn_handle, uint16_t cid,

mtu = ble_att_mtu_by_cid(conn_handle, cid);

/* Per Core Spec: max attribute value length is min(ATT_MTU - 4, 253). */
max_attr_len = mtu - 4;
if (max_attr_len > 253) {
max_attr_len = 253;
}

/* Find all matching attributes, writing a record for each. */
entry = NULL;
while (1) {
Expand All @@ -1336,15 +1368,29 @@ ble_att_svr_build_read_type_rsp(uint16_t conn_handle, uint16_t cid,
}

if (entry->ha_handle_id >= start_handle) {
rc = ble_att_svr_read_flat(conn_handle, entry, 0, sizeof buf, buf,
&attr_len, att_err);
/* Lazily allocate a temporary mbuf for reading attribute values. */
if (attr_om == NULL) {
attr_om = ble_hs_mbuf_l2cap_pkt();
if (attr_om == NULL) {
*att_err = BLE_ATT_ERR_INSUFFICIENT_RES;
*err_handle = entry->ha_handle_id;
rc = BLE_HS_ENOMEM;
goto done;
}
} else {
os_mbuf_adj(attr_om, OS_MBUF_PKTLEN(attr_om));
}

/* Read attribute value into temporary mbuf. */
rc = ble_att_svr_read(conn_handle, entry, 0, attr_om, att_err);
if (rc != 0) {
*err_handle = entry->ha_handle_id;
goto done;
}

if (attr_len > mtu - 4) {
attr_len = mtu - 4;
attr_len = OS_MBUF_PKTLEN(attr_om);
if (attr_len > max_attr_len) {
attr_len = max_attr_len;
}

if (prev_attr_len == 0) {
Expand All @@ -1367,12 +1413,14 @@ ble_att_svr_build_read_type_rsp(uint16_t conn_handle, uint16_t cid,
}

data->handle = htole16(entry->ha_handle_id);
memcpy(data->value, buf, attr_len);
os_mbuf_copydata(attr_om, 0, attr_len, data->value);
entry_written = 1;
}
}

done:
os_mbuf_free_chain(attr_om);

if (!entry_written) {
/* No matching attributes. */
if (*att_err == 0) {
Expand Down
137 changes: 137 additions & 0 deletions nimble/host/test/src/ble_att_svr_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2126,6 +2126,142 @@ TEST_CASE_SELF(ble_att_svr_test_unsupported_req)
ble_att_svr_test_assert_mbufs_freed();
}

/**
* Tests that Read By Type and Find By Type Value work correctly with attribute
* values larger than the old hardcoded buffer limits (19 and 16 bytes
* respectively). Verifies proper truncation per Core Spec 3.4.4.2:
* max value length = min(ATT_MTU - 4, 253).
*/
TEST_CASE_SELF(ble_att_svr_test_large_value)
{
static uint8_t large_val[128];
uint16_t conn_handle;
uint16_t mtu;
uint16_t expected_len;
int rc;
int i;

/* Fill test buffer with a recognizable pattern. */
for (i = 0; i < (int)sizeof(large_val); i++) {
large_val[i] = i;
}

/***
* Read By Type with large attribute value (MTU = 260).
* max value = min(260 - 4, 253) = 253, but our value is 128 bytes so
* the full value should be returned without truncation.
*/
mtu = 260;
conn_handle = ble_att_svr_test_misc_init(mtu);

ble_att_svr_test_attr_r_1 = large_val;
ble_att_svr_test_attr_r_1_len = sizeof(large_val);

/* Register a primary service attribute with the r_1 access callback. */
ble_att_svr_test_misc_register_uuid(
BLE_UUID16_DECLARE(BLE_ATT_UUID_PRIMARY_SERVICE),
HA_FLAG_PERM_RW, 1, ble_att_svr_test_misc_attr_fn_r_1);

/* Read by type: value is 128 bytes, fits within min(256, 253) = 253. */
rc = ble_hs_test_util_rx_att_read_type_req16(conn_handle, 1, 0xffff,
BLE_ATT_UUID_PRIMARY_SERVICE);
TEST_ASSERT(rc == 0);
ble_att_svr_test_misc_verify_tx_read_type_rsp(
((struct ble_att_svr_test_type_entry[]) { {
.handle = 1,
.value = large_val,
.value_len = sizeof(large_val),
}, {
.handle = 0,
} }));

/***
* Read By Type with value truncated by MTU.
* MTU = 64, max value = min(64 - 4, 253) = 60. Value is 128 bytes,
* so response should contain only the first 60 bytes.
*/
mtu = 64;
conn_handle = ble_att_svr_test_misc_init(mtu);

ble_att_svr_test_attr_r_1 = large_val;
ble_att_svr_test_attr_r_1_len = sizeof(large_val);

ble_att_svr_test_misc_register_uuid(
BLE_UUID16_DECLARE(BLE_ATT_UUID_PRIMARY_SERVICE),
HA_FLAG_PERM_RW, 1, ble_att_svr_test_misc_attr_fn_r_1);

expected_len = mtu - 4; /* 60 */

rc = ble_hs_test_util_rx_att_read_type_req16(conn_handle, 1, 0xffff,
BLE_ATT_UUID_PRIMARY_SERVICE);
TEST_ASSERT(rc == 0);
ble_att_svr_test_misc_verify_tx_read_type_rsp(
((struct ble_att_svr_test_type_entry[]) { {
.handle = 1,
.value = large_val,
.value_len = expected_len,
}, {
.handle = 0,
} }));

/***
* Find By Type Value with large attribute value (> 16 bytes).
* The request value must match the full attribute value for a hit.
*/
mtu = 260;
conn_handle = ble_att_svr_test_misc_init(mtu);

ble_att_svr_test_attr_r_1 = large_val;
ble_att_svr_test_attr_r_1_len = sizeof(large_val);

ble_att_svr_test_misc_register_uuid(
BLE_UUID16_DECLARE(BLE_ATT_UUID_PRIMARY_SERVICE),
HA_FLAG_PERM_RW, 1, ble_att_svr_test_misc_attr_fn_r_1);

/* Search with the correct 128-byte value: should match. */
rc = ble_hs_test_util_rx_att_find_type_value_req(
conn_handle, 1, 0xffff, BLE_ATT_UUID_PRIMARY_SERVICE,
large_val, sizeof(large_val));
TEST_ASSERT(rc == 0);
ble_att_svr_test_misc_verify_tx_find_type_value_rsp(
((struct ble_att_svr_test_type_value_entry[]) { {
.first = 1,
.last = 1,
}, {
.first = 0,
} }));

/***
* Find By Type Value with wrong large value: should not match.
*/
{
uint8_t wrong_val[128];
memcpy(wrong_val, large_val, sizeof(wrong_val));
wrong_val[64] ^= 0xff; /* Flip one byte in the middle. */

rc = ble_hs_test_util_rx_att_find_type_value_req(
conn_handle, 1, 0xffff, BLE_ATT_UUID_PRIMARY_SERVICE,
wrong_val, sizeof(wrong_val));
TEST_ASSERT(rc != 0);
ble_hs_test_util_verify_tx_err_rsp(
BLE_ATT_OP_FIND_TYPE_VALUE_REQ, 1,
BLE_ATT_ERR_ATTR_NOT_FOUND);
}

/***
* Find By Type Value with length mismatch: should not match.
*/
rc = ble_hs_test_util_rx_att_find_type_value_req(
conn_handle, 1, 0xffff, BLE_ATT_UUID_PRIMARY_SERVICE,
large_val, 64);
TEST_ASSERT(rc != 0);
ble_hs_test_util_verify_tx_err_rsp(
BLE_ATT_OP_FIND_TYPE_VALUE_REQ, 1,
BLE_ATT_ERR_ATTR_NOT_FOUND);

ble_att_svr_test_assert_mbufs_freed();
}

TEST_SUITE(ble_att_svr_suite)
{
ble_att_svr_test_mtu();
Expand All @@ -2143,4 +2279,5 @@ TEST_SUITE(ble_att_svr_suite)
ble_att_svr_test_indicate();
ble_att_svr_test_oom();
ble_att_svr_test_unsupported_req();
ble_att_svr_test_large_value();
}
4 changes: 2 additions & 2 deletions nimble/host/test/src/ble_hs_test_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,10 +777,10 @@ ble_hs_test_util_rx_att_find_type_value_req(uint16_t conn_handle,
uint16_t attr_len)
{
struct ble_att_find_type_value_req req;
uint8_t buf[BLE_ATT_FIND_TYPE_VALUE_REQ_BASE_SZ + 16];
uint8_t buf[BLE_ATT_FIND_TYPE_VALUE_REQ_BASE_SZ + BLE_ATT_ATTR_MAX_LEN];
int rc;

TEST_ASSERT(attr_len <= 16);
TEST_ASSERT(attr_len <= BLE_ATT_ATTR_MAX_LEN);

req.bavq_start_handle = start_handle;
req.bavq_end_handle = end_handle;
Expand Down
Loading