* [PATCH 1/2] Bluetooth: HIDP: reject frames without a transaction header
2026-07-23 3:28 [PATCH 0/2] Bluetooth: HIDP: validate short receive frames Sangho Lee
@ 2026-07-23 3:28 ` Sangho Lee
2026-07-23 4:58 ` Bluetooth: HIDP: validate short receive frames bluez.test.bot
2026-07-23 3:28 ` [PATCH 2/2] Bluetooth: HIDP: validate numbered report payloads Sangho Lee
1 sibling, 1 reply; 4+ messages in thread
From: Sangho Lee @ 2026-07-23 3:28 UTC (permalink / raw)
To: luiz.dentz, marcel, linux-bluetooth
Cc: jikos, alan, padovan, linux-kernel, stable, kudo3228
hidp_recv_ctrl_frame() and hidp_recv_intr_frame() read skb->data[0]
before checking that the L2CAP SDU contains a transaction header. A
connected HIDP peer can send an empty basic-mode SDU and make both paths
use an uninitialized byte from skb tailroom.
KMSAN reports the use in hidp_session_run(), with the uninitialized value
originating in __alloc_skb() through vhci_write(). The control path
produces two reports and the interrupt path produces one.
The byte can also be controlled by a malformed lower-layer packet. If an
HCI ACL packet contains an L2CAP PDU with a declared zero-length payload
followed by an extra 0x15 byte, l2cap_recv_acldata() reduces skb->len to
the declared PDU length before dispatch. The current HIDP path nevertheless
consumes the extra byte as HIDP_TRANS_HID_CONTROL |
HIDP_CTRL_VIRTUAL_CABLE_UNPLUG and terminates the HIDP session. With this
change, the same packet is discarded and a subsequent feature report
request succeeds.
Pull the transaction header with skb_pull_data() and discard frames that
do not contain it.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Sangho Lee <kudo3228@gmail.com>
---
net/bluetooth/hidp/core.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0e24c5e2955e..194208d03d18 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
static void hidp_recv_ctrl_frame(struct hidp_session *session,
struct sk_buff *skb)
{
- unsigned char hdr, type, param;
+ unsigned char type, param;
+ u8 *hdr;
int free_skb = 1;
BT_DBG("session %p skb %p len %u", session, skb, skb->len);
- hdr = skb->data[0];
- skb_pull(skb, 1);
+ hdr = skb_pull_data(skb, 1);
+ if (!hdr)
+ goto free;
- type = hdr & HIDP_HEADER_TRANS_MASK;
- param = hdr & HIDP_HEADER_PARAM_MASK;
+ type = *hdr & HIDP_HEADER_TRANS_MASK;
+ param = *hdr & HIDP_HEADER_PARAM_MASK;
switch (type) {
case HIDP_TRANS_HANDSHAKE:
@@ -590,6 +592,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
break;
}
+free:
if (free_skb)
kfree_skb(skb);
}
@@ -597,14 +600,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
static void hidp_recv_intr_frame(struct hidp_session *session,
struct sk_buff *skb)
{
- unsigned char hdr;
+ u8 *hdr;
BT_DBG("session %p skb %p len %u", session, skb, skb->len);
- hdr = skb->data[0];
- skb_pull(skb, 1);
+ hdr = skb_pull_data(skb, 1);
+ if (!hdr)
+ goto free;
- if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
+ if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
hidp_set_timer(session);
if (session->input)
@@ -616,9 +620,10 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
BT_DBG("report len %d", skb->len);
}
} else {
- BT_DBG("Unsupported protocol header 0x%02x", hdr);
+ BT_DBG("Unsupported protocol header 0x%02x", *hdr);
}
+free:
kfree_skb(skb);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] Bluetooth: HIDP: validate numbered report payloads
2026-07-23 3:28 [PATCH 0/2] Bluetooth: HIDP: validate short receive frames Sangho Lee
2026-07-23 3:28 ` [PATCH 1/2] Bluetooth: HIDP: reject frames without a transaction header Sangho Lee
@ 2026-07-23 3:28 ` Sangho Lee
1 sibling, 0 replies; 4+ messages in thread
From: Sangho Lee @ 2026-07-23 3:28 UTC (permalink / raw)
To: luiz.dentz, marcel, linux-bluetooth
Cc: jikos, alan, padovan, linux-kernel, stable, kudo3228
When hidp_get_raw_report() waits for a numbered report,
hidp_process_data() compares the expected report number with skb->data[0].
A connected HIDP peer can reply with only a DATA transaction header,
leaving the skb empty after the header is removed.
KMSAN reports an uninitialized-value use in hidp_session_run(), with the
value originating in __alloc_skb() through vhci_write(). The transaction
header checks remove the empty-frame reports, but this report remains until
the payload check is added.
The comparison can also consume a peer-controlled byte beyond the declared
L2CAP PDU. A DATA | FEATURE response followed by an extra 0x01 byte made
the current code accept that byte as report ID 1 and complete
HIDIOCGFEATURE with a zero-byte result. With this change the malformed
response is rejected with -EIO, while a subsequent valid response still
succeeds.
Require a payload byte before comparing a numbered report ID. Unnumbered
reports continue to accept an empty payload.
Fixes: 0ff1731a1ae5 ("HID: bt: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE")
Cc: stable@vger.kernel.org
Signed-off-by: Sangho Lee <kudo3228@gmail.com>
---
net/bluetooth/hidp/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 194208d03d18..f5bdf9f1ca63 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -543,9 +543,10 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
}
if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags) &&
- param == session->waiting_report_type) {
+ param == session->waiting_report_type) {
if (session->waiting_report_number < 0 ||
- session->waiting_report_number == skb->data[0]) {
+ (skb->len &&
+ session->waiting_report_number == skb->data[0])) {
/* hidp_get_raw_report() is waiting on this report. */
session->report_return = skb;
done_with_skb = 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread