* [PATCH bluez v2] monitor: Fix stack-buffer-overflow in packet_hci_scodata()
@ 2026-10-27 12:03 Zijun Hu
2026-10-27 15:26 ` [bluez,v2] " bluez.test.bot
0 siblings, 1 reply; 2+ messages in thread
From: Zijun Hu @ 2026-10-27 12:03 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz
Cc: Zijun Hu, linux-bluetooth, linux-kernel
packet_hci_scodata() defines 'char handle_str[42]', but the combined
format strings require 61 bytes in the worst case, including the
terminating NUL:
"Handle %d [%u/%u]" " [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X (%.16s)]"
Handle 4095 [255/255] [11:22:33:44:55:66 (0123456789abcdef)]
It calls handle_str_append_addr() to append the string to the array
without validating the remaining size, causing the stack-buffer-overflow
shown by the AddressSanitizer trace below.
Fix by:
- Increase the buffer size to 64.
- Use snprintf() instead of sprintf().
Also validate the SCO header before accessing its fields.
AddressSanitizer: WRITE of size 40 at 0x7bd1443dedba thread T0
#0 0x7bd1474ced68 in vsprintf
#1 0x7bd1474d0643 in __sprintf_chk
#2 0x5f5c39d02ea7 in sprintf
#3 0x5f5c39d02ea7 in handle_str_append_addr
../monitor/packet.c:14995
#4 0x5f5c39d58f19 in packet_hci_scodata
../monitor/packet.c:15125
#5 0x5f5c39d5eaf9 in packet_monitor
../monitor/packet.c:4841
#6 0x5f5c39cc4094 in data_callback
../monitor/control.c:991
#7 0x5f5c39e7fa7d in mainloop_run
../src/shared/mainloop.c:104
Fixes: 611f84a0ff0e ("monitor: Annotate ACL/SCO/ISO data with device address")
Assisted-by: Codex:gpt-5.6-terra
---
Changes in v2:
- Wrap long lines to fit within 80 columns.
- Improve commit message and add Assisted-by tag.
- Link to v1: https://patch.msgid.link/20260924-fix_sco-v1-1-ddca18333914@oss.qualcomm.com
---
monitor/packet.c | 48 +++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/monitor/packet.c b/monitor/packet.c
index a301e1ae7326..e2f0073e5a49 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -14968,62 +14968,68 @@ static void packet_enqueue_tx(struct timeval *tv, uint16_t handle,
frame = new0(struct packet_frame, 1);
if (tv)
memcpy(&frame->tv, tv, sizeof(*tv));
frame->num = num;
frame->len = len;
queue_push_tail(conn->tx_q, frame);
}
-static void handle_str_append_addr(char *handle_str,
+static void handle_str_append_addr(char *handle_str, size_t handle_str_size,
struct packet_conn_data *conn)
{
+ size_t len;
+
if (!conn)
return;
+ len = strlen(handle_str);
+ if (len >= handle_str_size)
+ return;
+
switch (conn->dst_type) {
case 0x00:
case 0x02:
if (conn->dst_oui) {
- sprintf(handle_str + strlen(handle_str),
+ snprintf(handle_str + len, handle_str_size - len,
" [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X (%.16s)]",
conn->dst[5], conn->dst[4], conn->dst[3],
conn->dst[2], conn->dst[1], conn->dst[0],
conn->dst_oui);
return;
}
break;
case 0x01:
case 0x03:
if (conn->dst_rtype) {
- sprintf(handle_str + strlen(handle_str),
+ snprintf(handle_str + len, handle_str_size - len,
" [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X (%.16s)]",
conn->dst[5], conn->dst[4], conn->dst[3],
conn->dst[2], conn->dst[1], conn->dst[0],
conn->dst_rtype);
return;
}
break;
}
- sprintf(handle_str + strlen(handle_str),
+ snprintf(handle_str + len, handle_str_size - len,
" [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X]",
conn->dst[5], conn->dst[4], conn->dst[3],
conn->dst[2], conn->dst[1], conn->dst[0]);
}
void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
bool in, const void *data, uint16_t size)
{
const struct bt_hci_acl_hdr *hdr = data;
uint16_t handle = le16_to_cpu(hdr->handle);
uint16_t dlen = le16_to_cpu(hdr->dlen);
uint8_t flags = acl_flags(handle);
char label[8];
- char handle_str[58], extra_str[32];
+ char handle_str[64], extra_str[32];
struct packet_conn_data *conn;
struct index_buf_pool *pool = &index_list[index].acl;
if (index >= MAX_INDEX) {
print_field("Invalid index (%d).", index);
return;
}
@@ -15043,22 +15049,24 @@ void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
data += HCI_ACL_HDR_SIZE;
size -= HCI_ACL_HDR_SIZE;
conn = packet_get_conn_data(handle);
if (conn && conn->type == 0x01 && index_list[index].le.total)
pool = &index_list[index].le;
if (!in && pool && pool->total)
- sprintf(handle_str, "Handle %d [%u/%u]", acl_handle(handle),
+ snprintf(handle_str, sizeof(handle_str),
+ "Handle %d [%u/%u]", acl_handle(handle),
++pool->tx, pool->total);
else
- sprintf(handle_str, "Handle %d", acl_handle(handle));
+ snprintf(handle_str, sizeof(handle_str), "Handle %d",
+ acl_handle(handle));
- handle_str_append_addr(handle_str, conn);
+ handle_str_append_addr(handle_str, sizeof(handle_str), conn);
sprintf(extra_str, "flags 0x%2.2x dlen %d", flags, dlen);
if (conn)
sprintf(label, "%s", conn_type_str(conn->type));
else
sprintf(label, "ACL");
@@ -15085,20 +15093,20 @@ void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
packet_set_context(NULL, 0);
}
void packet_hci_scodata(struct timeval *tv, struct ucred *cred, uint16_t index,
bool in, const void *data, uint16_t size)
{
const hci_sco_hdr *hdr = data;
- uint16_t handle = le16_to_cpu(hdr->handle);
- uint8_t flags = acl_flags(handle);
+ uint16_t handle;
+ uint8_t flags;
char label[8];
- char handle_str[42], extra_str[32];
+ char handle_str[64], extra_str[32];
struct packet_conn_data *conn;
if (index >= MAX_INDEX) {
print_field("Invalid index (%d).", index);
return;
}
index_list[index].frame++;
@@ -15109,27 +15117,32 @@ void packet_hci_scodata(struct timeval *tv, struct ucred *cred, uint16_t index,
"Malformed SCO Data RX packet", NULL, NULL);
else
print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
"Malformed SCO Data TX packet", NULL, NULL);
packet_hexdump(data, size);
return;
}
+ handle = le16_to_cpu(hdr->handle);
+ flags = acl_flags(handle);
+
data += HCI_SCO_HDR_SIZE;
size -= HCI_SCO_HDR_SIZE;
conn = packet_get_conn_data(handle);
if (index_list[index].sco.total && !in)
- sprintf(handle_str, "Handle %d [%u/%u]", acl_handle(handle),
+ snprintf(handle_str, sizeof(handle_str),
+ "Handle %d [%u/%u]", acl_handle(handle),
index_list[index].sco.total, index_list[index].sco.tx);
else
- sprintf(handle_str, "Handle %d", acl_handle(handle));
+ snprintf(handle_str, sizeof(handle_str), "Handle %d",
+ acl_handle(handle));
- handle_str_append_addr(handle_str, conn);
+ handle_str_append_addr(handle_str, sizeof(handle_str), conn);
sprintf(extra_str, "flags 0x%2.2x dlen %d", flags, hdr->dlen);
if (conn)
sprintf(label, "%s", conn_type_str(conn->type));
else
sprintf(label, "SCO");
@@ -15224,22 +15237,23 @@ void packet_hci_isodata(struct timeval *tv, struct ucred *cred, uint16_t index,
}
conn = packet_get_conn_data(handle);
if (in && have_hdr && conn)
packet_loss_add(&conn->rx_loss, sn, sflags);
if (!in && pool->total)
- sprintf(handle_str, "Handle %d [%u/%u]%s",
+ snprintf(handle_str, sizeof(handle_str), "Handle %d [%u/%u]%s",
acl_handle(handle), ++pool->tx, pool->total, sn_str);
else
- sprintf(handle_str, "Handle %u%s", acl_handle(handle), sn_str);
+ snprintf(handle_str, sizeof(handle_str), "Handle %u%s",
+ acl_handle(handle), sn_str);
- handle_str_append_addr(handle_str, conn);
+ handle_str_append_addr(handle_str, sizeof(handle_str), conn);
sprintf(extra_str, "flags 0x%2.2x dlen %u%s%s", flags, dlen, slen_str,
ts_str);
if (conn)
sprintf(label, "%s", conn_type_str(conn->type));
else
sprintf(label, "ISO");
---
base-commit: b0bf49270d445da0852851f393cd67a0a1e5f05e
change-id: 20260924-fix_sco-059769173ee0
Best regards,
--
Zijun Hu <zijun.hu@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [bluez,v2] monitor: Fix stack-buffer-overflow in packet_hci_scodata()
2026-10-27 12:03 [PATCH bluez v2] monitor: Fix stack-buffer-overflow in packet_hci_scodata() Zijun Hu
@ 2026-10-27 15:26 ` bluez.test.bot
0 siblings, 0 replies; 2+ messages in thread
From: bluez.test.bot @ 2026-10-27 15:26 UTC (permalink / raw)
To: linux-bluetooth, zijun.hu
[-- Attachment #1: Type: text/plain, Size: 3829 bytes --]
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/series/1194542/
---Test result---
Test Summary:
CheckPatch PASS 0.42 seconds
GitLint FAIL 0.41 seconds
BuildEll PASS 19.73 seconds
BluezMake PASS 300.39 seconds
MakeCheck PASS 0.83 seconds
MakeDistcheck PASS 142.66 seconds
CheckValgrind PASS 147.43 seconds
CheckSmatch WARNING 293.18 seconds
bluezmakeextell PASS 93.97 seconds
TestFunctional FAIL 1182.64 seconds
IncrementalBuild PASS 296.50 seconds
ScanBuild PASS 1149.70 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[bluez,v2] monitor: Fix stack-buffer-overflow in packet_hci_scodata()
50: B2 Line has trailing whitespace: "-- "
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
monitor/packet.c:2289:26: warning: Variable length array is used.monitor/packet.c: note: in included file:monitor/bt.h:3925:52: warning: array of flexible structuresmonitor/bt.h:3913:40: warning: array of flexible structures
##############################
Test: TestFunctional - FAIL
Desc: Run test-functional
Output:
FAIL functional.test_bluetoothctl::test_bluetoothctl_pair_bredr[hosts15-vm2]: pytest_bluezenv.rpc.RemoteError: <pexpect.popen_spawn.PopenSpawn object at 0x7f2baea7e510>
searcher: searcher_re:
0: re.compile(b'Changing pairable on succeeded')
<pexpect.popen_spawn.PopenSpawn object at 0x7f2baea7e510>
searcher: searcher_re:
0: re.compile(b'Changing pairable on succeeded')
Remote traceback:
Traceback (most recent call last):
File "/usr/local/lib/python3.14/dist-packages/pytest_bluezenv/../pytest_bluezenv/rpc.py", line 102, in server_stream
result = method(*msg["a"], **msg["kw"])
File "/usr/local/lib/python3.14/dist-packages/pytest_bluezenv/../pytest_bluezenv/env.py", line 256, in call_plugin
return getattr(self.plugins[name], method)(*a, **kw)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
File "/usr/local/lib/python3.14/dist-packages/pytest_bluezenv/../pytest_bluezenv/host_plugins.py", line 502, in expect
ret = self.ctl.expect(*a, **kw)
File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 354, in expect
return self.expect_list(compiled_pattern_list,
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
timeout, searchwindowsize, async_)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 383, in expect_list
return exp.expect_loop(timeout)
~~~~~~~~~~~~~~~^^^^^^^^^
File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 181, in expect_loop
return self.timeout(e)
~~~~~~~~~~~~^^^
File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 144, in timeout
raise exc
pexpect.exceptions.TIMEOUT: <pexpect.popen_spawn.PopenSpawn object at 0x7f2baea7e510>
searcher: searcher_re:
0: re.compile(b'Changing pairable on succeeded')
<pexpect.popen_spawn.PopenSpawn object at 0x7f2baea7e510>
searcher: searcher_re:
0: re.compile(b'Changing pairable on succeeded')
https://github.com/bluez/bluez/pull/2720
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-27 15:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 12:03 [PATCH bluez v2] monitor: Fix stack-buffer-overflow in packet_hci_scodata() Zijun Hu
2026-10-27 15:26 ` [bluez,v2] " bluez.test.bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox