From: Chris Mason sk_msg_memcopy_from_iter() writes to the copy position: byte sg.copybreak after the offset of element sg.curr. Both tcp_bpf_push() and sk_msg_xfer() remove bytes from the front of an element. They increase its offset, but they do not change sg.copybreak when the element is sg.curr. tcp_bpf_send_verdict() can send a part of a message and keep the rest as the cork. tcp_bpf_sendmsg() appends new data to it. If sk_msg_alloc() then extends element sg.curr, the peer can get memory that nobody wrote: old offset offset end of data offset + sg.copybreak | removed | rest | not written | new data Move sk_msg_reset_curr() from filter.c to skmsg.h. Call it when a message becomes the cork, as the helpers that change a message do. It sets the copy position to the end of the data. Without removed bytes it changes nothing. Fixes: 91843d540a13 ("bpf: sockmap, add msg_cork_bytes() helper") Signed-off-by: Chris Mason Signed-off-by: Alexei Starovoitov --- include/linux/skmsg.h | 19 +++++++++++++++++++ net/core/filter.c | 14 -------------- net/ipv4/tcp_bpf.c | 5 +++++ 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h index d5e35f24738d..f0e27b4b929f 100644 --- a/include/linux/skmsg.h +++ b/include/linux/skmsg.h @@ -196,6 +196,25 @@ static inline void sk_msg_init(struct sk_msg *msg) sg_init_marker(msg->sg.data, NR_MSG_FRAG_IDS); } +/* + * sk_msg_memcopy_from_iter() writes to byte sg.copybreak of element sg.curr, + * counted from the offset of that element. Point both past the last byte of + * the message, after something other than a copy has changed its elements. + */ +static inline void sk_msg_reset_curr(struct sk_msg *msg) +{ + if (!msg->sg.size) { + msg->sg.curr = msg->sg.start; + msg->sg.copybreak = 0; + } else { + u32 i = msg->sg.end; + + sk_msg_iter_var_prev(i); + msg->sg.curr = i; + msg->sg.copybreak = msg->sg.data[i].length; + } +} + static inline void sk_msg_xfer(struct sk_msg *dst, struct sk_msg *src, int which, u32 size) { diff --git a/net/core/filter.c b/net/core/filter.c index 9245009d9f1e..50c0cda778db 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2677,20 +2677,6 @@ BPF_CALL_2(bpf_msg_cork_bytes, struct sk_msg *, msg, u32, bytes) return 0; } -static void sk_msg_reset_curr(struct sk_msg *msg) -{ - if (!msg->sg.size) { - msg->sg.curr = msg->sg.start; - msg->sg.copybreak = 0; - } else { - u32 i = msg->sg.end; - - sk_msg_iter_var_prev(i); - msg->sg.curr = i; - msg->sg.copybreak = msg->sg.data[i].length; - } -} - static bool sk_msg_elem_is_copy(const struct sk_msg *msg, u32 i) { return test_bit(i, msg->sg.copy); diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c index 3d703f1517d8..d534974c33f8 100644 --- a/net/ipv4/tcp_bpf.c +++ b/net/ipv4/tcp_bpf.c @@ -469,6 +469,11 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, return -ENOMEM; } } + /* + * A push or a redirect may have taken bytes off the front of + * element sg.curr. The next sendmsg() appends to the cork. + */ + sk_msg_reset_curr(msg); memcpy(psock->cork, msg, sizeof(*msg)); return 0; } -- 2.55.0