A splice or zero-copy append can leave the destination copy cursor at
sg.end. If sk_msg_clone() subsequently coalesces with the last entry,
sg.end does not advance and the cursor still points at an unused slot.
sk_msg_memcopy_from_iter() can then walk around the ring and write into
older entries, including file pages spliced from a read-only descriptor.
Software TLS can reach this with an ordinary MSG_MORE send after a file
splice and a splice from another TCP socket. Both TCP and the TLS
encrypted-message allocation can use the sending task's page fragment,
so the TCP-sourced plaintext can be contiguous with the next cloned
encrypted-message range.
When coalescing with an exhausted destination, move the copy cursor to
the tail and set copybreak to its old length before extending it. Find
the actual tail on each iteration using the ring iterator, including
when end wraps to zero. Leave an existing copy cursor unchanged and
preserve the normal coalescing, reference counting, and memory charging.
Fixes: fda497e5f5b8 ("Optimize sk_msg_clone() by data merge to end dst sg entry")
Cc: <redacted>
Signed-off-by: Kyle Zeng <redacted>
---
I have a reproducer for this issue and can share via DM
net/core/skmsg.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index df385a5a961e..0e9ff4be78ba 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -103,16 +103,26 @@ int sk_msg_clone(struct sock *sk, struct sk_msg *dst, struct sk_msg *src,
}
while (len) {
+ int last = dst->sg.end;
+
sge_len = sge->length - off;
if (sge_len > len)
sge_len = len;
- if (dst->sg.end)
- sgd = sk_msg_elem(dst, dst->sg.end - 1);
+ sgd = NULL;
+ if (dst->sg.size) {
+ sk_msg_iter_var_prev(last);
+ sgd = sk_msg_elem(dst, last);
+ }
if (sgd &&
(sg_page(sge) == sg_page(sgd)) &&
(sg_virt(sge) + off == sg_virt(sgd) + sgd->length)) {
+ /* Start copying at the newly cloned suffix. */
+ if (dst->sg.curr == dst->sg.end) {
+ dst->sg.curr = last;
+ dst->sg.copybreak = sgd->length;
+ }
sgd->length += sge_len;
dst->sg.size += sge_len;
} else if (!sk_msg_full(dst)) {
base-commit: d89e1d186ec9271bf78e62771779e9fc03bf0fc3
--
2.56.0