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

[David Heidelberg <david@ixit.cz>]

On 12/09/2026 12:16, Simon Horman wrote:
> On Wed, Sep 09, 2026 at 01:19:24PM +0800, Ren Wei wrote:
>> From: Luxiao Xu <rakukuip@gmail.com>
>>
>> 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 <vega@nebusec.ai>
>> Assisted-by: LLM
>> Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
>> Signed-off-by: Ren Wei <weir@nebusec.ai>
>> ---

[...]

> 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