[PATCH net 0/3] nfc: fix skb leaks in LLC nop and in data exchange error paths

[PATCH net 0/3] nfc: fix skb leaks in LLC nop and in data exchange error paths

[Joas Antonio dos Santos]

Three skb leaks in the NFC core, found by LLM-assisted code review while
auditing the HCI drivers that need real hardware.

Patch 1: with the LLC "nop" engine (used by the MEI attached pn544 and
microread) every frame sent to the controller is leaked.  Confirmed with
a test-only module that registers st21nfca with LLC nop on a virtual
phy and a userspace controller emulator: 20 device up/down cycles leak
598 skbuff_head_cache objects without the patch and none with it.

Patches 2 and 3: the HCI core and the HCI drivers (pn544, st21nfca,
microread) hand the response skb to the data exchange callback together
with an error when the controller reports a failed exchange, and NCI
does the same for a frame interface status error, but the raw socket
and LLCP callbacks only free the skb on success.  These two are compile
tested only (W=1); the leak follows from the code: __nfc_hci_cmd_completion()
gives the skb to the callback and never frees it itself, and
nci_add_rx_data_frag() passes it on with the error.


Joas Antonio dos Santos (3):
  nfc: hci: free the skb sent through LLC nop
  nfc: rawsock: free the response skb when the data exchange fails
  nfc: llcp: free the response skb when the data exchange fails

 net/nfc/hci/llc_nop.c | 11 ++++++++++-
 net/nfc/llcp_core.c   |  2 ++
 net/nfc/rawsock.c     |  4 ++--
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH net 2/3] nfc: rawsock: free the response skb when the data exchange fails

[Joas Antonio dos Santos]

rawsock_data_exchange_complete() only frees the skb it is given when
the exchange succeeded.  But the skb can carry the response even when
err is set: the HCI core passes it on with the error derived from the
controller's result code (__nfc_hci_cmd_completion() gives it to the
callback and does not free it, and hci_transceive_cb() and the pn544,
st21nfca and microread data exchange callbacks forward it as is), and
NCI does the same for a frame interface data packet with a non-zero
status byte.  So each failed exchange on a raw socket leaks an skb, and
a controller or tag that keeps failing exchanges leaks memory without
bound.

This was found by LLM-assisted code review.

Free the skb on the error path too; it is NULL when the error does not
come with a response (digital, timeouts, device close), and
kfree_skb() handles that.

Fixes: 23b7869c0fd0 ("NFC: add the NFC socket raw protocol")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
 net/nfc/rawsock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index f7d7a599f..74ef3906c 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -156,8 +156,9 @@ static void rawsock_data_exchange_complete(void *context, struct sk_buff *skb,
 
 	pr_debug("sk=%p err=%d\n", sk, err);
 
+	/* skb may carry the response even when the exchange failed */
 	if (err)
-		goto error;
+		goto error_skb;
 
 	err = rawsock_add_header(skb);
 	if (err)
@@ -180,7 +181,6 @@ static void rawsock_data_exchange_complete(void *context, struct sk_buff *skb,
 error_skb:
 	kfree_skb(skb);
 
-error:
 	rawsock_report_error(sk, err);
 	sock_put(sk);
 }
-- 
2.43.0

[PATCH net 1/3] nfc: hci: free the skb sent through LLC nop

[Joas Antonio dos Santos]

nfc_hci_msg_tx_work() hands each frame to nfc_llc_xmit_from_hci() and
only frees it if that fails: on success the LLC engine owns it.  SHDLC
queues the frame and frees it once it is acked, but LLC nop passes it
straight to the driver's xmit, and the phy write operations of its
users (the MEI phy of pn544 and microread) copy the data out and leave
the skb alone, so every frame sent to the controller is leaked.

This was found by LLM-assisted code review and confirmed with a
test-only module that registers st21nfca with LLC nop on a virtual phy
and a userspace controller emulator: 20 device up/down cycles leaked
598 skbuff_head_cache objects, and none with this patch.

Free the frame in llc_nop_xmit_from_hci() once the driver has sent it.

Fixes: 8af00d48dc92 ("NFC: Add a nop (passthrough) llc module to llc core")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
 net/nfc/hci/llc_nop.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/nfc/hci/llc_nop.c b/net/nfc/hci/llc_nop.c
index f9c826a13..1603e435b 100644
--- a/net/nfc/hci/llc_nop.c
+++ b/net/nfc/hci/llc_nop.c
@@ -67,8 +67,17 @@ static void llc_nop_rcv_from_drv(struct nfc_llc *llc, struct sk_buff *skb)
 static int llc_nop_xmit_from_hci(struct nfc_llc *llc, struct sk_buff *skb)
 {
 	struct llc_nop *llc_nop = nfc_llc_get_data(llc);
+	int r;
 
-	return llc_nop->xmit_to_drv(llc_nop->hdev, skb);
+	/* The driver does not take ownership of the frame (the phy write
+	 * operations copy it out), and nfc_hci_msg_tx_work() only frees it
+	 * on error, so it is ours to free once it has been sent.
+	 */
+	r = llc_nop->xmit_to_drv(llc_nop->hdev, skb);
+	if (r == 0)
+		consume_skb(skb);
+
+	return r;
 }
 
 static const struct nfc_llc_ops llc_nop_ops = {
-- 
2.43.0

[PATCH net 3/3] nfc: llcp: free the response skb when the data exchange fails

[Joas Antonio dos Santos]

nfc_llcp_recv() is the data exchange callback for LLCP PDUs sent in
initiator mode, and it returns without freeing the skb when err is set.
Like for raw sockets, the skb can carry the response even when err is
set: the HCI core passes it on with the error derived from the
controller's result code, and NCI does the same for a frame interface
data packet with a non-zero status byte.  So each failed exchange leaks
an skb.

This was found by LLM-assisted code review.

Free the skb on the error path; it is NULL when the error does not come
with a response, and kfree_skb() handles that.

Fixes: d646960f7986 ("NFC: Initial LLCP support")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
 net/nfc/llcp_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 74bf81700..f0632a0b9 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1638,6 +1638,8 @@ void nfc_llcp_recv(void *data, struct sk_buff *skb, int err)
 
 	if (err < 0) {
 		pr_err("LLCP PDU receive err %d\n", err);
+		/* skb may carry the response even when the exchange failed */
+		kfree_skb(skb);
 		return;
 	}
 
-- 
2.43.0