Re: [PATCH v3 1/1] net: nfc: fix use-after-free in nfc_get_local_general_bytes

On 12/09/2026 12:16, Simon Horman wrote:

On Wed, Sep 09, 2026 at 01:19:24PM +0800, Ren Wei wrote:

quoted

From: Luxiao Xu <redacted>

Commit 6709d4b7bc2e ("net: nfc: Fix use-after-free caused by
nfc_llcp_find_local") attempted to fix a use-after-free (UAF) issue by
invoking nfc_llcp_local_put(local) after accessing local->gb. However,
if the reference count drops to zero, local is freed immediately,
leading to a use-after-free when callers access the returned pointer.
Alternative approaches using dynamic allocation (e.g. kmemdup) introduced
memory leaks because callers consistently treat the returned pointer as
borrowed memory.

Fix this properly by refactoring nfc_llcp_general_bytes() and
nfc_get_local_general_bytes() to accept a caller-provided output buffer
(out_gb) and its maximum length (gb_max_len). The general bytes are
safely copied into out_gb before calling nfc_llcp_local_put(local),
ensuring safe lifetime management without ownership transfer complications.

Update all callers across drivers (microread, pn533, pn544, st21nfca,
digital_dep, and nci) to provide their own destination buffers and pass
them to nfc_get_local_general_bytes().

Fixes: 6709d4b7bc2e ("net: nfc: Fix use-after-free caused by nfc_llcp_find_local")
Cc: stable@vger.kernel.org
Reported-by: Vega <redacted>
Assisted-by: LLM
Signed-off-by: Luxiao Xu <redacted>
Signed-off-by: Ren Wei <redacted>
---

[...]

As far as I can see the return value of nfc_llcp_general_bytes() is now
ignored. And for a bug fix the approach you have taken looks good,
as the interface provided by nfc_llcp_general_bytes() is only slightly
modified.

But, FWIIW, I think it would be cleaner if nfc_llcp_general_bytes()
returned the length, or 0 on error. And the general_bytes_len parameter was
passed by value rather than reference.

Something like this:

size_t nfc_llcp_general_bytes(struct nfc_dev *dev, u8 *out_gb,
			      size_t gb_max_len, size_t general_bytes_len)
{
	...

	if (error_condition)
		return 0;

	...

	return general_bytes_len;
}

Perhaps that approach could be considered as a follow-up.

Yes, that would be really great!

Thanks
David