net/nfc/llcp_sock.c | 3 +++ 1 file changed, 3 insertions(+)
nfc_llcp_getsockopt() returns each of its option values via:
put_user(value, (u32 __user *) optval);
The (u32 __user *) cast tells put_user() to store sizeof(u32) = 4
bytes, regardless of what optlen the caller supplied. The earlier
clamp
len = min_t(u32, len, sizeof(u32));
only affects the optlen value that is later reported back to user
space; it does not constrain the put_user() store itself.
If a caller invokes getsockopt(NFC_LLCP_RW/MIUX/REMOTE_*) with
optlen < 4 (for example optlen = 1 against a single-byte buffer),
the kernel still writes 4 bytes into optval, scribbling up to 3
bytes of the caller's adjacent memory.
That violates the getsockopt(2) contract that the kernel must not write
more than *optlen bytes into the user buffer.
All five supported optnames (NFC_LLCP_RW, NFC_LLCP_MIUX,
NFC_LLCP_REMOTE_MIU, NFC_LLCP_REMOTE_LTO, NFC_LLCP_REMOTE_RW)
are affected because they share the same put_user() pattern.
Reject any call with optlen < sizeof(u32) up front so the
put_user() stores always have enough room. This formalises the
implicit u32 ABI these options have always returned.o
Maybe it is possible to change nfc_llcp_getsockopt() to accept optlen
< 4, but this might be a riskier operation than just keep the current
approach and guarantee that users are doing the right thing.
Fixes: 26fd76cab2e6 ("NFC: llcp: Implement socket options")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
net/nfc/llcp_sock.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index f1be1e84f6653..dc74e725ddd05 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -319,6 +319,9 @@ static int nfc_llcp_getsockopt(struct socket *sock, int level, int optname,
if (get_user(len, optlen))
return -EFAULT;
+ if (len < sizeof(u32))
+ return -EINVAL;
+
local = llcp_sock->local;
if (!local)
return -ENODEV;
---
base-commit: 1d5dcaa3bd65f2e8c9baa14a393d3a2dc5db7524
change-id: 20260513-fix_llc-27f483568135
Best regards,
--
Breno Leitao <leitao@debian.org>
From: 'Simon Horman' <horms@kernel.org> This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- nfc: llcp: avoid userspace overflow on invalid optlen This commit attempts to fix a userspace buffer overflow in nfc_llcp_getsockopt() by rejecting calls where the provided optlen is less than the size of a u32. > diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c > index f1be1e84f6653..dc74e725ddd05 100644 > --- a/net/nfc/llcp_sock.c > +++ b/net/nfc/llcp_sock.c > @@ -319,6 +319,9 @@ static int nfc_llcp_getsockopt(struct socket *sock, int level, int optname, > if (get_user(len, optlen)) > return -EFAULT; > > + if (len < sizeof(u32)) > + return -EINVAL; Since len is a signed int and sizeof(u32) is an unsigned size_t, does C integer promotion cause negative lengths to bypass this check? If a user provides an optlen of -1, it appears it would be promoted to a large positive number, making this condition false. It would then be explicitly cast to u32 in the min_t() call below, allowing the code to continue and overflow the userspace buffer exactly as this patch was attempting to prevent. > + > local = llcp_sock->local; > if (!local) > return -ENODEV; While looking at this, is there a potential use-after-free regression here if llcp_sock->local is read outside of the socket lock? If another thread calls llcp_sock_bind() on the same socket, it might successfully set llcp_sock->local, but then hit an error path. In that error path, bind() would drop the reference and set llcp_sock->local to NULL. If the getsockopt() thread reads local here, gets the valid pointer, and then blocks waiting for lock_sock(sk): getsockopt() thread: local = llcp_sock->local; // blocks on lock_sock(sk) bind() thread: // hits error path nfc_llcp_local_put(llcp_sock->local); llcp_sock->local = NULL; release_sock(sk); Will the getsockopt() thread wake up, acquire the lock, and then dereference the now-freed local pointer in the switch statement below?
On Mon, May 18, 2026 at 10:11:04AM +0100, Simon Horman wrote:
> > @@ -319,6 +319,9 @@ static int nfc_llcp_getsockopt(struct socket *sock, int level, int optname,
> > if (get_user(len, optlen))
> > return -EFAULT;
> >
> > + if (len < sizeof(u32))
> > + return -EINVAL;
>
> Since len is a signed int and sizeof(u32) is an unsigned size_t, does C
> integer promotion cause negative lengths to bypass this check?
Good catch, you're right. `len` is `int` and might get promoted to unsigned in the
comparison, so optlen = -1 becomes a huge value and slips past the check, then
min_t(u32, ...) clamps it back to 4 and the overflow happens anyway.
I'll fix this in v2 by casting:
if (len < (int)sizeof(u32))
return -EINVAL;
© 2016 - 2026 Red Hat, Inc.