[PATCH net v2] xfrm: espintcp: reserve partial message during allocation

[PATCH net v2] xfrm: espintcp: reserve partial message during allocation

[Bruno Produit]

espintcp_sendmsg() builds a new message directly in ctx->partial. If
allocation fails, sk_stream_wait_memory() drops the socket lock while
the shared sk_msg remains unpublished with emsg->len equal to zero. A
concurrent sender can then reuse the same slot. If the first sender is
interrupted, its failure path frees state now owned by the second sender
while TCP may still be consuming it, causing a use-after-free.

Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Cc: stable@vger.kernel.org
Reported-by: Kyle Zeng <kylebot@openai.com>
Assisted-by: Codex:gpt-5.6-cyber
Signed-off-by: Bruno Produit <bruno.produit@trailofbits.com>
---
Changes in v2:
- Add an ->owned flag to espintcp_msg
- Use ->owned instead of a local sk_msg

v1: https://lore.kernel.org/netdev/20260922145335.2016559-1-bruno.produit@trailofbits.com/

 include/net/espintcp.h |  1 +
 net/xfrm/espintcp.c    | 25 ++++++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/net/espintcp.h b/include/net/espintcp.h
index c70efd704b6d..083c11373c16 100644
--- a/include/net/espintcp.h
+++ b/include/net/espintcp.h
@@ -15,6 +15,7 @@ struct espintcp_msg {
 	struct sk_buff *skb;
 	struct sk_msg skmsg;
+	bool owned;
 	int offset;
 	int len;
 };
 
 struct espintcp_ctx {
diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index 3e72b9f067b9..68b9201c98d3 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -251,6 +251,8 @@ static int espintcp_push_msgs(struct sock *sk, int flags)
 	struct espintcp_msg *emsg = &ctx->partial;
 	int err;
 
+	if (emsg->owned)
+		return -EAGAIN;
 	if (!emsg->len)
 		return 0;
 
@@ -274,6 +276,12 @@ static int espintcp_push_msgs(struct sock *sk, int flags)
 	return err;
 }
 
+static void espintcp_unreserve_msg(struct sock *sk, struct espintcp_msg *emsg)
+{
+	WRITE_ONCE(emsg->owned, false);
+	sk->sk_write_space(sk);
+}
+
 int espintcp_push_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct espintcp_ctx *ctx = espintcp_getctx(sk);
@@ -291,7 +299,7 @@ int espintcp_push_skb(struct sock *sk, struct sk_buff *skb)
 
 	espintcp_push_msgs(sk, 0);
 
-	if (emsg->len) {
+	if (emsg->owned || emsg->len) {
 		kfree_skb(skb);
 		return -ENOBUFS;
 	}
@@ -336,10 +344,11 @@ static int espintcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 			err = -ENOBUFS;
 		goto unlock;
 	}
-	if (emsg->len) {
+	if (emsg->owned || emsg->len) {
 		err = -ENOBUFS;
 		goto unlock;
 	}
+	WRITE_ONCE(emsg->owned, true);
 
 	sk_msg_init(&emsg->skmsg);
 	while (1) {
@@ -368,9 +377,10 @@ static int espintcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 		goto fail;
 
 	end = emsg->skmsg.sg.end;
-	emsg->len = size;
 	sk_msg_iter_var_prev(end);
 	sg_mark_end(sk_msg_elem(&emsg->skmsg, end));
+	emsg->len = size;
+	espintcp_unreserve_msg(sk, emsg);
 
 	tcp_rate_check_app_limited(sk);
 
@@ -383,7 +393,7 @@ static int espintcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 
 fail:
 	sk_msg_free(sk, &emsg->skmsg);
-	memset(emsg, 0, sizeof(*emsg));
+	espintcp_unreserve_msg(sk, emsg);
 unlock:
 	release_sock(sk);
 	return err;
@@ -549,8 +559,13 @@ static __poll_t espintcp_poll(struct file *file, struct socket *sock,
 {
 	struct sock *sk = sock->sk;
 	struct espintcp_ctx *ctx = espintcp_getctx(sk);
+	__poll_t mask;
+
+	mask = datagram_poll_queue(file, sock, wait, &ctx->ike_queue);
+	if (READ_ONCE(ctx->partial.owned))
+		mask &= ~(EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);
 
-	return datagram_poll_queue(file, sock, wait, &ctx->ike_queue);
+	return mask;
 }
 
 static void build_protos(struct proto *espintcp_prot,
-- 
2.53.0

Re: [PATCH net v2] xfrm: espintcp: reserve partial message during allocation

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] xfrm: espintcp: reserve partial message during allocation

[Bruno Produit]

Hi, we triggered the issue locally (KASAN warning in this case) with a
poc that I can share. The (shortened) output of KASAN:

WARNING: at skb_splice_from_iter+0x380/0x7a0
   tcp_sendmsg_locked
   espintcp_sendskmsg_locked
   espintcp_sendmsg



On Fri, Oct 2, 2026 at 12:29 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net v2] xfrm: espintcp: reserve partial message during allocation

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] espintcp_poll() can lose an EPOLLOUT wakeup.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
  espintcp_push_msgs(sk, 0) can sleep, and self-deadlock on…
- [Medium] This problem wasn't introduced by this patch, but
  espintcp_poll() and espintcp_sendmsg() use different definitions of a…

--

xfrm: espintcp: reserve partial message during allocation

This patch adds an owned flag to struct espintcp_msg. espintcp_sendmsg()
uses it to reserve ctx->partial while it builds a new message, and the
reservation holds across sk_stream_wait_memory(). While owned is set,
other senders and espintcp_push_skb() treat the slot as busy, and
espintcp_poll() masks out EPOLLOUT.

> diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
> index 3e72b9f067b9b..68b9201c98d3f 100644
> --- a/net/xfrm/espintcp.c
> +++ b/net/xfrm/espintcp.c

[ ... ]

> @@ -291,7 +299,7 @@ int espintcp_push_skb(struct sock *sk, struct sk_buff *skb)
>  
>  	espintcp_push_msgs(sk, 0);

[Severity: High]
This is a pre-existing issue and this patch did not introduce it. Can
espintcp_push_msgs(sk, 0) sleep here while the caller is in atomic
context?

esp_output_tcp_finish() calls espintcp_push_skb() under bh_lock_sock(),
and esp6 has the same pattern:

net/ipv4/esp4.c:esp_output_tcp_finish() {
    ...
	bh_lock_sock(sk);
	if (sock_owned_by_user(sk))
		err = espintcp_queue_out(sk, skb);
	else
		err = espintcp_push_skb(sk, skb);
	bh_unlock_sock(sk);
    ...
}

It is also called from espintcp_release(). That function runs as
->release_cb from release_sock() with sk->sk_lock.slock held.

Suppose ctx->partial holds an IKE sk_msg that was only partly sent
(emsg->skb == NULL, emsg->len != 0). A MSG_DONTWAIT sendmsg, an
SO_SNDTIMEO timeout or a signal can leave that state behind. In that
case espintcp_sendskmsg_locked() calls tcp_sendmsg_locked() without
MSG_DONTWAIT:

	struct msghdr msghdr = {
		.msg_flags = flags | MSG_SPLICE_PAGES | MSG_MORE,
	};

That means tcp_sendmsg_locked() uses sk_sndtimeo. If the send buffer is
still full, it can take this path:

espintcp_push_msgs(sk, 0)
  espintcp_sendskmsg_locked()
    tcp_sendmsg_locked()
      sk_stream_wait_memory()
        sk_wait_event()
          release_sock()
            spin_lock_bh(&sk->sk_lock.slock)

The caller already holds sk->sk_lock.slock. Wouldn't this self-deadlock
with BH disabled? Even without the recursion, wait_woken() would sleep in
atomic context.

Here is one sequence that looks reachable:

1. An IKE sendmsg(MSG_DONTWAIT) leaves a partial message behind.
2. During that call, an ESP packet gets queued to out_queue.
3. The final release_sock() in espintcp_sendmsg() then runs
   espintcp_release()->espintcp_push_skb()->espintcp_push_msgs(sk, 0).

A peer that withholds ACKs or advertises a zero window can keep the send
buffer full.

The skb path seems safe only because __skb_send_sock() forces
MSG_DONTWAIT. The skmsg path doesn't do that. This patch changes
espintcp_push_msgs() with the owned early return, but leaves this
behaviour as it was.

>  
> -	if (emsg->len) {
> +	if (emsg->owned || emsg->len) {
>  		kfree_skb(skb);
>  		return -ENOBUFS;
>  	}

[ ... ]

> @@ -549,8 +559,13 @@ static __poll_t espintcp_poll(struct file *file, struct socket *sock,
>  {
>  	struct sock *sk = sock->sk;
>  	struct espintcp_ctx *ctx = espintcp_getctx(sk);
> +	__poll_t mask;
> +
> +	mask = datagram_poll_queue(file, sock, wait, &ctx->ike_queue);
> +	if (READ_ONCE(ctx->partial.owned))
> +		mask &= ~(EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);

[Severity: Medium]
Can a poll or epoll waiter miss the EPOLLOUT wakeup here?

espintcp_poll() runs without the socket lock. A thread that polls only
for EPOLLOUT can see partial.owned == true while another thread is inside
espintcp_sendmsg(), and then go to sleep. That window covers both the
sk_msg_memcopy_from_iter() copies, which can fault, and
sk_stream_wait_memory().

When owned is cleared, this is the only notification:

espintcp_unreserve_msg()
  sk->sk_write_space()
    espintcp_write_space()
      ctx->saved_write_space()    /* sk_stream_write_space() */

sk_stream_write_space() only wakes waiters when __sk_stream_is_writeable()
holds:

	if (__sk_stream_is_writeable(sk, 1) && sock) {
		clear_bit(SOCK_NOSPACE, &sock->flags);
		...
			wake_up_interruptible_poll(&wq->wait, EPOLLOUT |
						EPOLLWRNORM | EPOLLWRBAND);

That check is based on sk_wmem_queued. datagram_poll_queue() decides
EPOLLOUT with sock_writeable(), which is based on sk_wmem_alloc.

Say sk_wmem_queued is between about 2/3 of sk_sndbuf and sk_sndbuf, for
example because of unacked ESP or IKE data:

- sk_stream_memory_free() is still true. The sender's own push doesn't
  reach wait_for_space, so SOCK_NOSPACE is never set.
- __sk_stream_is_writeable() is false, so the unreserve wakes nobody.

Later ACKs go through tcp_check_space(). It only calls
__tcp_check_space() when SOCK_NOSPACE is set, and datagram_poll_queue()
only sets SOCKWQ_ASYNC_NOSPACE.

Could the poller then sleep until its timeout, even though a fresh poll
would report EPOLLOUT? Before this patch EPOLLOUT never depended on owned,
so this path looks new.

[Severity: Medium]
This is a pre-existing issue and this patch did not introduce it.
espintcp_poll() and espintcp_sendmsg() still disagree on when the partial
slot is busy.

espintcp_sendmsg() returns -ENOBUFS when it sees this:

	if (emsg->owned || emsg->len) {

With MSG_DONTWAIT it can also return -EAGAIN from espintcp_push_msgs()
when the pending message can't be drained. espintcp_poll() only checks
partial.owned.

Take a published message that is still pending (emsg->len != 0, owned ==
false). Poll reports EPOLLOUT whenever sock_writeable() is true, but
sendmsg reports busy.

Can an event-driven, non-blocking IKE sender spin in that state? The len
half of this mismatch was already there before this patch. The patch only
adds the owned half to poll.

>  
> -	return datagram_poll_queue(file, sock, wait, &ctx->ike_queue);
> +	return mask;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261002102447.148835-1-bruno.produit%40trailofbits.com