[PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()

[PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()

[Chengfeng Ye]

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>
---
 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;
 		}
-- 
2.43.0

Re: [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()

[sashiko-bot]

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

Pre-existing issues:
- [High] Memory leak of stack-allocated `tmp` scatterlist pages when aborting via `goto out_err`.
- [High] Lost error code from `sk_msg_memcopy_from_iter` causes sendmsg to incorrectly return 0 instead of propagating -EFAULT.
- [High] Silent data loss on `__SK_REDIRECT` failure for corked messages due to masked error.
- [Medium] Overlapping `memcpy()` when corking an already-corked message.
--

--- Patch [1]: [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg() ---
commit ef0d38759a207a519b5d12273a55446155a7d5f4
Author: Chengfeng Ye <nicoyip.dev@gmail.com>

bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()

tcp_bpf_sendmsg() keeps msg_tx across sk_stream_wait_memory(), which drops
and reacquires the socket lock. This commit fixes a use-after-free where a
stale cork could be mistaken for the local temporary message and freed again.
It tests for tmp directly instead of comparing with psock->cork.

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6d..b9f0366333653 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -589,7 +589,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);

[Severity: High]
This isn't a bug introduced by this patch, but can this leak page references
or memory for the stack-allocated tmp?

If kzalloc_obj() for psock->cork fails in tcp_bpf_send_verdict(), returning
-ENOMEM but leaving psock->cork_bytes > 0, and userspace retries sendmsg(),
msg_tx defaults to the stack-allocated &tmp.

Data is copied into tmp, allocating pages. The code then hits:
if (psock->cork_bytes && !enospc)
    goto out_err;

And the out_err block returns without freeing tmp. Unprivileged userspace
could repeatedly trigger this state after a single induced -ENOMEM, leaking
pages on subsequent small sendmsg() calls.

>  			goto out_err;

[Severity: High]
This is a pre-existing issue, but does this goto out_err drop the
error code from sk_msg_memcopy_from_iter()?

Earlier in tcp_bpf_sendmsg():
ret = sk_msg_memcopy_from_iter(sk, &msg->msg_iter, msg_tx, copy);
if (ret < 0) {
    sk_msg_trim(sk, msg_tx, osize);
    goto out_err;
}

The out_err block returns err (which is initialized to 0), rather than ret.
If a local user passes a bad pointer, it will get a 0 return value instead
of -EFAULT, which usually causes userspace blocking socket writes to spin
endlessly.

>  		}

[Severity: Medium]
This isn't a bug introduced by this patch, but does tcp_bpf_send_verdict()
trigger an overlapping memory copy?

If a BPF program evaluates an already-corked message and assigns a new
cork_bytes threshold that exceeds the current size, tcp_bpf_send_verdict()
executes:

memcpy(psock->cork, msg, sizeof(*msg));

If msg is identical to psock->cork, this results in an overlapping memory
copy onto itself. This can violate the C standard and trigger UBSAN panics.

[Severity: High]
This is a pre-existing issue, but does tcp_bpf_send_verdict() silently
drop data on __SK_REDIRECT failure for corked messages?

If a redirected message is corked and tcp_bpf_sendmsg_redir() fails, the
code frees the message:

if (cork) {
    sk_msg_free(sk, msg);
    kfree(msg);
    msg = NULL;
    ret = 0;
}

This intentionally skips subtracting the freed bytes from *copied and
overwrites ret = 0. This causes tcp_bpf_sendmsg() to assume success,
continue processing the user's buffer, and return a positive byte count
to userspace despite dropping the corked data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719161630.2901208-1-nicoyip.dev@gmail.com?part=1

Re: [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()

[Emil Tsalapatis]

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;
>  		}

[PATCH v2] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

[Chengfeng Ye]

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

Re: [PATCH] bpf, sockmap: Fix cork use-after-free in tcp_bpf_sendmsg()

[Chengfeng Ye]

Dear Emil,

Thanks for your effort in reviewing the patch! I have sent a new v2 patch
to address the Sashiko comments on that function. I will wait until later to
see if any more related issues are flagged by Sashiko.

Re: [PATCH v2] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Unconditionally freeing the stack-allocated temporary message (`&tmp`) on the common exit path causes a Use-After-Free of corked pages.
- [High] Setting `*copied = 0` on failed redirects of a corked message wipes out the success count of independent messages sent earlier in the same syscall.
--

--- Patch [1]: [PATCH v2] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg() ---
commit e8fa0f17c7ce1143b0d6775fd8a3f303b4e73ff2
Author: Chengfeng Ye <nicoyip.dev@gmail.com>

bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

tcp_bpf_sendmsg() keeps msg_tx across sk_stream_wait_memory(), which drops
and reacquires the socket lock. This commit makes temporary ownership explicit
by freeing only &tmp at the common exit, resets cork_bytes when allocating
the persistent cork fails, propagates iterator errors, and clears the copied
count when corked data is discarded.

Link: https://lore.kernel.org/netdev/20260719161630.2901208-1-nicoyip.dev@gmail.com/ [v1]

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6d..ce06bc1cff2b6 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c

[ ... ]

> @@ -480,14 +482,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;

[Severity: High]
Does setting *copied to 0 here erase the accumulated byte count of any
independent messages that were successfully transmitted earlier in the same
sendmsg syscall?

In tcp_bpf_sendmsg(), the copied variable tracks the total bytes successfully
sent across multiple iterations of the while loop. Because copied is zeroed
when a corked message fails redirect, tcp_bpf_sendmsg() returns the error
code instead of the number of bytes successfully sent prior to the failure.

Will this cause userspace to assume the entire buffer failed to send and retry
sending the first parts, leading to silent data duplication on the network?

>  		}
>  		if (cork) {
>  			sk_msg_free(sk, msg);

[ ... ]

> @@ -588,13 +592,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);

[Severity: Critical]
Does unconditionally freeing the stack-allocated temporary message (&tmp) here
cause a use-after-free of corked pages?

When a new cork is initiated on the final iteration of tcp_bpf_sendmsg(),
msg_tx points to the local stack variable tmp. tcp_bpf_send_verdict()
decides to cork it, allocates psock->cork, and performs a shallow copy of
tmp via memcpy(). It returns 0 without clearing tmp.sg.size or tmp.sg.data.

When the while loop finishes, execution falls through to out_err. Because
msg_tx is still &tmp and its state was never cleared after the shallow copy,
sk_msg_free() iterates over the scatterlist and frees all elements.

Doesn't this leave psock->cork holding dangling pointers to these freed pages,
meaning a subsequent send or socket close will trigger a use-after-free?

>  	if (err < 0)
>  		err = sk_stream_error(sk, msg->msg_flags, err);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723153450.3224353-1-nicoyip.dev@gmail.com?part=1

[PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

[Chengfeng Ye]

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

Re: [PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

[sashiko-bot]

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

New issues:
- [High] ABA problem in pointer comparison causes `msg_copied` to retain stale values, leading to incorrect rollback of `copied`.

Pre-existing issues:
- [High] Incomplete fix leads to integer underflow in `copied` for `__SK_PASS` and `__SK_DROP` verdicts when a persistent cork fails.
- [High] Premature cork completion due to subtracting loop-invariant `size` instead of the copied amount `ret`.
--

--- Patch [1]: [PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg() ---
commit bd0bfd2d01fcc018a6a3e62db13f0096892e960b
Author: Chengfeng Ye <nicoyip.dev@gmail.com>

bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

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.

Link: https://lore.kernel.org/netdev/20260719161630.2901208-1-nicoyip.dev@gmail.com/

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6d..fcf535b7a7546 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -428,12 +430,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;
>  			}

[Severity: High]
This is a pre-existing issue, but does the __SK_PASS path need this same
msg_copied bounds check?

When the verdict evaluates to __SK_PASS but tcp_bpf_push() fails,
the error path unconditionally subtracts the full freed amount from copied:

tcp_bpf_send_verdict() {
...
        case __SK_PASS:
                ret = tcp_bpf_push(sk, msg, tosend, flags, true);
                if (unlikely(ret)) {
                        *copied -= sk_msg_free(sk, msg);
                        break;
                }
...
}

Similarly, does the __SK_DROP path have the same issue?

tcp_bpf_send_verdict() {
...
        case __SK_DROP:
        default:
                sk_msg_free(sk, msg);
                sk_msg_apply_bytes(psock, tosend);
                *copied -= (tosend + delta);
                return -EACCES;
}

If data accumulates in a persistent cork across multiple syscalls,
subtracting the full size here causes copied to underflow. Could this be
returned to userspace as a bogus error code or a massive positive return
value?

[ ... ]

> @@ -545,9 +555,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 {

[Severity: High]
Is this pointer comparison vulnerable to an ABA problem?

If sk_stream_wait_memory() is called later in this loop, it drops the
socket lock. While asleep, a concurrent thread could acquire the lock,
complete and free the existing psock->cork.

Another thread could then allocate a new psock->cork, and the slab allocator
might return the exact same memory address.

When this thread wakes up and reacquires the lock, the check
(msg_tx != psock->cork) would evaluate to false because the pointers match,
and msg_copied wouldn't be reset. On a subsequent error, could this cause
an artificially inflated msg_copied to be subtracted from the total copied
amount?

[ ... ]

> @@ -564,10 +577,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;

[Severity: High]
This is a pre-existing issue, but does the cork_bytes accounting use the
wrong size variable here?

tcp_bpf_sendmsg() {
...
                copied += ret;
                msg_copied += ret;
                if (psock->cork_bytes) {
                        if (size > psock->cork_bytes)
                                psock->cork_bytes = 0;
                        else
                                psock->cork_bytes -= size;
...
}

Here, size is the total loop-invariant syscall size, while ret is the
dynamically copied amount for this iteration. If ret < size (e.g., due to
scatterlist limits or memory exhaustion), subtracting size will deplete
cork_bytes faster than data is actually copied.

Could this cause premature cork completion and truncated messages being sent
to the BPF program?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723162602.3275661-1-nicoyip.dev@gmail.com?part=1

Re: [PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

[Jakub Kicinski]

On Fri, 24 Jul 2026 00:26:02 +0800 Chengfeng Ye wrote:
> Subject: [PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

Since you are spamming the list with the reposts of this could you
please add [PATCH bpf] to the subject so that netdev CI is not confused
into caring?

Re: [PATCH v3] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

[bot+bpf-ci]

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

> bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

The subject line is missing the [PATCH bpf] prefix. This helps netdev
CI correctly categorize the patch and prevents CI confusion when multiple
versions are posted in quick succession. Could the subject be updated to:

  [PATCH bpf] bpf, sockmap: Fix cork ownership in tcp_bpf_sendmsg()

(This issue was raised by Jakub Kicinski at
https://lore.kernel.org/bpf/20260723100030.3eee6d51@kernel.org/)


---
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/30028532303