* [PATCH bluez v2] monitor: Fix stack-buffer-overflow in packet_hci_scodata()
@ 2026-10-27 12:03 Zijun Hu
0 siblings, 0 replies; only message 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 [flat|nested] only message in thread
only message in thread, other threads:[~2026-10-27 12:04 UTC | newest]
Thread overview: (only message) (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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®