[PATCH net v2 0/2] nfc: llcp: two fixes for nfc_llcp_getsockopt()

[PATCH net v2 0/2] nfc: llcp: two fixes for nfc_llcp_getsockopt()

[Breno Leitao]

While converting the NFC LLCP socket layer to the new getsockopt_iter()
API, I noticed that nfc_llcp_getsockopt() unconditionally stores four
bytes through a (u32 __user *) cast regardless of the caller-supplied
optlen, overflowing the user buffer when optlen < 4. Patch 1 adds an
explicit length check (with a signed-int guard so a negative optlen
cannot slip past it) and is what I originally sent as v1.

While reviewing v1, Simon/sashiko[1] pointed out that llcp_sock->local
is read outside lock_sock(sk) and can be freed by a concurrent
llcp_sock_bind() error path before getsockopt() dereferences it. Patch
2 moves the load and the NULL check inside the lock. Both fixes target
the same original commit, so they are now sent together as a two-patch
series.

Note: These fixes were compile-tested.

[1] https://lore.kernel.org/all/20260513-fix_llc-v1-1-33c76f931ff6@debian.org/

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Guard the length check against negative optlen (Simon Horman / sashiko).
- Add patch 2: move llcp_sock->local read inside lock_sock(sk) to close
  a UAF race with llcp_sock_bind() (Simon Horman / sashiko).
- Link to v1: https://patch.msgid.link/20260513-fix_llc-v1-1-33c76f931ff6@debian.org

To: David Heidelberg <david+nfc@ixit.cz>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Simon Horman <horms@kernel.org>
To: Samuel Ortiz <sameo@linux.intel.com>
Cc: oe-linux-nfc@lists.linux.dev
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Breno Leitao (2):
      nfc: llcp: avoid userspace overflow on invalid optlen
      nfc: llcp: read llcp_sock->local under the socket lock in getsockopt

 net/nfc/llcp_sock.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
---
base-commit: 1d5dcaa3bd65f2e8c9baa14a393d3a2dc5db7524
change-id: 20260513-fix_llc-27f483568135

Best regards,
--  
Breno Leitao <leitao@debian.org>

[PATCH net v2 1/2] nfc: llcp: avoid userspace overflow on invalid optlen

[Breno Leitao]

nfc_llcp_getsockopt() casts optval to (u32 __user *) for put_user(), so
the kernel always stores 4 bytes regardless of the caller-supplied
optlen. The existing min_t(u32, len, sizeof(u32)) only clamps the length
reported back to userspace; it does not constrain the store. A call with
optlen < 4 therefore writes past the user buffer, violating the
getsockopt(2) contract for all five supported optnames.

Reject any call with optlen < sizeof(u32) up front. 'len' is int, so a
plain size comparison would promote a negative optlen to size_t and slip
past the check; an explicit 'len < 0' test is added first to catch
negative values before the size compare.

Fixes: 26fd76cab2e6 ("NFC: llcp: Implement socket options")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/nfc/llcp_sock.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index f1be1e84f6653..aa9a78a671521 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -319,6 +319,12 @@ static int nfc_llcp_getsockopt(struct socket *sock, int level, int optname,
 	if (get_user(len, optlen))
 		return -EFAULT;
 
+	if (len < 0)
+		return -EINVAL;
+
+	if (len < sizeof(u32))
+		return -EINVAL;
+
 	local = llcp_sock->local;
 	if (!local)
 		return -ENODEV;

-- 
2.53.0-Meta

[PATCH net v2 2/2] nfc: llcp: read llcp_sock->local under the socket lock in getsockopt

[Breno Leitao]

nfc_llcp_getsockopt() read llcp_sock->local before lock_sock(sk) and
then dereferenced the cached pointer inside the locked region.
llcp_sock_bind() assigns and clears llcp_sock->local under the same
socket lock, dropping the last reference on its error path. A
getsockopt() racing an in-flight bind() can observe the pointer, block
on lock_sock(), and then dereference a freed nfc_llcp_local once bind()
has unwound.

Move the llcp_sock->local read and the NULL check inside the
lock_sock(sk) region so bind() cannot mutate or free the pointer between
the load and the use.

Fixes: 26fd76cab2e6 ("NFC: llcp: Implement socket options")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/nfc/llcp_sock.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index aa9a78a671521..266590d402664 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -325,14 +325,16 @@ static int nfc_llcp_getsockopt(struct socket *sock, int level, int optname,
 	if (len < sizeof(u32))
 		return -EINVAL;
 
-	local = llcp_sock->local;
-	if (!local)
-		return -ENODEV;
-
 	len = min_t(u32, len, sizeof(u32));
 
 	lock_sock(sk);
 
+	local = llcp_sock->local;
+	if (!local) {
+		release_sock(sk);
+		return -ENODEV;
+	}
+
 	switch (optname) {
 	case NFC_LLCP_RW:
 		rw = llcp_sock->rw > LLCP_MAX_RW ? local->rw : llcp_sock->rw;

-- 
2.53.0-Meta

Re: [PATCH net v2 0/2] nfc: llcp: two fixes for nfc_llcp_getsockopt()

[Simon Horman]

On Thu, May 21, 2026 at 07:32:08AM -0700, Breno Leitao wrote:
> While converting the NFC LLCP socket layer to the new getsockopt_iter()
> API, I noticed that nfc_llcp_getsockopt() unconditionally stores four
> bytes through a (u32 __user *) cast regardless of the caller-supplied
> optlen, overflowing the user buffer when optlen < 4. Patch 1 adds an
> explicit length check (with a signed-int guard so a negative optlen
> cannot slip past it) and is what I originally sent as v1.
> 
> While reviewing v1, Simon/sashiko[1] pointed out that llcp_sock->local
> is read outside lock_sock(sk) and can be freed by a concurrent
> llcp_sock_bind() error path before getsockopt() dereferences it. Patch
> 2 moves the load and the NULL check inside the lock. Both fixes target
> the same original commit, so they are now sent together as a two-patch
> series.
> 
> Note: These fixes were compile-tested.
> 
> [1] https://lore.kernel.org/all/20260513-fix_llc-v1-1-33c76f931ff6@debian.org/
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - Guard the length check against negative optlen (Simon Horman / sashiko).
> - Add patch 2: move llcp_sock->local read inside lock_sock(sk) to close
>   a UAF race with llcp_sock_bind() (Simon Horman / sashiko).
> - Link to v1: https://patch.msgid.link/20260513-fix_llc-v1-1-33c76f931ff6@debian.org

Thanks for the update.

There is an AI-generated review of this patch on sashiko.dev.
It looks like it flags pre-existing issue that doesn't directly
impact the intent of this patch-set. So I don't believe it should delay
progress of this patch-set.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v2 0/2] nfc: llcp: two fixes for nfc_llcp_getsockopt()

[Breno Leitao]

Hello Simon,

On Tue, May 26, 2026 at 06:33:51PM +0000, Simon Horman wrote:
> > Changes in v2:
> > - Guard the length check against negative optlen (Simon Horman / sashiko).
> > - Add patch 2: move llcp_sock->local read inside lock_sock(sk) to close
> >   a UAF race with llcp_sock_bind() (Simon Horman / sashiko).
> > - Link to v1: https://patch.msgid.link/20260513-fix_llc-v1-1-33c76f931ff6@debian.org
> 
> Thanks for the update.
> 
> There is an AI-generated review of this patch on sashiko.dev.
> It looks like it flags pre-existing issue that doesn't directly
> impact the intent of this patch-set. So I don't believe it should delay
> progress of this patch-set.

Agree, and looking at what sashiko raised, it seems reasonable, I would
say the state check in llcp_sock_connect() is incomplete, and
llcp_sock_connect() should refuse non-LLCP_CLOSED states.

I will send an independent patch later, if we don't hear any concern
from anyone else.

Thanks for the review,
--breno