[PATCH v2] nfc: hci: fix OOB heap read on short HCP frames

Ashutosh Desai posted 1 patch 2 months ago
net/nfc/hci/core.c | 9 +++++++++
net/nfc/nci/hci.c  | 9 +++++++++
2 files changed, 18 insertions(+)
[PATCH v2] nfc: hci: fix OOB heap read on short HCP frames
Posted by Ashutosh Desai 2 months ago
Both nfc_hci_recv_from_llc() and nfc_hci_msg_rx_work() cast skb->data
to struct hcp_packet and read the message header byte without verifying
the data is present in the linear sk_buff area. The same issue exists in
the NCI HCI path via nci_hci_data_received_cb() and nci_hci_msg_rx_work().

The initial fix checked skb->len, but that counts bytes in non-linear
fragments too. skb->data only covers the linear head, so a fragmented
skb with len >= 2 but the payload in a fragment would still result in
an out-of-bounds read. Eric Dumazet pointed this out.

Switch to pskb_may_pull() which validates that the requested bytes are
available and pulls fragment data into the linear area if needed, which
is the correct approach here.

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
 net/nfc/hci/core.c | 9 +++++++++
 net/nfc/nci/hci.c  | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index 0d33c81a1..b8fe59f44 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -134,6 +134,10 @@ static void nfc_hci_msg_rx_work(struct work_struct *work)
 	u8 instruction;
 
 	while ((skb = skb_dequeue(&hdev->msg_rx_queue)) != NULL) {
+		if (!pskb_may_pull(skb, NFC_HCI_HCP_HEADER_LEN)) {
+			kfree_skb(skb);
+			continue;
+		}
 		pipe = skb->data[0];
 		skb_pull(skb, NFC_HCI_HCP_PACKET_HEADER_LEN);
 		message = (struct hcp_message *)skb->data;
@@ -904,6 +908,11 @@ static void nfc_hci_recv_from_llc(struct nfc_hci_dev *hdev, struct sk_buff *skb)
 	 * unblock waiting cmd context. Otherwise, enqueue to dispatch
 	 * in separate context where handler can also execute command.
 	 */
+	if (!pskb_may_pull(hcp_skb, NFC_HCI_HCP_HEADER_LEN)) {
+		kfree_skb(hcp_skb);
+		return;
+	}
+
 	packet = (struct hcp_packet *)hcp_skb->data;
 	type = HCP_MSG_GET_TYPE(packet->message.header);
 	if (type == NFC_HCI_HCP_RESPONSE) {
diff --git a/net/nfc/nci/hci.c b/net/nfc/nci/hci.c
index 40ae8e5a7..4243ca9b1 100644
--- a/net/nfc/nci/hci.c
+++ b/net/nfc/nci/hci.c
@@ -412,6 +412,10 @@ static void nci_hci_msg_rx_work(struct work_struct *work)
 
 	for (; (skb = skb_dequeue(&hdev->msg_rx_queue)); kcov_remote_stop()) {
 		kcov_remote_start_common(skb_get_kcov_handle(skb));
+		if (!pskb_may_pull(skb, NCI_HCI_HCP_HEADER_LEN)) {
+			kfree_skb(skb);
+			continue;
+		}
 		pipe = NCI_HCP_MSG_GET_PIPE(skb->data[0]);
 		skb_pull(skb, NCI_HCI_HCP_PACKET_HEADER_LEN);
 		message = (struct nci_hcp_message *)skb->data;
@@ -482,6 +486,11 @@ void nci_hci_data_received_cb(void *context,
 	 * unblock waiting cmd context. Otherwise, enqueue to dispatch
 	 * in separate context where handler can also execute command.
 	 */
+	if (!pskb_may_pull(hcp_skb, NCI_HCI_HCP_HEADER_LEN)) {
+		kfree_skb(hcp_skb);
+		return;
+	}
+
 	packet = (struct nci_hcp_packet *)hcp_skb->data;
 	type = NCI_HCP_MSG_GET_TYPE(packet->message.header);
 	if (type == NCI_HCI_HCP_RESPONSE) {
-- 
2.34.1
Re: [PATCH v2] nfc: hci: fix OOB heap read on short HCP frames
Posted by Jakub Kicinski 2 months ago
On Thu,  9 Apr 2026 15:08:25 +0000 Ashutosh Desai wrote:
> Suggested-by: Eric Dumazet <edumazet@google.com>

As Eric mentioned elsewhere - he did not suggest any of this,
merely reviewed your submission.

> +++ b/net/nfc/hci/core.c
> @@ -134,6 +134,10 @@ static void nfc_hci_msg_rx_work(struct work_struct *work)
>  	u8 instruction;
>  
>  	while ((skb = skb_dequeue(&hdev->msg_rx_queue)) != NULL) {
> +		if (!pskb_may_pull(skb, NFC_HCI_HCP_HEADER_LEN)) {
> +			kfree_skb(skb);
> +			continue;

How did a broken packet get enqueued in the first place?
Re: [PATCH v2] nfc: hci: fix OOB heap read on short HCP frames
Posted by Ashutosh Desai 2 months ago
On Sun, 12 Apr 2026 13:42:18 -0700 Jakub Kicinski wrote:
> As Eric mentioned elsewhere - he did not suggest any of this,
> merely reviewed your submission.

Agree, that tag was incorrect on my part. Will remove it in the
next version.

> How did a broken packet get enqueued in the first place?

You are right to point that out. nfc_hci_recv_from_llc() already
gates the queue with pskb_may_pull(), so a short skb cannot reach
nfc_hci_msg_rx_work() to begin with. The same holds for the nci
path. Those two checks are redundant and will be dropped in v3.
[PATCH v3] nfc: hci: fix out-of-bounds read in HCP header parsing
Posted by Ashutosh Desai 2 months ago
nfc_hci_recv_from_llc() and nci_hci_data_received_cb() cast skb->data
to struct hcp_packet and read the message header byte without checking
that enough data is present in the linear sk_buff area. A malicious NFC
peer can send a 1-byte HCP frame that passes through the SHDLC layer
and reaches these functions, causing an out-of-bounds heap read.

Fix this by adding pskb_may_pull() before each cast to ensure the full
2-byte HCP header is pulled into the linear area before it is accessed.

Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
v3: drop redundant pskb_may_pull checks from msg_rx_work functions,
    remove incorrect Suggested-by tag
v2: switch skb->len check to pskb_may_pull

 net/nfc/hci/core.c | 5 +++++
 net/nfc/nci/hci.c  | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index 0d33c81a1..cd9cf6c94 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -904,6 +904,11 @@ static void nfc_hci_recv_from_llc(struct nfc_hci_dev *hdev, struct sk_buff *skb)
 	 * unblock waiting cmd context. Otherwise, enqueue to dispatch
 	 * in separate context where handler can also execute command.
 	 */
+	if (!pskb_may_pull(hcp_skb, NFC_HCI_HCP_HEADER_LEN)) {
+		kfree_skb(hcp_skb);
+		return;
+	}
+
 	packet = (struct hcp_packet *)hcp_skb->data;
 	type = HCP_MSG_GET_TYPE(packet->message.header);
 	if (type == NFC_HCI_HCP_RESPONSE) {
diff --git a/net/nfc/nci/hci.c b/net/nfc/nci/hci.c
index 40ae8e5a7..6e633da25 100644
--- a/net/nfc/nci/hci.c
+++ b/net/nfc/nci/hci.c
@@ -482,6 +482,11 @@ void nci_hci_data_received_cb(void *context,
 	 * unblock waiting cmd context. Otherwise, enqueue to dispatch
 	 * in separate context where handler can also execute command.
 	 */
+	if (!pskb_may_pull(hcp_skb, NCI_HCI_HCP_HEADER_LEN)) {
+		kfree_skb(hcp_skb);
+		return;
+	}
+
 	packet = (struct nci_hcp_packet *)hcp_skb->data;
 	type = NCI_HCP_MSG_GET_TYPE(packet->message.header);
 	if (type == NCI_HCI_HCP_RESPONSE) {
-- 
2.34.1
Re: [PATCH v3] nfc: hci: fix out-of-bounds read in HCP header parsing
Posted by Jakub Kicinski 2 months ago
On Mon, 13 Apr 2026 02:43:29 +0000 Ashutosh Desai wrote:
> nfc_hci_recv_from_llc() and nci_hci_data_received_cb() cast skb->data
> to struct hcp_packet and read the message header byte without checking
> that enough data is present in the linear sk_buff area. A malicious NFC
> peer can send a 1-byte HCP frame that passes through the SHDLC layer
> and reaches these functions, causing an out-of-bounds heap read.
> 
> Fix this by adding pskb_may_pull() before each cast to ensure the full
> 2-byte HCP header is pulled into the linear area before it is accessed.

This is missing a Fixes tag.
Also please do not post new revision of a patch in response to the
previous one
-- 
pw-bot:  cr
pv-bot: fixes
pv-bot: thread