BPF List
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: fix UAF in sock clone early bailouts
@ 2026-07-09  2:53 Matt Bobrowski
  2026-07-09  3:20 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Matt Bobrowski @ 2026-07-09  2:53 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	Jiri Olsa, jannh, Matt Bobrowski, stable

Similar to recent commit 9b51a6155d14 ("bpf,fork: wipe ->bpf_storage
before bailouts that access it"), sk_clone() performs an initial
shallow copy of the socket field ->sk_bpf_storage via sock_copy() for
the cloned socket newsk.

If sk_clone() bails out early (e.g. if sk_filter_charge() fails) prior
to calling bpf_sk_storage_clone(), newsk->sk_bpf_storage still points
to the parent socket's BPF local storage. When newsk is subsequently
freed via sk_free(), the deallocation path (__sk_destruct() ->
bpf_sk_storage_free()) destroys the parent socket's BPF local storage,
leading to a use-after-free (UAF) on the parent socket.

Fix this by resetting newsk->sk_bpf_storage to NULL immediately after
sock_copy() in sk_clone(), and remove the now redundant initialization
from bpf_sk_storage_clone().

Cc: stable@vger.kernel.org
Fixes: 6ac99e8f23d4 ("bpf: Introduce bpf sk local storage")
Fixes: f12dd75959b0 ("bpf: net: Set sk_bpf_storage back to NULL for cloned sk")
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 net/core/bpf_sk_storage.c | 2 --
 net/core/sock.c           | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index ecd659f79fd4..1d295a8769fa 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -158,8 +158,6 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
 	struct bpf_local_storage_elem *selem;
 	int ret = 0;
 
-	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
-
 	rcu_read_lock_dont_migrate();
 	sk_storage = rcu_dereference(sk->sk_bpf_storage);
 
diff --git a/net/core/sock.c b/net/core/sock.c
index 8a59bfaa8096..498a57f34f5b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2492,6 +2492,9 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 	sock_copy(newsk, sk);
 
 	newsk->sk_prot_creator = prot;
+#ifdef CONFIG_BPF_SYSCALL
+	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
+#endif
 
 	/* SANITY */
 	if (likely(newsk->sk_net_refcnt)) {
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: fix UAF in sock clone early bailouts
  2026-07-09  2:53 [PATCH bpf] bpf: fix UAF in sock clone early bailouts Matt Bobrowski
@ 2026-07-09  3:20 ` sashiko-bot
  2026-07-09 11:55   ` Daniel Borkmann
  2026-07-09 11:53 ` Daniel Borkmann
  2026-07-09 17:00 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-07-09  3:20 UTC (permalink / raw)
  To: Matt Bobrowski; +Cc: bpf

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

Pre-existing issues:
- [High] BPF map corruption via shallow-copied sk_user_data and sk_reuseport_cb during sk_clone early bailouts.
- [High] Unbalanced decrement of global network timestamp static key during sk_clone early bailouts.
--

--- Patch [1]: [PATCH bpf] bpf: fix UAF in sock clone early bailouts ---
commit 81577dfc9f1b10000a3b1dfc19a5b577e986ec33
Author: Matt Bobrowski <mattbobrowski@google.com>

bpf: fix UAF in sock clone early bailouts

This commit fixes a use-after-free on parent socket's BPF local storage
caused by an early bailout in sk_clone() prior to clearing
newsk->sk_bpf_storage. By resetting newsk->sk_bpf_storage to NULL
immediately after sock_copy(), the deallocation path won't destroy the
parent socket's storage.

> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8a59bfaa8096e..498a57f34f5b5 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2492,6 +2492,9 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
>  	sock_copy(newsk, sk);
>  
>  	newsk->sk_prot_creator = prot;
> +#ifdef CONFIG_BPF_SYSCALL
> +	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
> +#endif

[Severity: High]
This is a pre-existing issue, but I noticed there might be a similar issue
with sk_reuseport_cb and sk_user_data. Since sock_copy() does a shallow
copy, newsk initially points to the parent's sk_reuseport_cb and
sk_user_data.

If sk_clone() bails out early (e.g., sk_filter_charge() fails):

net/core/sock.c:sk_clone() {
    ...
    if (unlikely(!is_charged || xfrm_sk_clone_policy(newsk, sk))) {
        ...
        goto free;
    }

    RCU_INIT_POINTER(newsk->sk_reuseport_cb, NULL);
    ...
}

The bailout happens before sk_reuseport_cb is cleared. When newsk is freed,
bpf_sk_reuseport_detach() can follow the copied sk_user_data to the parent's
BPF map slot and write NULL to it:

kernel/bpf/reuseport_array.c:bpf_sk_reuseport_detach() {
    ...
    RCU_INIT_POINTER(*socks, NULL);
    ...
}

Does this corrupt the parent socket's BPF map during early bailouts?


[Severity: High]
This is also a pre-existing issue, but could the same early bailout pattern
cause an unbalanced decrement of the global network timestamp static key?

sock_copy() copies SK_FLAGS_TIMESTAMP into newsk->sk_flags. The global
timestamp counter is only incremented at the end of sk_clone():

net/core/sock.c:sk_clone() {
    ...
    if (sock_needs_netstamp(sk) && newsk->sk_flags & SK_FLAGS_TIMESTAMP)
        net_enable_timestamp();
}

However, during destruction of the aborted clone, the global counter is
decremented unconditionally because the copied flag was set:

net/core/sock.c:__sk_destruct() {
    ...
    sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
    ...
}

Could an early bailout skip the net_enable_timestamp() call but still
execute the sock_disable_timestamp() call on free, leading to an underflow?

>  	/* SANITY */
>  	if (likely(newsk->sk_net_refcnt)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709025316.999913-1-mattbobrowski@google.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: fix UAF in sock clone early bailouts
  2026-07-09  2:53 [PATCH bpf] bpf: fix UAF in sock clone early bailouts Matt Bobrowski
  2026-07-09  3:20 ` sashiko-bot
@ 2026-07-09 11:53 ` Daniel Borkmann
  2026-07-09 16:18   ` Kuniyuki Iwashima
  2026-07-09 17:00 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2026-07-09 11:53 UTC (permalink / raw)
  To: Matt Bobrowski, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, Jiri Olsa, jannh,
	stable, Kuniyuki Iwashima

[ +Kuniyuki ]

On 7/9/26 4:53 AM, Matt Bobrowski wrote:
> Similar to recent commit 9b51a6155d14 ("bpf,fork: wipe ->bpf_storage
> before bailouts that access it"), sk_clone() performs an initial
> shallow copy of the socket field ->sk_bpf_storage via sock_copy() for
> the cloned socket newsk.
> 
> If sk_clone() bails out early (e.g. if sk_filter_charge() fails) prior
> to calling bpf_sk_storage_clone(), newsk->sk_bpf_storage still points
> to the parent socket's BPF local storage. When newsk is subsequently
> freed via sk_free(), the deallocation path (__sk_destruct() ->
> bpf_sk_storage_free()) destroys the parent socket's BPF local storage,
> leading to a use-after-free (UAF) on the parent socket.
> 
> Fix this by resetting newsk->sk_bpf_storage to NULL immediately after
> sock_copy() in sk_clone(), and remove the now redundant initialization
> from bpf_sk_storage_clone().
> 
> Cc: stable@vger.kernel.org
> Fixes: 6ac99e8f23d4 ("bpf: Introduce bpf sk local storage")
> Fixes: f12dd75959b0 ("bpf: net: Set sk_bpf_storage back to NULL for cloned sk")
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>

LGTM, thanks!

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

>   net/core/bpf_sk_storage.c | 2 --
>   net/core/sock.c           | 3 +++
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index ecd659f79fd4..1d295a8769fa 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -158,8 +158,6 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
>   	struct bpf_local_storage_elem *selem;
>   	int ret = 0;
>   
> -	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
> -
>   	rcu_read_lock_dont_migrate();
>   	sk_storage = rcu_dereference(sk->sk_bpf_storage);
>   
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8a59bfaa8096..498a57f34f5b 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2492,6 +2492,9 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
>   	sock_copy(newsk, sk);
>   
>   	newsk->sk_prot_creator = prot;
> +#ifdef CONFIG_BPF_SYSCALL
> +	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
> +#endif
>   
>   	/* SANITY */
>   	if (likely(newsk->sk_net_refcnt)) {


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: fix UAF in sock clone early bailouts
  2026-07-09  3:20 ` sashiko-bot
@ 2026-07-09 11:55   ` Daniel Borkmann
  2026-07-09 16:20     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2026-07-09 11:55 UTC (permalink / raw)
  To: sashiko-reviews, Matt Bobrowski; +Cc: bpf, Kuniyuki Iwashima

Hi Matt, hi Kuniyuki,

On 7/9/26 5:20 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] BPF map corruption via shallow-copied sk_user_data and sk_reuseport_cb during sk_clone early bailouts.
> - [High] Unbalanced decrement of global network timestamp static key during sk_clone early bailouts.

Do either one of you also have a chance to look into these?

Thanks a lot,
Daniel

> --- Patch [1]: [PATCH bpf] bpf: fix UAF in sock clone early bailouts ---
> commit 81577dfc9f1b10000a3b1dfc19a5b577e986ec33
> Author: Matt Bobrowski <mattbobrowski@google.com>
> 
> bpf: fix UAF in sock clone early bailouts
> 
> This commit fixes a use-after-free on parent socket's BPF local storage
> caused by an early bailout in sk_clone() prior to clearing
> newsk->sk_bpf_storage. By resetting newsk->sk_bpf_storage to NULL
> immediately after sock_copy(), the deallocation path won't destroy the
> parent socket's storage.
> 
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 8a59bfaa8096e..498a57f34f5b5 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -2492,6 +2492,9 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
>>   	sock_copy(newsk, sk);
>>   
>>   	newsk->sk_prot_creator = prot;
>> +#ifdef CONFIG_BPF_SYSCALL
>> +	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
>> +#endif
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed there might be a similar issue
> with sk_reuseport_cb and sk_user_data. Since sock_copy() does a shallow
> copy, newsk initially points to the parent's sk_reuseport_cb and
> sk_user_data.
> 
> If sk_clone() bails out early (e.g., sk_filter_charge() fails):
> 
> net/core/sock.c:sk_clone() {
>      ...
>      if (unlikely(!is_charged || xfrm_sk_clone_policy(newsk, sk))) {
>          ...
>          goto free;
>      }
> 
>      RCU_INIT_POINTER(newsk->sk_reuseport_cb, NULL);
>      ...
> }
> 
> The bailout happens before sk_reuseport_cb is cleared. When newsk is freed,
> bpf_sk_reuseport_detach() can follow the copied sk_user_data to the parent's
> BPF map slot and write NULL to it:
> 
> kernel/bpf/reuseport_array.c:bpf_sk_reuseport_detach() {
>      ...
>      RCU_INIT_POINTER(*socks, NULL);
>      ...
> }
> 
> Does this corrupt the parent socket's BPF map during early bailouts?
> 
> 
> [Severity: High]
> This is also a pre-existing issue, but could the same early bailout pattern
> cause an unbalanced decrement of the global network timestamp static key?
> 
> sock_copy() copies SK_FLAGS_TIMESTAMP into newsk->sk_flags. The global
> timestamp counter is only incremented at the end of sk_clone():
> 
> net/core/sock.c:sk_clone() {
>      ...
>      if (sock_needs_netstamp(sk) && newsk->sk_flags & SK_FLAGS_TIMESTAMP)
>          net_enable_timestamp();
> }
> 
> However, during destruction of the aborted clone, the global counter is
> decremented unconditionally because the copied flag was set:
> 
> net/core/sock.c:__sk_destruct() {
>      ...
>      sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
>      ...
> }
> 
> Could an early bailout skip the net_enable_timestamp() call but still
> execute the sock_disable_timestamp() call on free, leading to an underflow?
> 
>>   	/* SANITY */
>>   	if (likely(newsk->sk_net_refcnt)) {
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: fix UAF in sock clone early bailouts
  2026-07-09 11:53 ` Daniel Borkmann
@ 2026-07-09 16:18   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2026-07-09 16:18 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Matt Bobrowski, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	Jiri Olsa, jannh, stable

On Thu, Jul 9, 2026 at 4:53 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> [ +Kuniyuki ]
>
> On 7/9/26 4:53 AM, Matt Bobrowski wrote:
> > Similar to recent commit 9b51a6155d14 ("bpf,fork: wipe ->bpf_storage
> > before bailouts that access it"), sk_clone() performs an initial
> > shallow copy of the socket field ->sk_bpf_storage via sock_copy() for
> > the cloned socket newsk.
> >
> > If sk_clone() bails out early (e.g. if sk_filter_charge() fails) prior
> > to calling bpf_sk_storage_clone(), newsk->sk_bpf_storage still points
> > to the parent socket's BPF local storage. When newsk is subsequently
> > freed via sk_free(), the deallocation path (__sk_destruct() ->
> > bpf_sk_storage_free()) destroys the parent socket's BPF local storage,
> > leading to a use-after-free (UAF) on the parent socket.
> >
> > Fix this by resetting newsk->sk_bpf_storage to NULL immediately after
> > sock_copy() in sk_clone(), and remove the now redundant initialization
> > from bpf_sk_storage_clone().
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 6ac99e8f23d4 ("bpf: Introduce bpf sk local storage")
> > Fixes: f12dd75959b0 ("bpf: net: Set sk_bpf_storage back to NULL for cloned sk")
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
>
> LGTM, thanks!
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: fix UAF in sock clone early bailouts
  2026-07-09 11:55   ` Daniel Borkmann
@ 2026-07-09 16:20     ` Kuniyuki Iwashima
  0 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2026-07-09 16:20 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: sashiko-reviews, Matt Bobrowski, bpf

On Thu, Jul 9, 2026 at 4:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Hi Matt, hi Kuniyuki,
>
> On 7/9/26 5:20 AM, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> >
> > Pre-existing issues:
> > - [High] BPF map corruption via shallow-copied sk_user_data and sk_reuseport_cb during sk_clone early bailouts.
> > - [High] Unbalanced decrement of global network timestamp static key during sk_clone early bailouts.
>
> Do either one of you also have a chance to look into these?

I'll post a fix.

Thank you, Daniel !

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: fix UAF in sock clone early bailouts
  2026-07-09  2:53 [PATCH bpf] bpf: fix UAF in sock clone early bailouts Matt Bobrowski
  2026-07-09  3:20 ` sashiko-bot
  2026-07-09 11:53 ` Daniel Borkmann
@ 2026-07-09 17:00 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-07-09 17:00 UTC (permalink / raw)
  To: Matt Bobrowski
  Cc: bpf, ast, daniel, andrii, martin.lau, eddyz87, song,
	yonghong.song, jolsa, jannh, stable

Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu,  9 Jul 2026 02:53:16 +0000 you wrote:
> Similar to recent commit 9b51a6155d14 ("bpf,fork: wipe ->bpf_storage
> before bailouts that access it"), sk_clone() performs an initial
> shallow copy of the socket field ->sk_bpf_storage via sock_copy() for
> the cloned socket newsk.
> 
> If sk_clone() bails out early (e.g. if sk_filter_charge() fails) prior
> to calling bpf_sk_storage_clone(), newsk->sk_bpf_storage still points
> to the parent socket's BPF local storage. When newsk is subsequently
> freed via sk_free(), the deallocation path (__sk_destruct() ->
> bpf_sk_storage_free()) destroys the parent socket's BPF local storage,
> leading to a use-after-free (UAF) on the parent socket.
> 
> [...]

Here is the summary with links:
  - [bpf] bpf: fix UAF in sock clone early bailouts
    https://git.kernel.org/bpf/bpf/c/7cbd0c4cebe4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-09 17:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09  2:53 [PATCH bpf] bpf: fix UAF in sock clone early bailouts Matt Bobrowski
2026-07-09  3:20 ` sashiko-bot
2026-07-09 11:55   ` Daniel Borkmann
2026-07-09 16:20     ` Kuniyuki Iwashima
2026-07-09 11:53 ` Daniel Borkmann
2026-07-09 16:18   ` Kuniyuki Iwashima
2026-07-09 17:00 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox