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; > } > ...