* [PATCH v5 net] nfc: hci: fix out-of-bounds read in HCP header parsing
@ 2026-04-16 5:15 Ashutosh Desai
2026-04-18 16:30 ` Simon Horman
0 siblings, 1 reply; 3+ messages in thread
From: Ashutosh Desai @ 2026-04-16 5:15 UTC (permalink / raw)
To: netdev
Cc: kuba, edumazet, davem, pabeni, horms, stable, linux-kernel,
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.
Fixes: 8b8d2e08bf0d ("NFC: HCI support")
Fixes: 11f54f228643 ("NFC: nci: Add HCI over NCI protocol support")
Cc: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
V4 -> V5: fix whitespace damage
V3 -> V4: add Fixes tags
V2 -> V3: drop redundant checks from nfc_hci_msg_rx_work/nci_hci_msg_rx_work;
remove incorrect Suggested-by tag
V1 -> V2: use pskb_may_pull() instead of skb->len check
v4: https://lore.kernel.org/netdev/177614425081.3600288.2536320552978506086@gmail.com/
v3: https://lore.kernel.org/netdev/20260413024329.3293075-1-ashutoshdesai993@gmail.com/
v2: https://lore.kernel.org/netdev/20260409150825.2217133-1-ashutoshdesai993@gmail.com/
v1: https://lore.kernel.org/netdev/20260408223113.2009304-1-ashutoshdesai993@gmail.com/
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 0d33c81a15fe..cd9cf6c94a50 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 40ae8e5a7ec7..6e633da257d1 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v5 net] nfc: hci: fix out-of-bounds read in HCP header parsing
2026-04-16 5:15 [PATCH v5 net] nfc: hci: fix out-of-bounds read in HCP header parsing Ashutosh Desai
@ 2026-04-18 16:30 ` Simon Horman
2026-04-21 8:06 ` Paolo Abeni
0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2026-04-18 16:30 UTC (permalink / raw)
To: Ashutosh Desai
Cc: netdev, kuba, edumazet, davem, pabeni, stable, linux-kernel
On Thu, Apr 16, 2026 at 05:15:22AM +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.
>
> Fixes: 8b8d2e08bf0d ("NFC: HCI support")
> Fixes: 11f54f228643 ("NFC: nci: Add HCI over NCI protocol support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---
> V4 -> V5: fix whitespace damage
> V3 -> V4: add Fixes tags
> V2 -> V3: drop redundant checks from nfc_hci_msg_rx_work/nci_hci_msg_rx_work;
> remove incorrect Suggested-by tag
> V1 -> V2: use pskb_may_pull() instead of skb->len check
>
> v4: https://lore.kernel.org/netdev/177614425081.3600288.2536320552978506086@gmail.com/
> v3: https://lore.kernel.org/netdev/20260413024329.3293075-1-ashutoshdesai993@gmail.com/
> v2: https://lore.kernel.org/netdev/20260409150825.2217133-1-ashutoshdesai993@gmail.com/
> v1: https://lore.kernel.org/netdev/20260408223113.2009304-1-ashutoshdesai993@gmail.com/
>
> net/nfc/hci/core.c | 5 +++++
> net/nfc/nci/hci.c | 5 +++++
> 2 files changed, 10 insertions(+)
Reviewed-by: Simon Horman <horms@kernel.org>
Review of this patch at Sashiko.dev flags a number of related problems in
this code. I believe none of them introduced by this patch. And that
they can all be treated as area for possible follow-up.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v5 net] nfc: hci: fix out-of-bounds read in HCP header parsing
2026-04-18 16:30 ` Simon Horman
@ 2026-04-21 8:06 ` Paolo Abeni
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Abeni @ 2026-04-21 8:06 UTC (permalink / raw)
To: Simon Horman, Ashutosh Desai
Cc: netdev, kuba, edumazet, davem, stable, linux-kernel
On 4/18/26 6:30 PM, Simon Horman wrote:
> On Thu, Apr 16, 2026 at 05:15:22AM +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.
>>
>> Fixes: 8b8d2e08bf0d ("NFC: HCI support")
>> Fixes: 11f54f228643 ("NFC: nci: Add HCI over NCI protocol support")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
>> ---
>> V4 -> V5: fix whitespace damage
>> V3 -> V4: add Fixes tags
>> V2 -> V3: drop redundant checks from nfc_hci_msg_rx_work/nci_hci_msg_rx_work;
>> remove incorrect Suggested-by tag
>> V1 -> V2: use pskb_may_pull() instead of skb->len check
>>
>> v4: https://lore.kernel.org/netdev/177614425081.3600288.2536320552978506086@gmail.com/
>> v3: https://lore.kernel.org/netdev/20260413024329.3293075-1-ashutoshdesai993@gmail.com/
>> v2: https://lore.kernel.org/netdev/20260409150825.2217133-1-ashutoshdesai993@gmail.com/
>> v1: https://lore.kernel.org/netdev/20260408223113.2009304-1-ashutoshdesai993@gmail.com/
>>
>> net/nfc/hci/core.c | 5 +++++
>> net/nfc/nci/hci.c | 5 +++++
>> 2 files changed, 10 insertions(+)
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> Review of this patch at Sashiko.dev flags a number of related problems in
> this code. I believe none of them introduced by this patch. And that
> they can all be treated as area for possible follow-up.
I agree that the issue reported by sashiko:
---
Does this patch fully resolve the out-of-bounds access?
Looking at the beginning of nfc_hci_recv_from_llc(), the code accesses
the packet header before checking if the skb has any data:
net/nfc/hci/core.c:nfc_hci_recv_from_llc() {
packet = (struct hcp_packet *)skb->data;
if ((packet->header & ~NFC_HCI_FRAGMENT) == 0) {
skb_queue_tail(&hdev->rx_hcp_frags, skb);
return;
}
...
}
If a maliciously crafted 0-byte payload is received, couldn't this
result in an out-of-bounds read of uninitialized memory?
Furthermore, if the fragmentation bit is clear and this 0-byte skb
is queued, when a subsequent final fragment arrives, the reassembly
loop calculates the message length:
net/nfc/hci/core.c:nfc_hci_recv_from_llc() {
...
skb_queue_walk(&hdev->rx_hcp_frags, frag_skb) {
msg_len += (frag_skb->len - NFC_HCI_HCP_PACKET_HEADER_LEN);
}
...
}
Since NFC_HCI_HCP_PACKET_HEADER_LEN is 1, wouldn't a 0-length fragment
cause this calculation to underflow to UINT_MAX, leading to an eventual
skb_over_panic() when skb_put_data() is called?
Would it be safer to add a pskb_may_pull(skb, 1) check at the very start
of the function before packet->header is accessed?
---
is pre-existing but it looks like the validation included here is almost
ineffective without addressing the above.
@Ashutosh, please include the additional validation in the next
revision, thanks!
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-21 8:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-16 5:15 [PATCH v5 net] nfc: hci: fix out-of-bounds read in HCP header parsing Ashutosh Desai
2026-04-18 16:30 ` Simon Horman
2026-04-21 8:06 ` Paolo Abeni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox