Re: [PATCH 1/1] nfc: llcp: Pass caller buffer to nfc_llcp_general_bytes to fix UAF and memory leaks

Simon Horman <[email protected]>
Newsgroups dev.linux.lists.oe-linux-nfc,org.kernel.vger.netdev
Message-ID <[email protected]>
On Sat, Aug 08, 2026 at 12:28:51AM +0800, Ren Wei wrote:
> From: Luxiao Xu <[email protected]>
> 
> 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 dropped to zero, local was freed prematurely,
> leading to a Use-After-Free when the returned pointer was accessed.
> Alternative approaches using dynamic allocation (such as kmemdup) introduced
> severe memory leaks and state inconsistency because callers consistently
> treated 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 allocate local stack buffers of size
> NFC_MAX_GT_LEN and pass them to nfc_get_local_general_bytes().
> 
> Fixes: 6709d4b7bc2e ("net: nfc: Fix use-after-free caused by nfc_llcp_find_local")
> Cc: [email protected]
> Reported-by: Vega <[email protected]>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Luxiao Xu <[email protected]>
> Signed-off-by: Ren Wei <[email protected]>

Hi Ren,

Thanks for your patch.
I've provided some minor feedback below.

> diff --git a/drivers/nfc/microread/microread.c b/drivers/nfc/microread/microread.c
> index 4149c5d735bd..0f0a03da9ff4 100644
> --- a/drivers/nfc/microread/microread.c
> +++ b/drivers/nfc/microread/microread.c
> @@ -251,9 +251,8 @@ static int microread_start_poll(struct nfc_hci_dev *hdev,
>  		param[1] |= (1 << 1);
>  
>  	if ((im_protocols | tm_protocols) & NFC_PROTO_NFC_DEP_MASK) {
> -		hdev->gb = nfc_get_local_general_bytes(hdev->ndev,
> -						       &hdev->gb_len);
> -		if (hdev->gb == NULL || hdev->gb_len == 0) {
> +		nfc_get_local_general_bytes(hdev->ndev, hdev->gb, sizeof(hdev->gb), &hdev->gb_len);

Please line wrap so that lines are 80 columns wide or less.
Likewise elsewhere in this patch.

> +		if (hdev->gb_len == 0) {
>  			im_protocols &= ~NFC_PROTO_NFC_DEP_MASK;
>  			tm_protocols &= ~NFC_PROTO_NFC_DEP_MASK;
>  		}

...

> diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
> index dc65c719f35f..1ed0ecde5872 100644
> --- a/net/nfc/llcp_core.c
> +++ b/net/nfc/llcp_core.c
> @@ -635,23 +635,29 @@ static int nfc_llcp_build_gb(struct nfc_llcp_local *local)
>  	return ret;
>  }
>  
> -u8 *nfc_llcp_general_bytes(struct nfc_dev *dev, size_t *general_bytes_len)
> +u8 *nfc_llcp_general_bytes(struct nfc_dev *dev, u8 *out_gb, size_t gb_max_len, size_t *general_bytes_len)
>  {
>  	struct nfc_llcp_local *local;
>  
> +	if (!out_gb || !general_bytes_len)
> +		return NULL;
> +
> +	*general_bytes_len = 0;
> +
>  	local = nfc_llcp_find_local(dev);
> -	if (local == NULL) {
> -		*general_bytes_len = 0;
> +	if (local == NULL)
>  		return NULL;
> -	}
>  
>  	nfc_llcp_build_gb(local);
>  
> -	*general_bytes_len = local->gb_len;
> +	if (local->gb && local->gb_len) {
> +		*general_bytes_len = min_t(size_t, local->gb_len, gb_max_len);
> +		memcpy(out_gb, local->gb, *general_bytes_len);
> +	}

x86_64 W=1 builds with GCC 16.1.0 warn that:

  net/nfc/llcp_core.c: In function 'nfc_llcp_general_bytes':
  net/nfc/llcp_core.c:653:13: warning: the comparison will always evaluate as 'true' for the address of 'gb' will never be NULL [-Waddress]
    653 |         if (local->gb && local->gb_len) {
        |             ^~~~~
  In file included from net/nfc/llcp_core.c:15:
  net/nfc/llcp.h:77:12: note: 'gb' declared here
     77 |         u8 gb[NFC_MAX_GT_LEN];
        |            ^~

>  
>  	nfc_llcp_local_put(local);
>  
> -	return local->gb;
> +	return out_gb;
>  }
>  

...
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.