| From 74ba1fbe5cc67200c08428cef8d473aa8fa316dc Mon Sep 17 00:00:00 2001 |
| From: Jakub Kicinski <jakub.kicinski@netronome.com> |
| Date: Mon, 4 Nov 2019 15:36:57 -0800 |
| Subject: [PATCH] net/tls: fix sk_msg trim on fallback to copy mode |
| |
| commit 683916f6a84023407761d843048f1aea486b2612 upstream. |
| |
| sk_msg_trim() tries to only update curr pointer if it falls into |
| the trimmed region. The logic, however, does not take into the |
| account pointer wrapping that sk_msg_iter_var_prev() does nor |
| (as John points out) the fact that msg->sg is a ring buffer. |
| |
| This means that when the message was trimmed completely, the new |
| curr pointer would have the value of MAX_MSG_FRAGS - 1, which is |
| neither smaller than any other value, nor would it actually be |
| correct. |
| |
| Special case the trimming to 0 length a little bit and rework |
| the comparison between curr and end to take into account wrapping. |
| |
| This bug caused the TLS code to not copy all of the message, if |
| zero copy filled in fewer sg entries than memcopy would need. |
| |
| Big thanks to Alexander Potapenko for the non-KMSAN reproducer. |
| |
| v2: |
| - take into account that msg->sg is a ring buffer (John). |
| |
| Link: https://lore.kernel.org/netdev/20191030160542.30295-1-jakub.kicinski@netronome.com/ (v1) |
| |
| Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface") |
| Reported-by: syzbot+f8495bff23a879a6d0bd@syzkaller.appspotmail.com |
| Reported-by: syzbot+6f50c99e8f6194bf363f@syzkaller.appspotmail.com |
| Co-developed-by: John Fastabend <john.fastabend@gmail.com> |
| Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> |
| Signed-off-by: John Fastabend <john.fastabend@gmail.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h |
| index 50ced8aba9db..4afabf2d2a42 100644 |
| --- a/include/linux/skmsg.h |
| +++ b/include/linux/skmsg.h |
| @@ -139,6 +139,11 @@ static inline void sk_msg_apply_bytes(struct sk_psock *psock, u32 bytes) |
| } |
| } |
| |
| +static inline u32 sk_msg_iter_dist(u32 start, u32 end) |
| +{ |
| + return end >= start ? end - start : end + (MAX_MSG_FRAGS - start); |
| +} |
| + |
| #define sk_msg_iter_var_prev(var) \ |
| do { \ |
| if (var == 0) \ |
| @@ -198,9 +203,7 @@ static inline u32 sk_msg_elem_used(const struct sk_msg *msg) |
| if (sk_msg_full(msg)) |
| return MAX_MSG_FRAGS; |
| |
| - return msg->sg.end >= msg->sg.start ? |
| - msg->sg.end - msg->sg.start : |
| - msg->sg.end + (MAX_MSG_FRAGS - msg->sg.start); |
| + return sk_msg_iter_dist(msg->sg.start, msg->sg.end); |
| } |
| |
| static inline struct scatterlist *sk_msg_elem(struct sk_msg *msg, int which) |
| diff --git a/net/core/skmsg.c b/net/core/skmsg.c |
| index 93bffaad2135..c7b1fe18a3b8 100644 |
| --- a/net/core/skmsg.c |
| +++ b/net/core/skmsg.c |
| @@ -271,18 +271,28 @@ void sk_msg_trim(struct sock *sk, struct sk_msg *msg, int len) |
| |
| msg->sg.data[i].length -= trim; |
| sk_mem_uncharge(sk, trim); |
| + /* Adjust copybreak if it falls into the trimmed part of last buf */ |
| + if (msg->sg.curr == i && msg->sg.copybreak > msg->sg.data[i].length) |
| + msg->sg.copybreak = msg->sg.data[i].length; |
| out: |
| - /* If we trim data before curr pointer update copybreak and current |
| - * so that any future copy operations start at new copy location. |
| + sk_msg_iter_var_next(i); |
| + msg->sg.end = i; |
| + |
| + /* If we trim data a full sg elem before curr pointer update |
| + * copybreak and current so that any future copy operations |
| + * start at new copy location. |
| * However trimed data that has not yet been used in a copy op |
| * does not require an update. |
| */ |
| - if (msg->sg.curr >= i) { |
| + if (!msg->sg.size) { |
| + msg->sg.curr = msg->sg.start; |
| + msg->sg.copybreak = 0; |
| + } else if (sk_msg_iter_dist(msg->sg.start, msg->sg.curr) >= |
| + sk_msg_iter_dist(msg->sg.start, msg->sg.end)) { |
| + sk_msg_iter_var_prev(i); |
| msg->sg.curr = i; |
| msg->sg.copybreak = msg->sg.data[i].length; |
| } |
| - sk_msg_iter_var_next(i); |
| - msg->sg.end = i; |
| } |
| EXPORT_SYMBOL_GPL(sk_msg_trim); |
| |
| -- |
| 2.7.4 |
| |