* Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery
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:00 ` Simon Horman
1 sibling, 1 reply; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-11-09 3:21 UTC (permalink / raw)
To: Xia Yan
Cc: netdev, oe-linux-nfc, David Heidelberg, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Thierry Escande, Samuel Ortiz, Samuel Ortiz, linux-kernel,
yana06070
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] NFC: digital: fix target leak after Type 1 Tag discovery
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-13 10:00 ` Simon Horman
1 sibling, 0 replies; 5+ messages in thread
From: Simon Horman @ 2026-11-13 10:00 UTC (permalink / raw)
To: Xia Yan
Cc: netdev, oe-linux-nfc, David Heidelberg, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Thierry Escande,
Samuel Ortiz, Samuel Ortiz, linux-kernel, yana06070
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)
^ permalink raw reply related [flat|nested] 5+ messages in thread