Skip to contentSkip to conversation

FAILED: patch "[PATCH] Bluetooth: HIDP: reject frames without a transaction header" failed to apply to 5.10-stable tree

PATCH New [email protected] 2 participants 3 messages Aug 5 – Aug 10

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 47778d2c2087b5d192398f6fddf692d16a5431cf
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026080515-broadness-backlash-dde1@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 47778d2c2087b5d192398f6fddf692d16a5431cf Mon Sep 17 00:00:00 2001
From: Sangho Lee <[email protected]>
Date: Thu, 23 Jul 2026 12:28:06 +0900
Subject: [PATCH] Bluetooth: HIDP: reject frames without a transaction header

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")
net/bluetooth/hidp/core.c +15 -10
index 0e24c5e2955e..194208d03d18 100644
@@ -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);
 }
 
maintainer
#1 lore raw

PATCH 1/2 skbuff: introduce skb_pull_data

From: Luiz Augusto von Dentz <[email protected]>

[ Upstream commit 13244cccc2b61ec715f0ac583d3037497004d4a5 ]

Like skb_pull but returns the original data pointer before pulling the data after performing a check against sbk->len.

This allows to change code that does "struct foo *p = (void *)skb->data;" which is hard to audit and error prone, to:

        p = skb_pull_data(skb, sizeof(*p));
        if (!p)
                return;

Which is both safer and cleaner.

Stable-dep-of: 47778d2c2087 ("Bluetooth: HIDP: reject frames without a transaction header")

---
 include/linux/skbuff.h |  2 ++
 net/core/skbuff.c      | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
include/linux/skbuff.h +2 -0
index 667de403b2ebb..01eb7bc4e84b0 100644
@@ -2317,6 +2317,8 @@ static inline void *skb_pull_inline(struct sk_buff *skb, unsigned int len)
 	return unlikely(len > skb->len) ? NULL : __skb_pull(skb, len);
 }
 
+void *skb_pull_data(struct sk_buff *skb, size_t len);
+
 void *__pskb_pull_tail(struct sk_buff *skb, int delta);
 
 static inline void *__pskb_pull(struct sk_buff *skb, unsigned int len)
net/core/skbuff.c +24 -0
index 90a63c356affb..83bf2e5bed2b2 100644
@@ -1976,6 +1976,30 @@ void *skb_pull(struct sk_buff *skb, unsigned int len)
 }
 EXPORT_SYMBOL(skb_pull);
 
+/**
+ *	skb_pull_data - remove data from the start of a buffer returning its
+ *	original position.
+ *	@skb: buffer to use
+ *	@len: amount of data to remove
+ *
+ *	This function removes data from the start of a buffer, returning
+ *	the memory to the headroom. A pointer to the original data in the buffer
+ *	is returned after checking if there is enough data to pull. Once the
+ *	data has been pulled future pushes will overwrite the old data.
+ */
+void *skb_pull_data(struct sk_buff *skb, size_t len)
+{
+	void *data = skb->data;
+
+	if (skb->len < len)
+		return NULL;
+
+	skb_pull(skb, len);
+
+	return data;
+}
+EXPORT_SYMBOL(skb_pull_data);
+
 /**
  *	skb_trim - remove end from a buffer
  *	@skb: buffer to alter
Signature · 1 line
2.53.0
maintainer
#2 lore raw

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

From: Sangho Lee <[email protected]>

[ Upstream commit 47778d2c2087b5d192398f6fddf692d16a5431cf ]

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")
---
 net/bluetooth/hidp/core.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
net/bluetooth/hidp/core.c +15 -10
index 068c3c2505170..8e9e07684ff78 100644
@@ -550,16 +550,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 %d", 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:
@@ -580,6 +582,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
 		break;
 	}
 
+free:
 	if (free_skb)
 		kfree_skb(skb);
 }
@@ -587,14 +590,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 %d", 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)
@@ -606,9 +610,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);
 }
 
Signature · 1 line
2.53.0