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

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 593

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

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 609

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

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 595

[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/[email protected]?part=1

net/ipv4/tcp_bpf.c static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, · line 488

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

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 600

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

net/ipv4/tcp_bpf.c static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, · line 439

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

[ ... ]

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 561

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

[ ... ]

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 588

[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/[email protected]?part=1

net/ipv4/tcp_bpf.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but could this leak the tmp message and cause silent data loss if an earlier cork allocation fails?

In tcp_bpf_send_verdict(), psock->cork_bytes is updated before allocating psock->cork:

net/ipv4/tcp_bpf.c:tcp_bpf_send_verdict() {
    ...
	if (msg->cork_bytes &&
	    msg->cork_bytes > msg->sg.size && !enospc) {
		psock->cork_bytes = msg->cork_bytes - msg->sg.size;
		if (!psock->cork) {
			psock->cork = kzalloc_obj(*psock->cork, ...);
			if (!psock->cork) {
				sk_msg_free(sk, msg);
				*copied = 0;
				return -ENOMEM;
			}
    ...
}

If the allocation fails, psock->cork_bytes remains greater than zero.

Then, on a subsequent sendmsg(), tcp_bpf_sendmsg() populates the stack-local tmp message, sees the non-zero psock->cork_bytes, and jumps directly to out_err:

net/ipv4/tcp_bpf.c:tcp_bpf_sendmsg() {
    ...
		if (psock->cork_bytes) {
			if (size > psock->cork_bytes)
				psock->cork_bytes = 0;
			else
				psock->cork_bytes -= size;
			if (psock->cork_bytes && !enospc)
				goto out_err;
    ...
}

Because it bypasses the error path cleanup below, doesn't this permanently leak the tmp message's page fragments and silently drop the data?

[Severity: High]
This is a pre-existing issue, but does appending to a cork execute undefined behavior by calling memcpy() with identical pointers?

In tcp_bpf_sendmsg(), if psock->cork is already allocated, msg_tx is set to point to it and passed to tcp_bpf_send_verdict() as the msg argument. If the BPF program requests further corking, the code does this:

net/ipv4/tcp_bpf.c:tcp_bpf_send_verdict() {
    ...
		if (!psock->cork) {
			psock->cork = kzalloc_obj(*psock->cork, ...);
            ...
		}
		memcpy(psock->cork, msg, sizeof(*msg));
		return 0;
    ...
}

Since msg is exactly psock->cork here, does this result in a memcpy() with identical source and destination pointers, potentially causing a kernel panic with FORTIFY_SOURCE or KASAN?

[Severity: High]
This is a pre-existing issue, but is there a use-after-free on sk_redir when multiple threads send on the same socket concurrently?

When a BPF program returns __SK_REDIRECT with apply_bytes > 0, sk_psock_msg_verdict() takes a single reference on the target socket via sock_hold(psock->sk_redir).

If multiple threads process the remaining apply_bytes for this socket, they all read psock->sk_redir and release the socket lock to call tcp_bpf_sendmsg_redir():

net/ipv4/tcp_bpf.c:tcp_bpf_send_verdict() {
    ...
	case __SK_REDIRECT:
		redir_ingress = psock->redir_ingress;
		sk_redir = psock->sk_redir;
		sk_msg_apply_bytes(psock, tosend);
		if (!psock->apply_bytes) {
			/* Clean up before releasing the sock lock. */
			eval = psock->eval;
			psock->eval = __SK_NONE;
			psock->sk_redir = NULL;
		}
        ...
		release_sock(sk);

		origsize = msg->sg.size;
		ret = tcp_bpf_sendmsg_redir(sk_redir, redir_ingress, ...);
		sent = origsize - msg->sg.size;

		if (eval == __SK_REDIRECT)
			sock_put(sk_redir);
    ...
}

The thread that exhausts apply_bytes clears psock->sk_redir and calls sock_put(sk_redir) to drop the single reference. Since it doesn't wait for other concurrent threads to finish, could the target socket be freed while they are still executing tcp_bpf_sendmsg_redir(sk_redir)?

[ ... ]

net/ipv4/tcp_bpf.c static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) · line 604

This patch folds several distinct changes into one: the stale-cork use-after-free fix at the common exit, a cork_bytes reset on the cork allocation failure path, the new msg_copied accounting for the failure paths, the switch from memcpy() to sk_msg_xfer_full() for the cork transfer, and iterator error propagation via err = ret.

The upstream discussion suggested fixing one issue at a time rather than combining several fixes in a single patch:

https://lore.kernel.org/bpf/[email protected]/

Would it be clearer to split these into separate patches? A minimal single-fix version addressing just the use-after-free was posted afterwards and was the form that landed (commit 2d66a033864e).

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