[PATCH bpf v2 0/4] bpf, sockmap: Fix sockmap leaking UDP socks

[PATCH bpf v2 0/4] bpf, sockmap: Fix sockmap leaking UDP socks

[Michal Luczaj]

Fix for UDP sockets getting leaked during sockmap lookup/release.
Accompanied by selftests updates.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Changes in v2:
- selftest: drop the original, adapt old tests
- fix: change approach to rejecting unbound UDP [Kuniyuki]
- Link to v1: https://patch.msgid.link/20260623-sockmap-lookup-udp-leak-v1-0-05804f9308e4@rbox.co

---
Michal Luczaj (4):
      bpf, sockmap: Reject unhashed UDP sockets on sockmap update
      selftests/bpf: Ensure UDP sockets are bound
      selftests/bpf: Adapt sockmap update error handling
      selftests/bpf: Fail unbound UDP on sockmap update

 net/core/sock_map.c                                     |  2 ++
 tools/testing/selftests/bpf/prog_tests/sockmap_basic.c  |  6 +++---
 tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 17 +++++++++--------
 tools/testing/selftests/bpf/test_maps.c                 |  6 +++---
 4 files changed, 17 insertions(+), 14 deletions(-)
---
base-commit: 26490a375cb9be9bac96b5171610fd85ca6c2305
change-id: 20260617-sockmap-lookup-udp-leak-bc4e5c5481d7

Best regards,
--  
Michal Luczaj <mhal@rbox.co>

[PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Michal Luczaj]

UDP sockets get SOCK_RCU_FREE set when (auto-)bound. This means
sk_is_refcounted(unbound) = true, while sk_is_refcounted(bound) = false.

Because sockmap accepts unbound UDP sockets, a BPF program can increment a
socket's refcount via lookup. If the socket is subsequently bound, the
transition from unbound to bound causes bpf_sk_release() to skip the
decrement of the refcount, causing a memory leak.

unreferenced object 0xffff88810bc2eb40 (size 1984):
  comm "test_progs", pid 2451, jiffies 4295320596
  hex dump (first 32 bytes):
    7f 00 00 01 7f 00 00 01 d2 04 1b b7 04 d2 00 00  ................
    02 00 01 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
  backtrace (crc bdee079d):
    kmem_cache_alloc_noprof+0x557/0x660
    sk_prot_alloc+0x69/0x240
    sk_alloc+0x30/0x460
    inet_create+0x2ce/0xf80
    __sock_create+0x25b/0x5c0
    __sys_socket+0x119/0x1d0
    __x64_sys_socket+0x72/0xd0
    do_syscall_64+0xa1/0x5f0
    entry_SYSCALL_64_after_hwframe+0x76/0x7e

Instead of special-casing for refcounted sockets, reject unhashed UDP
sockets during sockmap updates, as there is no benefit to supporting those.
This effectively reverts the commit under Fixes, with two exceptions:

1. sock_map_sk_state_allowed() maintains a fall-through `return true`.
2. In the spirit of commit b8b8315e39ff ("bpf, sockmap: Remove unhash
   handler for BPF sockmap usage"), the proto::unhash BPF handler is not
   reintroduced.

Historical note: this issue is related to commit 67312adc96b5 ("bpf: reject
unhashed sockets in bpf_sk_assign").

Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 net/core/sock_map.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index c60ba6d292f9..9efbd8ca7db8 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -542,6 +542,8 @@ static bool sock_map_sk_state_allowed(const struct sock *sk)
 {
 	if (sk_is_tcp(sk))
 		return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN);
+	if (sk_is_udp(sk))
+		return sk_hashed(sk);
 	if (sk_is_stream_unix(sk))
 		return (1 << READ_ONCE(sk->sk_state)) & TCPF_ESTABLISHED;
 	if (sk_is_vsock(sk) &&

-- 
2.54.0

[PATCH bpf v2 2/4] selftests/bpf: Ensure UDP sockets are bound

[Michal Luczaj]

Update sockmap_basic tests to bind sockets before they are used. This
accommodates the recent change in sockmap that rejects unbound UDP sockets.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 tools/testing/selftests/bpf/prog_tests/sockmap_basic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index cb3229711f93..2d22a9058a8e 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -853,7 +853,7 @@ static void test_sockmap_many_socket(void)
 		return;
 	}
 
-	udp = xsocket(AF_INET, SOCK_DGRAM | SOCK_NONBLOCK, 0);
+	udp = socket_loopback(AF_INET, SOCK_DGRAM | SOCK_NONBLOCK);
 	if (udp < 0) {
 		close(dgram);
 		close(tcp);
@@ -922,7 +922,7 @@ static void test_sockmap_many_maps(void)
 		return;
 	}
 
-	udp = xsocket(AF_INET, SOCK_DGRAM | SOCK_NONBLOCK, 0);
+	udp = socket_loopback(AF_INET, SOCK_DGRAM | SOCK_NONBLOCK);
 	if (udp < 0) {
 		close(dgram);
 		close(tcp);
@@ -993,7 +993,7 @@ static void test_sockmap_same_sock(void)
 		return;
 	}
 
-	udp = xsocket(AF_INET, SOCK_DGRAM | SOCK_NONBLOCK, 0);
+	udp = socket_loopback(AF_INET, SOCK_DGRAM | SOCK_NONBLOCK);
 	if (udp < 0) {
 		close(dgram);
 		close(tcp);

-- 
2.54.0

[PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[Michal Luczaj]

Update sockmap_listen to accommodate the recent change in sockmap that
rejects unbound UDP sockets.

TCP: Reject unbound and bound (unless established or listening).
UDP: Accept only bound sockets.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index cc0c68bab907..6ee1bc6b3b23 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -63,11 +63,8 @@ static void test_insert_opened(struct test_sockmap_listen *skel __always_unused,
 	errno = 0;
 	value = s;
 	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
-	if (sotype == SOCK_STREAM) {
-		if (!err || errno != EOPNOTSUPP)
-			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
-	} else if (err)
-		FAIL_ERRNO("map_update: expected success");
+	if (!err || errno != EOPNOTSUPP)
+		FAIL_ERRNO("map_update: expected EOPNOTSUPP");
 	xclose(s);
 }
 
@@ -93,8 +90,12 @@ static void test_insert_bound(struct test_sockmap_listen *skel __always_unused,
 	errno = 0;
 	value = s;
 	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
-	if (!err || errno != EOPNOTSUPP)
-		FAIL_ERRNO("map_update: expected EOPNOTSUPP");
+	if (sotype == SOCK_STREAM) {
+		if (!err || errno != EOPNOTSUPP)
+			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
+	} else if (err) {
+		FAIL_ERRNO("map_update: expected success");
+	}
 close:
 	xclose(s);
 }
@@ -1289,7 +1290,7 @@ static void test_ops(struct test_sockmap_listen *skel, struct bpf_map *map,
 		/* insert */
 		TEST(test_insert_invalid),
 		TEST(test_insert_opened),
-		TEST(test_insert_bound, SOCK_STREAM),
+		TEST(test_insert_bound),
 		TEST(test_insert),
 		/* delete */
 		TEST(test_delete_after_insert),

-- 
2.54.0

[PATCH bpf v2 4/4] selftests/bpf: Fail unbound UDP on sockmap update

[Michal Luczaj]

sockmap now rejects unbound UDP sockets. Adjust test_maps.

This effectively reverts commit c39aa2159974 ("bpf, selftests: Fix
test_maps now that sockmap supports UDP").

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 tools/testing/selftests/bpf/test_maps.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index c32da7bd8be2..81cd5d0d69c1 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -759,12 +759,12 @@ static void test_sockmap(unsigned int tasks, void *data)
 		goto out_sockmap;
 	}
 
-	/* Test update with unsupported UDP socket */
+	/* Test update with unsupported unbound UDP socket */
 	udp = socket(AF_INET, SOCK_DGRAM, 0);
 	i = 0;
 	err = bpf_map_update_elem(fd, &i, &udp, BPF_ANY);
-	if (err) {
-		printf("Failed socket update SOCK_DGRAM '%i:%i'\n",
+	if (!err) {
+		printf("Failed allowed unbound SOCK_DGRAM socket update '%i:%i'\n",
 		       i, udp);
 		goto out_sockmap;
 	}

-- 
2.54.0

Re: [PATCH bpf v2 4/4] selftests/bpf: Fail unbound UDP on sockmap update

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing check for `socket()` failure leads to a false positive test pass.
--

commit 7f8484db13c3ae1f9676009ab7e283f2b68e4ae0
Author: Michal Luczaj <mhal@rbox.co>

selftests/bpf: Fail unbound UDP on sockmap update

This commit modifies test_maps to expect a failure when updating a sockmap
with an unbound UDP socket. It effectively reverts a previous selftests
change now that sockmap rejects unbound UDP sockets.

> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -759,12 +759,12 @@ static void test_sockmap(unsigned int tasks, void *data)
>  		goto out_sockmap;
>  	}
>  
> -	/* Test update with unsupported UDP socket */
> +	/* Test update with unsupported unbound UDP socket */
>  	udp = socket(AF_INET, SOCK_DGRAM, 0);
>  	i = 0;
>  	err = bpf_map_update_elem(fd, &i, &udp, BPF_ANY);

[Severity: Medium]
Will test_sockmap() in tools/testing/selftests/bpf/test_maps.c now silently
pass if socket() fails to create a file descriptor?

Because this patch changes the logic to expect an error from
bpf_map_update_elem() by checking if (!err), if socket() returns -1 due to
setup failures, bpf_map_update_elem() will fail with an error like -EBADF.

This would cause the test to erroneously pass without actually testing the
kernel's behavior for unbound UDP sockets. Does this test need a check to
ensure the returned udp file descriptor is valid before proceeding with the
map update?

> -	if (err) {
> -		printf("Failed socket update SOCK_DGRAM '%i:%i'\n",
> +	if (!err) {
> +		printf("Failed allowed unbound SOCK_DGRAM socket update '%i:%i'\n",
>  		       i, udp);
>  		goto out_sockmap;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-sockmap-lookup-udp-leak-v2-0-7e7e201c951a@rbox.co?part=4

Re: [PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Low] Usage of legacy FAIL_ERRNO / CHECK_FAIL macros in updated tests instead of modern ASSERT_* macros.
--

commit bc0e5d743f397cf1347e049495ba4a7950323606
Author: Michal Luczaj <mhal@rbox.co>

selftests/bpf: Adapt sockmap update error handling

This patch updates sockmap_listen to accommodate recent changes in sockmap
that reject unbound UDP sockets.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index cc0c68bab9079..6ee1bc6b3b235 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -63,11 +63,8 @@ static void test_insert_opened(struct test_sockmap_listen *skel __always_unused,
>  	errno = 0;
>  	value = s;
>  	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
> -	if (sotype == SOCK_STREAM) {
> -		if (!err || errno != EOPNOTSUPP)
> -			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> -	} else if (err)
> -		FAIL_ERRNO("map_update: expected success");
> +	if (!err || errno != EOPNOTSUPP)
> +		FAIL_ERRNO("map_update: expected EOPNOTSUPP");

[Severity: Low]
This is a pre-existing issue, but since this block in test_insert_opened() is
being updated, should these deprecated FAIL_ERRNO() macros be migrated to the
modern ASSERT_*() macros? The BPF selftests subsystem guidelines prefer using
ASSERT_* macros when updating existing tests.

[ ... ]

> @@ -93,8 +90,12 @@ static void test_insert_bound(struct test_sockmap_listen *skel __always_unused,
>  	errno = 0;
>  	value = s;
>  	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
> -	if (!err || errno != EOPNOTSUPP)
> -		FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> +	if (sotype == SOCK_STREAM) {
> +		if (!err || errno != EOPNOTSUPP)
> +			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> +	} else if (err) {
> +		FAIL_ERRNO("map_update: expected success");
> +	}

[Severity: Low]
This isn't a bug introduced by this patch, but would it make sense to replace
these FAIL_ERRNO() checks in test_insert_bound() with ASSERT_ERR() or similar
modern macros while refactoring this logic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-sockmap-lookup-udp-leak-v2-0-7e7e201c951a@rbox.co?part=3

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Kuniyuki Iwashima]

On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>
> UDP sockets get SOCK_RCU_FREE set when (auto-)bound. This means
> sk_is_refcounted(unbound) = true, while sk_is_refcounted(bound) = false.
>
> Because sockmap accepts unbound UDP sockets, a BPF program can increment a
> socket's refcount via lookup. If the socket is subsequently bound, the
> transition from unbound to bound causes bpf_sk_release() to skip the
> decrement of the refcount, causing a memory leak.
>
> unreferenced object 0xffff88810bc2eb40 (size 1984):
>   comm "test_progs", pid 2451, jiffies 4295320596
>   hex dump (first 32 bytes):
>     7f 00 00 01 7f 00 00 01 d2 04 1b b7 04 d2 00 00  ................
>     02 00 01 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
>   backtrace (crc bdee079d):
>     kmem_cache_alloc_noprof+0x557/0x660
>     sk_prot_alloc+0x69/0x240
>     sk_alloc+0x30/0x460
>     inet_create+0x2ce/0xf80
>     __sock_create+0x25b/0x5c0
>     __sys_socket+0x119/0x1d0
>     __x64_sys_socket+0x72/0xd0
>     do_syscall_64+0xa1/0x5f0
>     entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Instead of special-casing for refcounted sockets, reject unhashed UDP
> sockets during sockmap updates, as there is no benefit to supporting those.
> This effectively reverts the commit under Fixes, with two exceptions:
>
> 1. sock_map_sk_state_allowed() maintains a fall-through `return true`.
> 2. In the spirit of commit b8b8315e39ff ("bpf, sockmap: Remove unhash
>    handler for BPF sockmap usage"), the proto::unhash BPF handler is not
>    reintroduced.
>
> Historical note: this issue is related to commit 67312adc96b5 ("bpf: reject
> unhashed sockets in bpf_sk_assign").
>
> Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>

Looks good, thanks !

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH bpf v2 2/4] selftests/bpf: Ensure UDP sockets are bound

[Kuniyuki Iwashima]

On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>
> Update sockmap_basic tests to bind sockets before they are used. This
> accommodates the recent change in sockmap that rejects unbound UDP sockets.
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>

nit: this should be patch 1.

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Incomplete fix: Memory ordering race allows unbound UDP sockets to be inserted into sockmap, bypassing the sk_hashed() check.

Pre-existing issues:
- [High] The patch addresses a memory leak caused by UDP sockets dynamically acquiring the SOCK_RCU_FREE flag while held by a BPF program, but leaves TCP sockets vulnerable to the exact same state-transition leak.
--

commit f2013e408c7b5bf3e40a884d6c22c888b4372b0b
Author: Michal Luczaj <mhal@rbox.co>

bpf, sockmap: Reject unhashed UDP sockets on sockmap update

This commit rejects unhashed UDP sockets during sockmap updates to prevent
a memory leak. The leak occurred because unbound UDP sockets are refcounted,
but become non-refcounted (SOCK_RCU_FREE) when bound, which skips the
refcount decrement in bpf_sk_release().

> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index c60ba6d292f9a..9efbd8ca7db83 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c
> @@ -542,6 +542,8 @@ static bool sock_map_sk_state_allowed(const struct sock *sk)
>  {
>  	if (sk_is_tcp(sk))
>  		return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN);

[Severity: High]
This is a pre-existing issue, but does this leave TCP sockets susceptible to
the exact same state-transition leak?

If a TCP socket is held by a BPF program in TCP_ESTABLISHED state and then
transitions to TCP_LISTEN, inet_hash() unconditionally sets SOCK_RCU_FREE:

net/ipv4/inet_hashtables.c:inet_hash() {
    ...
    sock_set_flag(sk, SOCK_RCU_FREE);
    ...
}

This dynamic state change would cause bpf_sk_release() to evaluate
sk_is_refcounted(sk) as false and skip the refcount decrement, leading to
a memory leak similar to the UDP case.

> +	if (sk_is_udp(sk))
> +		return sk_hashed(sk);

[Severity: High]
Could a memory ordering regression here allow unbound UDP sockets to still
bypass this check?

In udp_v4_get_port(), the flag and hash are set:

net/ipv4/udp.c:udp_v4_get_port() {
    ...
    sock_set_flag(sk, SOCK_RCU_FREE);
    sk_add_node_rcu(sk, &hslot->head);
    ...
}

Because these stores lack a memory barrier between them, could sk_hashed(sk)
return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
the leak to still trigger?

>  	if (sk_is_stream_unix(sk))
>  		return (1 << READ_ONCE(sk->sk_state)) & TCPF_ESTABLISHED;
>  	if (sk_is_vsock(sk) &&

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-sockmap-lookup-udp-leak-v2-0-7e7e201c951a@rbox.co?part=1

Re: [PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[Kuniyuki Iwashima]

On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>
> Update sockmap_listen to accommodate the recent change in sockmap that
> rejects unbound UDP sockets.
>
> TCP: Reject unbound and bound (unless established or listening).
> UDP: Accept only bound sockets.
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
>  tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index cc0c68bab907..6ee1bc6b3b23 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -63,11 +63,8 @@ static void test_insert_opened(struct test_sockmap_listen *skel __always_unused,
>         errno = 0;
>         value = s;
>         err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
> -       if (sotype == SOCK_STREAM) {
> -               if (!err || errno != EOPNOTSUPP)
> -                       FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> -       } else if (err)
> -               FAIL_ERRNO("map_update: expected success");

Initially I thought AF_UNIX still exercised this path but it was removed
in f3de1cf621f7.  The leftover in family_str() was a bit confusing, so please
follow up on bpf-next.

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH bpf v2 4/4] selftests/bpf: Fail unbound UDP on sockmap update

[Kuniyuki Iwashima]

On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>
> sockmap now rejects unbound UDP sockets. Adjust test_maps.
>
> This effectively reverts commit c39aa2159974 ("bpf, selftests: Fix
> test_maps now that sockmap supports UDP").
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
>  tools/testing/selftests/bpf/test_maps.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index c32da7bd8be2..81cd5d0d69c1 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -759,12 +759,12 @@ static void test_sockmap(unsigned int tasks, void *data)
>                 goto out_sockmap;
>         }
>
> -       /* Test update with unsupported UDP socket */
> +       /* Test update with unsupported unbound UDP socket */
>         udp = socket(AF_INET, SOCK_DGRAM, 0);
>         i = 0;
>         err = bpf_map_update_elem(fd, &i, &udp, BPF_ANY);
> -       if (err) {
> -               printf("Failed socket update SOCK_DGRAM '%i:%i'\n",
> +       if (!err) {
> +               printf("Failed allowed unbound SOCK_DGRAM socket update '%i:%i'\n",

nit: Maybe s/Failed/Unexpectedly succeeded/ ?

If we want to avoid breakage, this patch needs to be squashed to
the fix patch, but it's discouraged in netdev, not sure about bpf tree.

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

>     selftests/bpf: Adapt sockmap update error handling
>
>     Update sockmap_listen to accommodate the recent change in sockmap that
>     rejects unbound UDP sockets.
>
>     TCP: Reject unbound and bound (unless established or listening).
>     UDP: Accept only bound sockets.
>
>     Signed-off-by: Michal Luczaj <mhal@rbox.co>

This adjusts the test expectations that commit 0c48eefae712 ("sock_map:
Lift socket state restriction for datagram sockets") had introduced, the
per-sotype branching in test_insert_opened() along with the matching
changes in test_insert_bound() and test_ops().

The companion kernel fix 77a34848a723 ("bpf, sockmap: Reject unhashed UDP
sockets on sockmap update") already carries:

  Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")

Should this selftest commit carry the same Fixes: tag?

  Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/28264556257

Re: [PATCH bpf v2 0/4] bpf, sockmap: Fix sockmap leaking UDP socks

[sun jian]

On Sat, Jun 27, 2026 at 4:37 AM Michal Luczaj <mhal@rbox.co> wrote:
>
> Fix for UDP sockets getting leaked during sockmap lookup/release.
> Accompanied by selftests updates.
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
> Changes in v2:
> - selftest: drop the original, adapt old tests
> - fix: change approach to rejecting unbound UDP [Kuniyuki]
> - Link to v1: https://patch.msgid.link/20260623-sockmap-lookup-udp-leak-v1-0-05804f9308e4@rbox.co
>
> ---
> Michal Luczaj (4):
>       bpf, sockmap: Reject unhashed UDP sockets on sockmap update
>       selftests/bpf: Ensure UDP sockets are bound
>       selftests/bpf: Adapt sockmap update error handling
>       selftests/bpf: Fail unbound UDP on sockmap update
>
>  net/core/sock_map.c                                     |  2 ++
>  tools/testing/selftests/bpf/prog_tests/sockmap_basic.c  |  6 +++---
>  tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 17 +++++++++--------
>  tools/testing/selftests/bpf/test_maps.c                 |  6 +++---
>  4 files changed, 17 insertions(+), 14 deletions(-)
> ---
> base-commit: 26490a375cb9be9bac96b5171610fd85ca6c2305
> change-id: 20260617-sockmap-lookup-udp-leak-bc4e5c5481d7
>
> Best regards,
> --
> Michal Luczaj <mhal@rbox.co>
>
>

Hi Michal,

I tested this series on bpf.git base commit:
  26490a375cb9be9bac96b5171610fd85ca6c2305
The test environment was virtme-ng x86_64.

Before, with only patches 2/4-4/4 applied:
  sockmap_basic:  PASS
  sockmap_listen: FAIL as expected
    - UDP test_insert_opened failed on sockmap/sockhash,
      both IPv4 and IPv6
  test_maps:      FAIL as expected
    - failed at the new unbound SOCK_DGRAM update check

After applying the full series:
  sockmap_basic:  PASS
  sockmap_listen: PASS
  test_maps:      no longer failed at the unbound
                  SOCK_DGRAM update check
test_maps still later reported:
  Failed sockmap unexpected timeout

However, I can reproduce the same timeout in three consecutive
runs on the unmodified base commit, so this looks pre-existing or
environmental in my virtme-ng setup rather than a regression from
this series.

I am not adding a Tested-by tag for now because test_maps does not
fully pass in my setup. The targeted before/after behavior behaved as
expected.

Thanks,
Sun Jian

Re: [PATCH bpf v2 4/4] selftests/bpf: Fail unbound UDP on sockmap update

[Michal Luczaj]

On 6/26/26 22:41, sashiko-bot@kernel.org wrote:
> [Severity: Medium]
> Will test_sockmap() in tools/testing/selftests/bpf/test_maps.c now silently
> pass if socket() fails to create a file descriptor?
> 
> Because this patch changes the logic to expect an error from
> bpf_map_update_elem() by checking if (!err), if socket() returns -1 due to
> setup failures, bpf_map_update_elem() will fail with an error like -EBADF.
> 
> This would cause the test to erroneously pass without actually testing the
> kernel's behavior for unbound UDP sockets. Does this test need a check to
> ensure the returned udp file descriptor is valid before proceeding with the
> map update?

That's true, will fix.

Michal

Re: [PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[Michal Luczaj]

On 6/26/26 22:43, sashiko-bot@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
>> index cc0c68bab9079..6ee1bc6b3b235 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
>> @@ -63,11 +63,8 @@ static void test_insert_opened(struct test_sockmap_listen *skel __always_unused,
>>  	errno = 0;
>>  	value = s;
>>  	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
>> -	if (sotype == SOCK_STREAM) {
>> -		if (!err || errno != EOPNOTSUPP)
>> -			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
>> -	} else if (err)
>> -		FAIL_ERRNO("map_update: expected success");
>> +	if (!err || errno != EOPNOTSUPP)
>> +		FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> 
> [Severity: Low]
> This is a pre-existing issue, but since this block in test_insert_opened() is
> being updated, should these deprecated FAIL_ERRNO() macros be migrated to the
> modern ASSERT_*() macros? The BPF selftests subsystem guidelines prefer using
> ASSERT_* macros when updating existing tests.
> 
> [ ... ]
> 
>> @@ -93,8 +90,12 @@ static void test_insert_bound(struct test_sockmap_listen *skel __always_unused,
>>  	errno = 0;
>>  	value = s;
>>  	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
>> -	if (!err || errno != EOPNOTSUPP)
>> -		FAIL_ERRNO("map_update: expected EOPNOTSUPP");
>> +	if (sotype == SOCK_STREAM) {
>> +		if (!err || errno != EOPNOTSUPP)
>> +			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
>> +	} else if (err) {
>> +		FAIL_ERRNO("map_update: expected success");
>> +	}
> 
> [Severity: Low]
> This isn't a bug introduced by this patch, but would it make sense to replace
> these FAIL_ERRNO() checks in test_insert_bound() with ASSERT_ERR() or similar
> modern macros while refactoring this logic?

All right, will do.

Michal

Re: [PATCH bpf v2 2/4] selftests/bpf: Ensure UDP sockets are bound

[Michal Luczaj]

On 6/26/26 22:47, Kuniyuki Iwashima wrote:
> On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>>
>> Update sockmap_basic tests to bind sockets before they are used. This
>> accommodates the recent change in sockmap that rejects unbound UDP sockets.
>>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> 
> nit: this should be patch 1.

OK, I'll push it up.

thanks,
Michal

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Michal Luczaj]

On 6/26/26 22:58, sashiko-bot@kernel.org wrote:
> ...
> [Severity: High]
> This is a pre-existing issue, but does this leave TCP sockets susceptible to
> the exact same state-transition leak?
> 
> If a TCP socket is held by a BPF program in TCP_ESTABLISHED state and then
> transitions to TCP_LISTEN, inet_hash() unconditionally sets SOCK_RCU_FREE:
> 
> net/ipv4/inet_hashtables.c:inet_hash() {
>     ...
>     sock_set_flag(sk, SOCK_RCU_FREE);
>     ...
> }
> 
> This dynamic state change would cause bpf_sk_release() to evaluate
> sk_is_refcounted(sk) as false and skip the refcount decrement, leading to
> a memory leak similar to the UDP case.
> 
>> +	if (sk_is_udp(sk))
>> +		return sk_hashed(sk);

Right, TCP sockets can leak in a similar fashion.

unreferenced object 0xffff88810544b400 (size 3200):
  comm "softirq", pid 0, jiffies 4299372426
  hex dump (first 32 bytes):
    7f 00 00 01 7f 00 00 01 48 40 f2 e3 00 00 00 00  ........H@......
    02 00 07 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
  backtrace (crc 82faf8d5):
    kmem_cache_alloc_noprof+0x53e/0x640
    sk_prot_alloc+0x69/0x240
    sk_clone+0x79/0x1200
    inet_csk_clone_lock+0x30/0x760
    tcp_create_openreq_child+0x34/0x2740
    tcp_v4_syn_recv_sock+0x12e/0x1080
    tcp_check_req+0x447/0x2310
    tcp_v4_rcv+0x1026/0x3c90
    ip_protocol_deliver_rcu+0x93/0x340
    ip_local_deliver_finish+0x356/0x5c0
    ip_local_deliver+0x184/0x4a0
    ip_rcv+0x4f4/0x5b0
    __netif_receive_skb_one_core+0x153/0x1b0
    process_backlog+0x28d/0x1190
    __napi_poll+0xa7/0x520
    net_rx_action+0x3f0/0xca0

But I guess that's a material for a followup series?

> [Severity: High]
> Could a memory ordering regression here allow unbound UDP sockets to still
> bypass this check?
> 
> In udp_v4_get_port(), the flag and hash are set:
> 
> net/ipv4/udp.c:udp_v4_get_port() {
>     ...
>     sock_set_flag(sk, SOCK_RCU_FREE);
>     sk_add_node_rcu(sk, &hslot->head);
>     ...
> }
> 
> Because these stores lack a memory barrier between them, could sk_hashed(sk)
> return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
> the leak to still trigger?

I'd like to verify it on a weakly-ordered CPU; please give me a day or two.

thanks,
Michal

Re: [PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[Michal Luczaj]

On 6/26/26 22:58, Kuniyuki Iwashima wrote:
> On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>>
>> Update sockmap_listen to accommodate the recent change in sockmap that
>> rejects unbound UDP sockets.
>>
>> TCP: Reject unbound and bound (unless established or listening).
>> UDP: Accept only bound sockets.
>>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>>  tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 17 +++++++++--------
>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
>> index cc0c68bab907..6ee1bc6b3b23 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
>> @@ -63,11 +63,8 @@ static void test_insert_opened(struct test_sockmap_listen *skel __always_unused,
>>         errno = 0;
>>         value = s;
>>         err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
>> -       if (sotype == SOCK_STREAM) {
>> -               if (!err || errno != EOPNOTSUPP)
>> -                       FAIL_ERRNO("map_update: expected EOPNOTSUPP");
>> -       } else if (err)
>> -               FAIL_ERRNO("map_update: expected success");
> 
> Initially I thought AF_UNIX still exercised this path but it was removed
> in f3de1cf621f7.  The leftover in family_str() was a bit confusing, so please
> follow up on bpf-next.

Sure, will do.

thanks,
Michal

Re: [PATCH bpf v2 0/4] bpf, sockmap: Fix sockmap leaking UDP socks

[Michal Luczaj]

On 6/27/26 19:34, sun jian wrote:
> On Sat, Jun 27, 2026 at 4:37 AM Michal Luczaj <mhal@rbox.co> wrote:
>>
>> Fix for UDP sockets getting leaked during sockmap lookup/release.
>> Accompanied by selftests updates.
>> ...
> 
> Hi Michal,
> 
> I tested this series on bpf.git base commit:
>   26490a375cb9be9bac96b5171610fd85ca6c2305
> The test environment was virtme-ng x86_64.

Great, thank you.

> However, I can reproduce the same timeout in three consecutive
> runs on the unmodified base commit, so this looks pre-existing or
> environmental in my virtme-ng setup rather than a regression from
> this series.

Yup, same for me. Good thing bpf-next appears to have this fixed.

thanks,
Michal

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Jakub Sitnicki]

On Sun, Jun 28, 2026 at 11:59 PM +02, Michal Luczaj wrote:
> On 6/26/26 22:58, sashiko-bot@kernel.org wrote:
>> [Severity: High]
>> Could a memory ordering regression here allow unbound UDP sockets to still
>> bypass this check?
>> 
>> In udp_v4_get_port(), the flag and hash are set:
>> 
>> net/ipv4/udp.c:udp_v4_get_port() {
>>     ...
>>     sock_set_flag(sk, SOCK_RCU_FREE);
>>     sk_add_node_rcu(sk, &hslot->head);
>>     ...
>> }
>> 
>> Because these stores lack a memory barrier between them, could sk_hashed(sk)
>> return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
>> the leak to still trigger?
>
> I'd like to verify it on a weakly-ordered CPU; please give me a day or two.

False positive, IMO.
Both ->get_port and sock_map_sk_state_allowed run with sk_lock held.

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Jakub Sitnicki]

On Fri, Jun 26, 2026 at 10:36 PM +02, Michal Luczaj wrote:
> UDP sockets get SOCK_RCU_FREE set when (auto-)bound. This means
> sk_is_refcounted(unbound) = true, while sk_is_refcounted(bound) = false.
>
> Because sockmap accepts unbound UDP sockets, a BPF program can increment a
> socket's refcount via lookup. If the socket is subsequently bound, the
> transition from unbound to bound causes bpf_sk_release() to skip the
> decrement of the refcount, causing a memory leak.
>
> unreferenced object 0xffff88810bc2eb40 (size 1984):
>   comm "test_progs", pid 2451, jiffies 4295320596
>   hex dump (first 32 bytes):
>     7f 00 00 01 7f 00 00 01 d2 04 1b b7 04 d2 00 00  ................
>     02 00 01 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
>   backtrace (crc bdee079d):
>     kmem_cache_alloc_noprof+0x557/0x660
>     sk_prot_alloc+0x69/0x240
>     sk_alloc+0x30/0x460
>     inet_create+0x2ce/0xf80
>     __sock_create+0x25b/0x5c0
>     __sys_socket+0x119/0x1d0
>     __x64_sys_socket+0x72/0xd0
>     do_syscall_64+0xa1/0x5f0
>     entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Instead of special-casing for refcounted sockets, reject unhashed UDP
> sockets during sockmap updates, as there is no benefit to supporting those.
> This effectively reverts the commit under Fixes, with two exceptions:
>
> 1. sock_map_sk_state_allowed() maintains a fall-through `return true`.
> 2. In the spirit of commit b8b8315e39ff ("bpf, sockmap: Remove unhash
>    handler for BPF sockmap usage"), the proto::unhash BPF handler is not
>    reintroduced.
>
> Historical note: this issue is related to commit 67312adc96b5 ("bpf: reject
> unhashed sockets in bpf_sk_assign").
>
> Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Michal Luczaj]

On 6/29/26 13:37, Jakub Sitnicki wrote:
> On Sun, Jun 28, 2026 at 11:59 PM +02, Michal Luczaj wrote:
>> On 6/26/26 22:58, sashiko-bot@kernel.org wrote:
>>> [Severity: High]
>>> Could a memory ordering regression here allow unbound UDP sockets to still
>>> bypass this check?
>>>
>>> In udp_v4_get_port(), the flag and hash are set:
>>>
>>> net/ipv4/udp.c:udp_v4_get_port() {
>>>     ...
>>>     sock_set_flag(sk, SOCK_RCU_FREE);
>>>     sk_add_node_rcu(sk, &hslot->head);
>>>     ...
>>> }
>>>
>>> Because these stores lack a memory barrier between them, could sk_hashed(sk)
>>> return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
>>> the leak to still trigger?
>>
>> I'd like to verify it on a weakly-ordered CPU; please give me a day or two.
> 
> False positive, IMO.
> Both ->get_port and sock_map_sk_state_allowed run with sk_lock held.

What about an update coming from iter/task_file? You get an unlocked socket
via bpf_sock_from_file(ctx->file). And sock_map_update_elem()'s
bh_lock_sock(sk) doesn't care if socket is owned.

I was looking at iter/task_file in the context of [0]'s follow-up. With or
without [1], we're getting closer to enforcing (stronger) locking in
sock_map_update_elem(), per Martin's suggestion. Along the lines of

   if (!has_current_bpf_ctx() && sock_owned_by_user_nocheck(sk))
      ret = -EBUSY;
   else if (WARN_ON_ONCE(has_current_bpf_ctx() && !sock_owned_by_user_nocheck(sk)))
      ret = -EINVAL;
   ...

Would that fly?

[0]: https://lore.kernel.org/bpf/20260414-unix-proto-update-null-ptr-deref-v4-0-2af6fe97918e@rbox.co/
[1]: https://lore.kernel.org/bpf/20260629172704.1302218-1-rhkrqnwk98@gmail.com/

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Jakub Sitnicki]

On Mon, Jun 29, 2026 at 11:40 PM +02, Michal Luczaj wrote:
> On 6/29/26 13:37, Jakub Sitnicki wrote:
>> On Sun, Jun 28, 2026 at 11:59 PM +02, Michal Luczaj wrote:
>>> On 6/26/26 22:58, sashiko-bot@kernel.org wrote:
>>>> [Severity: High]
>>>> Could a memory ordering regression here allow unbound UDP sockets to still
>>>> bypass this check?
>>>>
>>>> In udp_v4_get_port(), the flag and hash are set:
>>>>
>>>> net/ipv4/udp.c:udp_v4_get_port() {
>>>>     ...
>>>>     sock_set_flag(sk, SOCK_RCU_FREE);
>>>>     sk_add_node_rcu(sk, &hslot->head);
>>>>     ...
>>>> }
>>>>
>>>> Because these stores lack a memory barrier between them, could sk_hashed(sk)
>>>> return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
>>>> the leak to still trigger?
>>>
>>> I'd like to verify it on a weakly-ordered CPU; please give me a day or two.
>> 
>> False positive, IMO.
>> Both ->get_port and sock_map_sk_state_allowed run with sk_lock held.
>
> What about an update coming from iter/task_file? You get an unlocked socket
> via bpf_sock_from_file(ctx->file). And sock_map_update_elem()'s
> bh_lock_sock(sk) doesn't care if socket is owned.

You're right. No exclusive access to socket there.

I think we should block map updates from all iterators but iter/sockmap,
where we know the socket is already in the correct state.

This is in the spirit of the API lockdown that is in progress [1]

[1] https://msgid.link/akRZ_e7DJh2aP-2i@john-p8

> I was looking at iter/task_file in the context of [0]'s follow-up. With or
> without [1], we're getting closer to enforcing (stronger) locking in
> sock_map_update_elem(), per Martin's suggestion. Along the lines of
>
>    if (!has_current_bpf_ctx() && sock_owned_by_user_nocheck(sk))
>       ret = -EBUSY;
>    else if (WARN_ON_ONCE(has_current_bpf_ctx() && !sock_owned_by_user_nocheck(sk)))
>       ret = -EINVAL;
>    ...
>
> Would that fly?
>
> [0]: https://lore.kernel.org/bpf/20260414-unix-proto-update-null-ptr-deref-v4-0-2af6fe97918e@rbox.co/
> [1]: https://lore.kernel.org/bpf/20260629172704.1302218-1-rhkrqnwk98@gmail.com/

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[John Fastabend]

On Wed, Jul 01, 2026 at 12:04:14PM +0200, Jakub Sitnicki wrote:
>On Mon, Jun 29, 2026 at 11:40 PM +02, Michal Luczaj wrote:
>> On 6/29/26 13:37, Jakub Sitnicki wrote:
>>> On Sun, Jun 28, 2026 at 11:59 PM +02, Michal Luczaj wrote:
>>>> On 6/26/26 22:58, sashiko-bot@kernel.org wrote:
>>>>> [Severity: High]
>>>>> Could a memory ordering regression here allow unbound UDP sockets to still
>>>>> bypass this check?
>>>>>
>>>>> In udp_v4_get_port(), the flag and hash are set:
>>>>>
>>>>> net/ipv4/udp.c:udp_v4_get_port() {
>>>>>     ...
>>>>>     sock_set_flag(sk, SOCK_RCU_FREE);
>>>>>     sk_add_node_rcu(sk, &hslot->head);
>>>>>     ...
>>>>> }
>>>>>
>>>>> Because these stores lack a memory barrier between them, could sk_hashed(sk)
>>>>> return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
>>>>> the leak to still trigger?
>>>>
>>>> I'd like to verify it on a weakly-ordered CPU; please give me a day or two.
>>>
>>> False positive, IMO.
>>> Both ->get_port and sock_map_sk_state_allowed run with sk_lock held.
>>
>> What about an update coming from iter/task_file? You get an unlocked socket
>> via bpf_sock_from_file(ctx->file). And sock_map_update_elem()'s
>> bh_lock_sock(sk) doesn't care if socket is owned.
>
>You're right. No exclusive access to socket there.
>
>I think we should block map updates from all iterators but iter/sockmap,
>where we know the socket is already in the correct state.
>
>This is in the spirit of the API lockdown that is in progress [1]

+1 lock this down to just iter/sockmap allowing these updates from
arbitrary context is too hard to reason about.

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[John Fastabend]

On Mon, Jun 29, 2026 at 01:38:00PM +0200, Jakub Sitnicki wrote:
>On Fri, Jun 26, 2026 at 10:36 PM +02, Michal Luczaj wrote:
>> UDP sockets get SOCK_RCU_FREE set when (auto-)bound. This means
>> sk_is_refcounted(unbound) = true, while sk_is_refcounted(bound) = false.
>>
>> Because sockmap accepts unbound UDP sockets, a BPF program can increment a
>> socket's refcount via lookup. If the socket is subsequently bound, the
>> transition from unbound to bound causes bpf_sk_release() to skip the
>> decrement of the refcount, causing a memory leak.
>>
>> unreferenced object 0xffff88810bc2eb40 (size 1984):
>>   comm "test_progs", pid 2451, jiffies 4295320596
>>   hex dump (first 32 bytes):
>>     7f 00 00 01 7f 00 00 01 d2 04 1b b7 04 d2 00 00  ................
>>     02 00 01 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
>>   backtrace (crc bdee079d):
>>     kmem_cache_alloc_noprof+0x557/0x660
>>     sk_prot_alloc+0x69/0x240
>>     sk_alloc+0x30/0x460
>>     inet_create+0x2ce/0xf80
>>     __sock_create+0x25b/0x5c0
>>     __sys_socket+0x119/0x1d0
>>     __x64_sys_socket+0x72/0xd0
>>     do_syscall_64+0xa1/0x5f0
>>     entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>
>> Instead of special-casing for refcounted sockets, reject unhashed UDP
>> sockets during sockmap updates, as there is no benefit to supporting those.
>> This effectively reverts the commit under Fixes, with two exceptions:
>>
>> 1. sock_map_sk_state_allowed() maintains a fall-through `return true`.
>> 2. In the spirit of commit b8b8315e39ff ("bpf, sockmap: Remove unhash
>>    handler for BPF sockmap usage"), the proto::unhash BPF handler is not
>>    reintroduced.
>>
>> Historical note: this issue is related to commit 67312adc96b5 ("bpf: reject
>> unhashed sockets in bpf_sk_assign").
>>
>> Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
>> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>
>Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

For me as well.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf v2 1/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[Michal Luczaj]

On 7/2/26 00:58, John Fastabend wrote:
> On Wed, Jul 01, 2026 at 12:04:14PM +0200, Jakub Sitnicki wrote:
>> On Mon, Jun 29, 2026 at 11:40 PM +02, Michal Luczaj wrote:
>>> On 6/29/26 13:37, Jakub Sitnicki wrote:
>>>> On Sun, Jun 28, 2026 at 11:59 PM +02, Michal Luczaj wrote:
>>>>> On 6/26/26 22:58, sashiko-bot@kernel.org wrote:
>>>>>> [Severity: High]
>>>>>> Could a memory ordering regression here allow unbound UDP sockets to still
>>>>>> bypass this check?
>>>>>>
>>>>>> In udp_v4_get_port(), the flag and hash are set:
>>>>>>
>>>>>> net/ipv4/udp.c:udp_v4_get_port() {
>>>>>>     ...
>>>>>>     sock_set_flag(sk, SOCK_RCU_FREE);
>>>>>>     sk_add_node_rcu(sk, &hslot->head);
>>>>>>     ...
>>>>>> }
>>>>>>
>>>>>> Because these stores lack a memory barrier between them, could sk_hashed(sk)
>>>>>> return true on another CPU while SOCK_RCU_FREE is not yet visible, allowing
>>>>>> the leak to still trigger?
>>>>>
>>>>> I'd like to verify it on a weakly-ordered CPU; please give me a day or two.
>>>>
>>>> False positive, IMO.
>>>> Both ->get_port and sock_map_sk_state_allowed run with sk_lock held.
>>>
>>> What about an update coming from iter/task_file? You get an unlocked socket
>>> via bpf_sock_from_file(ctx->file). And sock_map_update_elem()'s
>>> bh_lock_sock(sk) doesn't care if socket is owned.
>>
>> You're right. No exclusive access to socket there.
>>
>> I think we should block map updates from all iterators but iter/sockmap,
>> where we know the socket is already in the correct state.
>>
>> This is in the spirit of the API lockdown that is in progress [1]
> 
> +1 lock this down to just iter/sockmap allowing these updates from
> arbitrary context is too hard to reason about.

All right, I think this also addresses Sashiko's concern. I'll do it in a
separate series.

thanks,
Michal

Re: [PATCH bpf v2 4/4] selftests/bpf: Fail unbound UDP on sockmap update

[Michal Luczaj]

On 6/26/26 23:03, Kuniyuki Iwashima wrote:
> On Fri, Jun 26, 2026 at 1:37 PM Michal Luczaj <mhal@rbox.co> wrote:
>>
>> sockmap now rejects unbound UDP sockets. Adjust test_maps.
>>
>> This effectively reverts commit c39aa2159974 ("bpf, selftests: Fix
>> test_maps now that sockmap supports UDP").
>>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>>  tools/testing/selftests/bpf/test_maps.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
>> index c32da7bd8be2..81cd5d0d69c1 100644
>> --- a/tools/testing/selftests/bpf/test_maps.c
>> +++ b/tools/testing/selftests/bpf/test_maps.c
>> @@ -759,12 +759,12 @@ static void test_sockmap(unsigned int tasks, void *data)
>>                 goto out_sockmap;
>>         }
>>
>> -       /* Test update with unsupported UDP socket */
>> +       /* Test update with unsupported unbound UDP socket */
>>         udp = socket(AF_INET, SOCK_DGRAM, 0);
>>         i = 0;
>>         err = bpf_map_update_elem(fd, &i, &udp, BPF_ANY);
>> -       if (err) {
>> -               printf("Failed socket update SOCK_DGRAM '%i:%i'\n",
>> +       if (!err) {
>> +               printf("Failed allowed unbound SOCK_DGRAM socket update '%i:%i'\n",
> 
> nit: Maybe s/Failed/Unexpectedly succeeded/ ?

Sure. I've tried to align with the other printfs ("failed allowed..."), but
I agree it was unclear.

> If we want to avoid breakage, this patch needs to be squashed to
> the fix patch, but it's discouraged in netdev, not sure about bpf tree.

I guess I'll just post v3 as is, and squash if requested.

thanks,
Michal