[PATCH 0/2] Bluetooth: HIDP: validate short receive frames

[PATCH 0/2] Bluetooth: HIDP: validate short receive frames

[Sangho Lee]

The HIDP control and interrupt receive paths assume that every L2CAP SDU
contains a transaction header. The raw-report return path also assumes that
a numbered DATA response contains a report ID. Both assumptions allow a
connected peer to make HIDP consume data beyond the logical skb boundary.

The tests used two BlueZ 5.87 btvirt BR/EDR controllers, real L2CAP PSM
0x11/0x13 channels, HIDPCONNADD, and HIDIOCGFEATURE on bluetooth.git at
df541cd485ff. The series also applies without changes to bluetooth-next at
6f55ad8fb0ac.

On the unpatched KMSAN kernel, an empty control SDU produced two
uninitialized-value reports in hidp_session_run(), an empty interrupt SDU
produced one, and a DATA | FEATURE header without a report payload produced
one. Patch 1 removed only the first three reports; applying both patches
removed all four.

The tests also placed a controlled byte after the declared L2CAP PDU. A
trailing 0x15 after a zero-length control SDU was interpreted as virtual
cable unplug and terminated the unpatched HIDP session. The patched session
rejected the frame and completed a later feature report request. A trailing
report ID after a header-only DATA response made the unpatched raw-report
request complete with a zero-byte result; the patched kernel rejected it.

Finally, each path received 10,000 malformed responses on KASAN and UBSAN
kernels. No KASAN, UBSAN, Oops, or kernel BUG was observed. The KASAN runs
and all patched UBSAN runs accepted a subsequent valid feature report. The
unpatched UBSAN numbered-report run instead completed requests from queued
short responses, which further exposed the missing payload validation. No
information disclosure or code execution is claimed.

Sangho Lee (2):
  Bluetooth: HIDP: reject frames without a transaction header
  Bluetooth: HIDP: validate numbered report payloads

 net/bluetooth/hidp/core.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

-- 
2.43.0

[PATCH 1/2] Bluetooth: HIDP: reject frames without a transaction header

[Sangho Lee]

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

[PATCH 2/2] Bluetooth: HIDP: validate numbered report payloads

[Sangho Lee]

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

Re: [PATCH 0/2] Bluetooth: HIDP: validate short receive frames

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 23 Jul 2026 12:28:05 +0900 you wrote:
> The HIDP control and interrupt receive paths assume that every L2CAP SDU
> contains a transaction header. The raw-report return path also assumes that
> a numbered DATA response contains a report ID. Both assumptions allow a
> connected peer to make HIDP consume data beyond the logical skb boundary.
> 
> The tests used two BlueZ 5.87 btvirt BR/EDR controllers, real L2CAP PSM
> 0x11/0x13 channels, HIDPCONNADD, and HIDIOCGFEATURE on bluetooth.git at
> df541cd485ff. The series also applies without changes to bluetooth-next at
> 6f55ad8fb0ac.
> 
> [...]

Here is the summary with links:
  - [1/2] Bluetooth: HIDP: reject frames without a transaction header
    https://git.kernel.org/bluetooth/bluetooth-next/c/678c435909f2
  - [2/2] Bluetooth: HIDP: validate numbered report payloads
    https://git.kernel.org/bluetooth/bluetooth-next/c/883532b7d4eb

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