Thread (3 messages) 3 messages, 3 authors, 8d ago

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

From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2026-05-21 14:39:35
Also in: stable
Subsystem: networking [general], networking [ipsec], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Steffen Klassert, Herbert Xu, Linus Torvalds

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)
quoted hunk ↗ jump to hunk
Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Esau <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help