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