Netdev List
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Xia Yan <yanxia@cmss.chinamobile.com>
Cc: netdev@vger.kernel.org, oe-linux-nfc@lists.linux.dev,
	David Heidelberg <david@ixit.cz>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Thierry Escande <thierry.escande@linux.intel.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Samuel Ortiz <sameo@rivosinc.com>,
	linux-kernel@vger.kernel.org, yana06070@163.com
Subject: Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery
Date: Fri, 13 Nov 2026 10:00:07 +0000	[thread overview]
Message-ID: <20261113100007.GI18859@horms.kernel.org> (raw)
In-Reply-To: <20261109031832.37642-1-yanxia@cmss.chinamobile.com>

On Mon, Nov 09, 2026 at 03:18:32AM +0000, Xia Yan wrote:
> digital_in_recv_sens_res() allocates a target for an NFC-A response.
> For a Type 1 Tag, it calls digital_target_found() synchronously.
> nfc_targets_found() copies the target into dev->targets.
> 
> On success, rc is zero and the callback skips kfree(target).
> The original allocation is lost when the callback returns.
> Each completed Type 1 Tag discovery leaks one struct nfc_target.
> 
> Free the target after digital_target_found() returns. Clear the pointer
> to avoid freeing it again on the error path. Keep the target for other
> tag types, whose discovery continues asynchronously.
> 
> Found during a code review of NFC-A target ownership.
> 
> Fixes: 2c66daecc409 ("NFC Digital: Add NFC-A technology support")
> Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
> ---
>  net/nfc/digital_technology.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
> index ae6487c10a25..2d52e390cc67 100644
> --- a/net/nfc/digital_technology.c
> +++ b/net/nfc/digital_technology.c
> @@ -510,10 +510,13 @@ static void digital_in_recv_sens_res(struct nfc_digital_dev *ddev, void *arg,
>  		goto exit;
>  	}
>  
> -	if (DIGITAL_SENS_RES_IS_T1T(target->sens_res))
> +	if (DIGITAL_SENS_RES_IS_T1T(target->sens_res)) {
>  		rc = digital_target_found(ddev, target, NFC_PROTO_JEWEL);
> -	else
> +		kfree(target);
> +		target = NULL;
> +	} else {
>  		rc = digital_in_send_sdd_req(ddev, target);
> +	}
>  
>  exit:
>  	dev_kfree_skb(resp);

Thanks Yan,

I agree this is a bug and that you patch correctly addresses it.
However, I wonder if it would be cleaner to confine cleanup to
the exit path, centralising the kfree logic.

Something like this (compile tested only!):

diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
index ae6487c10a25..94880172f016 100644
--- a/net/nfc/digital_technology.c
+++ b/net/nfc/digital_technology.c
@@ -483,6 +483,7 @@ static void digital_in_recv_sens_res(struct nfc_digital_dev *ddev, void *arg,
 				     struct sk_buff *resp)
 {
 	struct nfc_target *target = NULL;
+	bool res_is_t1t = false;
 	int rc;
 
 	if (IS_ERR(resp)) {
@@ -510,7 +511,8 @@ static void digital_in_recv_sens_res(struct nfc_digital_dev *ddev, void *arg,
 		goto exit;
 	}
 
-	if (DIGITAL_SENS_RES_IS_T1T(target->sens_res))
+	res_is_t1t = DIGITAL_SENS_RES_IS_T1T(target->sens_res);
+	if (res_is_t1t)
 		rc = digital_target_found(ddev, target, NFC_PROTO_JEWEL);
 	else
 		rc = digital_in_send_sdd_req(ddev, target);
@@ -518,10 +520,11 @@ static void digital_in_recv_sens_res(struct nfc_digital_dev *ddev, void *arg,
 exit:
 	dev_kfree_skb(resp);
 
-	if (rc) {
+	if (rc || res_is_t1t)
 		kfree(target);
+
+	if (rc)
 		digital_poll_next_tech(ddev);
-	}
 }
 
 int digital_in_send_sens_req(struct nfc_digital_dev *ddev, u8 rf_tech)

      parent reply	other threads:[~2026-11-13 10:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09  3:18 [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery Xia Yan
2026-11-09  3:21 ` netdev-bot+sinfo
2026-11-09  3:52   ` Xia Yan
2026-11-13 10:08     ` Simon Horman
2026-11-13 10:00 ` Simon Horman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261113100007.GI18859@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=david@ixit.cz \
    --cc=edumazet@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oe-linux-nfc@lists.linux.dev \
    --cc=pabeni@redhat.com \
    --cc=sameo@linux.intel.com \
    --cc=sameo@rivosinc.com \
    --cc=thierry.escande@linux.intel.com \
    --cc=yana06070@163.com \
    --cc=yanxia@cmss.chinamobile.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox