* [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