Re: [PATCH] xfrm: espintcp: fix sg.size corruption on partial send error

[Sabrina Dubroca <sd@queasysnail.net>]

2026-05-18, 12:21:09 +0900, Aaron Esau wrote:
> espintcp_sendskmsg_locked() calls put_page() and sk_mem_uncharge() for
> each scatterlist element it successfully sends, but never decrements
> sg.size. If tcp_sendmsg_locked() then fails partway through, the error
> path advances sg.start past the freed elements while sg.size still
> accounts for them. A subsequent sk_msg_free() in espintcp_close() loops
> until sg.size reaches zero, overshoots sg.end, hits zeroed entries with
> NULL pages, and crashes in put_page().
> 
> Fix this by decrementing sg.size as each element is freed. Also use
> sk_msg_iter_var_next() instead of raw addition for sg.start, so it
> wraps at NR_MSG_FRAG_IDS.

(wrapping shouldn't be an issue since I don't think we can have start
!= 0 in espintcp)

> Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Esau <aaron1esau@gmail.com>
> ---
>  net/xfrm/espintcp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
> index e1b11ab59..6755f6df6 100644
> --- a/net/xfrm/espintcp.c
> +++ b/net/xfrm/espintcp.c
> @@ -237,7 +237,8 @@ static int espintcp_sendskmsg_locked(struct sock *sk,
>  		ret = tcp_sendmsg_locked(sk, &msghdr, size);
>  		if (ret < 0) {
>  			emsg->offset = offset - sg->offset;
> -			skmsg->sg.start += done;
> +			while (done--)
> +				sk_msg_iter_var_next(skmsg->sg.start);
>  			return ret;
>  		}
>  
> @@ -250,6 +251,7 @@ static int espintcp_sendskmsg_locked(struct sock *sk,
>  		done++;
>  		put_page(p);
>  		sk_mem_uncharge(sk, sg->length);
> +		skmsg->sg.size -= sg->length;
>  		sg = sg_next(sg);
>  	} while (sg);

Or maybe switch to using sk_msg_free_partial()? It should fix the
issue and clean up the code at the same time. The diff looks a bit
nasty but this boils down to "remove all the custom
size/offset/partial send handling":

-------- 8< --------
diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index a2756186e13a..4802b68a833d 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -212,43 +212,23 @@ static int espintcp_sendskmsg_locked(struct sock *sk,
 	struct sk_msg *skmsg = &emsg->skmsg;
 	bool more = flags & MSG_MORE;
 	struct scatterlist *sg;
-	int done = 0;
 	int ret;
 
-	sg = &skmsg->sg.data[skmsg->sg.start];
 	do {
 		struct bio_vec bvec;
-		size_t size = sg->length - emsg->offset;
-		int offset = sg->offset + emsg->offset;
-		struct page *p;
-
-		emsg->offset = 0;
 
+		sg = &skmsg->sg.data[skmsg->sg.start];
 		if (sg_is_last(sg) && !more)
 			msghdr.msg_flags &= ~MSG_MORE;
 
-		p = sg_page(sg);
-retry:
-		bvec_set_page(&bvec, p, size, offset);
-		iov_iter_bvec(&msghdr.msg_iter, ITER_SOURCE, &bvec, 1, size);
-		ret = tcp_sendmsg_locked(sk, &msghdr, size);
-		if (ret < 0) {
-			emsg->offset = offset - sg->offset;
-			skmsg->sg.start += done;
+		bvec_set_page(&bvec, sg_page(sg), sg->length, sg->offset);
+		iov_iter_bvec(&msghdr.msg_iter, ITER_SOURCE, &bvec, 1, sg->length);
+		ret = tcp_sendmsg_locked(sk, &msghdr, sg->length);
+		if (ret < 0)
 			return ret;
-		}
-
-		if (ret != size) {
-			offset += ret;
-			size -= ret;
-			goto retry;
-		}
 
-		done++;
-		put_page(p);
-		sk_mem_uncharge(sk, sg->length);
-		sg = sg_next(sg);
-	} while (sg);
+		sk_msg_free_partial(sk, skmsg, ret);
+	} while (skmsg->sg.size);
 
 	memset(emsg, 0, sizeof(*emsg));
 
-- 
Sabrina