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

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

[Eric Dumazet]

On Wed, Apr 8, 2026 at 3:15 PM ashutosh desai
<ashutoshdesai993@gmail.com> wrote:
>
> Both nfc_hci_recv_from_llc() and nfc_hci_msg_rx_work() read byte 1 of
> an sk_buff (the HCP message header field) without first verifying the
> buffer contains at least NFC_HCI_HCP_HEADER_LEN (2) bytes.
>
> The SHDLC LLC layer only filters zero-length frames; a single-byte
> I-frame from a malicious NFC peer therefore reaches the HCI reassembly
> path where packet->message.header is read one byte past the valid data.
> The same issue is present in the NCI HCI implementation (nci/hci.c)
> via nci_hci_data_received_cb() and nci_hci_msg_rx_work().
>
> Add an explicit length check before accessing the message header at
> all four locations, freeing the skb on malformed input.
>
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---

Same answer: Testing skb->len alone is not enough.

skbs can have fragments, pskb_may_pull() or skb_linearize() would be needed.

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

[Ashutosh Desai]

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

[Jakub Kicinski]

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

[Ashutosh Desai]

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

[Ashutosh Desai]

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

[Jakub Kicinski]

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