* Re: [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()
2026-07-19 16:16 [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg() Chengfeng Ye
@ 2026-07-20 20:12 ` Emil Tsalapatis
2026-07-23 15:41 ` Chengfeng Ye
2026-07-23 15:34 ` [PATCH v2] bpf, sockmap: Fix cork ownership " Chengfeng Ye
2026-07-23 16:26 ` [PATCH v3] " Chengfeng Ye
2 siblings, 1 reply; 7+ messages in thread
From: Emil Tsalapatis @ 2026-07-20 20:12 UTC (permalink / raw)
To: Chengfeng Ye, Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima,
John Fastabend, Jakub Sitnicki, Jiayuan Chen, David S. Miller,
Jakub Kicinski, Paolo Abeni, Simon Horman, Alexei Starovoitov,
Daniel Borkmann, open list:BPF [L7 FRAMEWORK] (sockmap)
Cc: netdev, linux-kernel, stable
On Sun Jul 19, 2026 at 12:16 PM EDT, Chengfeng Ye wrote:
> tcp_bpf_sendmsg() keeps msg_tx across sk_stream_wait_memory(), which
> drops and reacquires the socket lock. Its error path tries to decide
> whether msg_tx names the local temporary message by comparing it with
> the current value of psock->cork.
>
> This comparison is unsafe when two threads send on the same socket:
>
> Thread A Thread B
> msg_tx = psock->cork
> sk_msg_alloc() fails
> sk_stream_wait_memory()
> releases the socket lock acquires the socket lock
> completes the cork
> psock->cork = NULL
> frees the cork
> reacquires the socket lock
> msg_tx != psock->cork
> sk_msg_free(msg_tx)
>
> The stale cork is therefore mistaken for the local temporary message
> and freed again. KASAN reported:
>
> BUG: KASAN: slab-use-after-free in sk_msg_free+0x49/0x50
> Read of size 4 at addr ffff88810c908800 by task poc/90
> Call Trace:
> sk_msg_free+0x49/0x50
> tcp_bpf_sendmsg+0x14f5/0x1cc0
> __sys_sendto+0x32c/0x3a0
> __x64_sys_sendto+0xdb/0x1b0
> Allocated by task 89:
> __kasan_kmalloc+0x8f/0xa0
> tcp_bpf_sendmsg+0x16b3/0x1cc0
> Freed by task 91:
> __kasan_slab_free+0x43/0x70
> kfree+0x131/0x3c0
> tcp_bpf_sendmsg+0xec3/0x1cc0
>
> msg_tx can only name the stack-local tmp or the shared cork. Test for
> tmp directly so a changed psock->cork cannot turn a shared message into
> an apparent local one.
>
> Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
Hi Chengfeng,
The patch looks good:
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
There is one caveat: Normally we ignore pre-existing issues Sashiko
finds while reviewing the patch that are unrelated to the change itself.
For this function, however, I think we should make an exception because
it has multiple glaring issues we can fix more cleanly if we do it all
at once. E.g., tmp never gets cleaned up even if there are allocations
hanging off of it.
Would you be willing to expand the patch that addresses the Sashiko
comments, even if unrelated to your fix? That would save us the time
to review the inevitable followups and provide more coherent
refactoring.
> net/ipv4/tcp_bpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 8e905b50dead..a30475afb6f8 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -604,7 +604,7 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> wait_for_memory:
> err = sk_stream_wait_memory(sk, &timeo);
> if (err) {
> - if (msg_tx && msg_tx != psock->cork)
> + if (msg_tx == &tmp)
> sk_msg_free(sk, msg_tx);
> goto out_err;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()
2026-07-19 16:16 [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg() Chengfeng Ye
2026-07-20 20:12 ` Emil Tsalapatis
@ 2026-07-23 15:34 ` Chengfeng Ye
2026-07-23 16:26 ` [PATCH v3] " Chengfeng Ye
2 siblings, 0 replies; 7+ messages in thread
From: Chengfeng Ye @ 2026-07-23 15:34 UTC (permalink / raw)
To: Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima, John Fastabend,
Jakub Sitnicki, Jiayuan Chen, David S. Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, linux-kernel, Chengfeng Ye, stable, Emil Tsalapatis
tcp_bpf_sendmsg() keeps msg_tx across sk_stream_wait_memory(), which drops
and reacquires the socket lock. Its error path used the current value of
psock->cork to decide whether msg_tx named the stack-local temporary
message.
Two senders can therefore interleave as follows:
Thread A Thread B
msg_tx = psock->cork
sk_msg_alloc() fails
sk_stream_wait_memory()
releases the socket lock acquires the socket lock
completes the cork
psock->cork = NULL
frees the cork
reacquires the socket lock
msg_tx != psock->cork
sk_msg_free(msg_tx)
The stale cork is mistaken for the local temporary message and freed again.
KASAN reported:
BUG: KASAN: slab-use-after-free in sk_msg_free+0x49/0x50
Read of size 4 at addr ffff88810c908800 by task poc/90
Call Trace:
sk_msg_free+0x49/0x50
tcp_bpf_sendmsg+0x14f5/0x1cc0
__sys_sendto+0x32c/0x3a0
__x64_sys_sendto+0xdb/0x1b0
Allocated by task 89:
__kasan_kmalloc+0x8f/0xa0
tcp_bpf_sendmsg+0x16b3/0x1cc0
Freed by task 91:
__kasan_slab_free+0x43/0x70
kfree+0x131/0x3c0
tcp_bpf_sendmsg+0xec3/0x1cc0
The same unclear ownership also leaves several error paths inconsistent.
A failed cork allocation leaves cork_bytes armed, a subsequent temporary
message can escape without releasing its pages, iterator errors are
returned as success, and a failed redirect of a corked message discards
data while reporting it as sent. Re-evaluating an existing cork can also
copy the object onto itself.
Make temporary ownership explicit by freeing only &tmp at the common exit.
Reset cork_bytes when allocating the persistent cork fails, propagate
iterator errors, preserve redirect failures and clear the copied count
when corked data is discarded. Skip the copy when the message is already
the persistent cork.
Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Cc: stable@vger.kernel.org
Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v2:
- Address Sashiko review findings for stale cork state, iterator-error
propagation, cork self-copy, and masked redirect failures.
- Keep the original stale-cork use-after-free fix.
Sashiko: https://sashiko.dev/#/patchset/20260719161630.2901208-1-nicoyip.dev%40gmail.com
Link: https://lore.kernel.org/netdev/20260719161630.2901208-1-nicoyip.dev@gmail.com/ [v1]
net/ipv4/tcp_bpf.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 8e905b50dead..0594ee4013b0 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -443,12 +443,14 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
psock->cork = kzalloc_obj(*psock->cork,
GFP_ATOMIC | __GFP_NOWARN);
if (!psock->cork) {
+ psock->cork_bytes = 0;
sk_msg_free(sk, msg);
*copied = 0;
return -ENOMEM;
}
}
- memcpy(psock->cork, msg, sizeof(*msg));
+ if (psock->cork != msg)
+ memcpy(psock->cork, msg, sizeof(*msg));
return 0;
}
@@ -495,14 +497,15 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
if (unlikely(ret < 0)) {
int free = sk_msg_free(sk, msg);
- if (!cork)
+ if (cork)
+ *copied = 0;
+ else
*copied -= free;
}
if (cork) {
sk_msg_free(sk, msg);
kfree(msg);
msg = NULL;
- ret = 0;
}
break;
case __SK_DROP:
@@ -579,6 +582,7 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
copy);
if (ret < 0) {
sk_msg_trim(sk, msg_tx, osize);
+ err = ret;
goto out_err;
}
@@ -603,13 +607,12 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
wait_for_memory:
err = sk_stream_wait_memory(sk, &timeo);
- if (err) {
- if (msg_tx && msg_tx != psock->cork)
- sk_msg_free(sk, msg_tx);
+ if (err)
goto out_err;
- }
}
out_err:
+ if (msg_tx == &tmp)
+ sk_msg_free(sk, msg_tx);
if (err < 0)
err = sk_stream_error(sk, msg->msg_flags, err);
release_sock(sk);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()
2026-07-19 16:16 [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg() Chengfeng Ye
2026-07-20 20:12 ` Emil Tsalapatis
2026-07-23 15:34 ` [PATCH v2] bpf, sockmap: Fix cork ownership " Chengfeng Ye
@ 2026-07-23 16:26 ` Chengfeng Ye
2026-07-23 17:00 ` Jakub Kicinski
2026-07-23 17:44 ` bot+bpf-ci
2 siblings, 2 replies; 7+ messages in thread
From: Chengfeng Ye @ 2026-07-23 16:26 UTC (permalink / raw)
To: Emil Tsalapatis, Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima,
John Fastabend, Jakub Sitnicki, Jiayuan Chen, David S. Miller,
Jakub Kicinski, Paolo Abeni, Simon Horman, Alexei Starovoitov,
Daniel Borkmann
Cc: netdev, bpf, linux-kernel, Chengfeng Ye, stable
tcp_bpf_sendmsg() keeps msg_tx across sk_stream_wait_memory(), which drops
and reacquires the socket lock. Its error path used the current value of
psock->cork to decide whether msg_tx named the stack-local temporary
message.
Two senders can therefore interleave as follows:
Thread A Thread B
msg_tx = psock->cork
sk_msg_alloc() fails
sk_stream_wait_memory()
releases the socket lock acquires the socket lock
completes the cork
psock->cork = NULL
frees the cork
reacquires the socket lock
msg_tx != psock->cork
sk_msg_free(msg_tx)
The stale cork is mistaken for the local temporary message and freed again.
KASAN reported:
BUG: KASAN: slab-use-after-free in sk_msg_free+0x49/0x50
Read of size 4 at addr ffff88810c908800 by task poc/90
Call Trace:
sk_msg_free+0x49/0x50
tcp_bpf_sendmsg+0x14f5/0x1cc0
__sys_sendto+0x32c/0x3a0
__x64_sys_sendto+0xdb/0x1b0
Allocated by task 89:
__kasan_kmalloc+0x8f/0xa0
tcp_bpf_sendmsg+0x16b3/0x1cc0
Freed by task 91:
__kasan_slab_free+0x43/0x70
kfree+0x131/0x3c0
tcp_bpf_sendmsg+0xec3/0x1cc0
Make temporary ownership explicit by freeing only the stack-local message
at the common exit. When a verdict moves that message into the persistent
cork, use sk_msg_xfer_full() to clear the source and record the persistent
cork as the current owner.
The related failure paths must also distinguish bytes in the current
message from bytes sent by earlier loop iterations. Track the former in
msg_copied. If cork allocation or redirect fails, subtract only the
unsent bytes belonging to that message, preserving the syscall-wide count
for data already sent. Also reset cork_bytes after allocation failure,
skip a self-transfer of an existing cork, and propagate iterator errors.
Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Cc: stable@vger.kernel.org
Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v3:
- Transfer the temporary message into the persistent cork with
sk_msg_xfer_full(), leaving the source empty at the common exit.
- Track the current message contribution separately so cork failures retain
the count of bytes sent by earlier iterations.
- Address the two ownership and return-value issues reported by Sashiko.
Changes in v2:
- Address Sashiko review findings for stale cork state, iterator-error
propagation, cork self-copy, and masked redirect failures.
- Keep the original stale-cork use-after-free fix.
Sashiko: https://sashiko.dev/#/patchset/20260719161630.2901208-1-nicoyip.dev%40gmail.com
Link: https://lore.kernel.org/netdev/20260719161630.2901208-1-nicoyip.dev@gmail.com/ [v1]
net/ipv4/tcp_bpf.c | 41 +++++++++++++++++++++++++++++------------
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 8e905b50dead..fbb11b5abcd4 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -416,7 +416,9 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
}
static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
- struct sk_msg *msg, int *copied, int flags)
+ struct sk_msg *msg, int *copied,
+ u32 msg_copied, bool *corked,
+ int flags)
{
bool cork = false, enospc = sk_msg_full(msg), redir_ingress;
struct sock *sk_redir;
@@ -443,12 +445,18 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
psock->cork = kzalloc_obj(*psock->cork,
GFP_ATOMIC | __GFP_NOWARN);
if (!psock->cork) {
- sk_msg_free(sk, msg);
- *copied = 0;
+ int free;
+
+ psock->cork_bytes = 0;
+ free = sk_msg_free(sk, msg);
+ *copied -= min_t(u32, msg_copied, free);
return -ENOMEM;
}
}
- memcpy(psock->cork, msg, sizeof(*msg));
+ if (psock->cork != msg) {
+ sk_msg_xfer_full(psock->cork, msg);
+ *corked = true;
+ }
return 0;
}
@@ -495,14 +503,15 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
if (unlikely(ret < 0)) {
int free = sk_msg_free(sk, msg);
- if (!cork)
+ if (cork)
+ *copied -= min_t(u32, msg_copied, free);
+ else
*copied -= free;
}
if (cork) {
sk_msg_free(sk, msg);
kfree(msg);
msg = NULL;
- ret = 0;
}
break;
case __SK_DROP:
@@ -534,6 +543,7 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
struct sk_msg tmp, *msg_tx = NULL;
int copied = 0, err = 0, ret = 0;
struct sk_psock *psock;
+ u32 msg_copied = 0;
long timeo;
int flags;
@@ -548,7 +558,7 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
lock_sock(sk);
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
while (msg_data_left(msg)) {
- bool enospc = false;
+ bool corked = false, enospc = false;
u32 copy, osize;
if (sk->sk_err) {
@@ -560,9 +570,12 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
if (!sk_stream_memory_free(sk))
goto wait_for_sndbuf;
if (psock->cork) {
+ if (msg_tx != psock->cork)
+ msg_copied = 0;
msg_tx = psock->cork;
} else {
msg_tx = &tmp;
+ msg_copied = 0;
sk_msg_init(msg_tx);
}
@@ -579,10 +592,12 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
copy);
if (ret < 0) {
sk_msg_trim(sk, msg_tx, osize);
+ err = ret;
goto out_err;
}
copied += ret;
+ msg_copied += ret;
if (psock->cork_bytes) {
if (size > psock->cork_bytes)
psock->cork_bytes = 0;
@@ -595,21 +610,23 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
psock->cork_bytes = 0;
}
- err = tcp_bpf_send_verdict(sk, psock, msg_tx, &copied, flags);
+ err = tcp_bpf_send_verdict(sk, psock, msg_tx, &copied,
+ msg_copied, &corked, flags);
if (unlikely(err < 0))
goto out_err;
+ if (corked)
+ msg_tx = psock->cork;
continue;
wait_for_sndbuf:
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
wait_for_memory:
err = sk_stream_wait_memory(sk, &timeo);
- if (err) {
- if (msg_tx && msg_tx != psock->cork)
- sk_msg_free(sk, msg_tx);
+ if (err)
goto out_err;
- }
}
out_err:
+ if (msg_tx == &tmp)
+ sk_msg_free(sk, msg_tx);
if (err < 0)
err = sk_stream_error(sk, msg->msg_flags, err);
release_sock(sk);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread