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

[Simon Horman <horms@kernel.org>]

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)