[PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[Xia Yan]

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);

Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[Xia Yan]

Hi,

This was found by code inspection. I haven't triggered it at runtime.
nfc_targets_found() copies the target, but the T1T success path returns
without freeing the original allocation.

Thanks,
Xia Yan

Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[Simon Horman]

On Mon, Nov 09, 2026 at 03:52:34AM +0000, Xia Yan wrote:
> Hi,
> 
> This was found by code inspection. I haven't triggered it at runtime.
> nfc_targets_found() copies the target, but the T1T success path returns
> without freeing the original allocation.

Thanks. I agree net-next is the right target for this change.

Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[Simon Horman]

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)

Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery

[Simon Horman]

On Fri, Nov 13, 2026 at 10:00:07AM +0000, Simon Horman wrote:
> 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) {

Probably a comment is warranted here. Something like this:

	/* Release target on error.
	 * digital_target_found() does not take ownership of target,
	 * so also release target if res_is_t1t is true.
	 */

> +	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)