| Message ID | 157851813461.1732.9406594355094857662.stgit@ubuntu3-kvm2 |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Fixes for sockmap/tls from more complex BPF progs | expand |
On 8 Jan 2020, at 13:15, John Fastabend wrote: > It is possible to build a plaintext buffer using push helper that is larger > than the allocated encrypt buffer. When this record is pushed to crypto > layers this can result in a NULL pointer dereference because the crypto > API expects the encrypt buffer is large enough to fit the plaintext > buffer. > > To resolve catch the cases this can happen and split the buffer into two > records to send individually. Unfortunately, there is still one case to > handle where the split creates a zero sized buffer. In this case we merge > the buffers and unmark the split. This happens when apply is zero and user > pushed data beyond encrypt buffer. This fixes the original case as well > because the split allocated an encrypt buffer larger than the plaintext > buffer and the merge simply moves the pointers around so we now have > a reference to the new (larger) encrypt buffer. > > Perhaps its not ideal but it seems the best solution for a fixes branch > and avoids handling these two cases, (a) apply that needs split and (b) > non apply case. The are edge cases anyways so optimizing them seems not > necessary unless someone wants later in next branches. > > Fixes: d3b18ad31f93d ("tls: add bpf support to sk_msg handling") > Signed-off-by: John Fastabend <john.fastabend@gmail.com> Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index c6803a82b769..31f6bbbc8992 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -682,12 +682,32 @@ static int tls_push_record(struct sock *sk, int flags, split_point = msg_pl->apply_bytes; split = split_point && split_point < msg_pl->sg.size; + if (unlikely((!split && + msg_pl->sg.size + + prot->overhead_size > msg_en->sg.size) || + (split && + split_point + + prot->overhead_size > msg_en->sg.size))) { + split = true; + split_point = msg_en->sg.size; + } if (split) { rc = tls_split_open_record(sk, rec, &tmp, msg_pl, msg_en, split_point, prot->overhead_size, &orig_end); if (rc < 0) return rc; + /* This can happen if above tls_split_open_record allocates + * a single large encryption buffer instead of two smaller + * ones. In this case adjust pointers and continue without + * split. + */ + if (!msg_pl->sg.size) { + tls_merge_open_record(sk, rec, tmp, orig_end); + msg_pl = &rec->msg_plaintext; + msg_en = &rec->msg_encrypted; + split = false; + } sk_msg_trim(sk, msg_en, msg_pl->sg.size + prot->overhead_size); }
It is possible to build a plaintext buffer using push helper that is larger than the allocated encrypt buffer. When this record is pushed to crypto layers this can result in a NULL pointer dereference because the crypto API expects the encrypt buffer is large enough to fit the plaintext buffer. To resolve catch the cases this can happen and split the buffer into two records to send individually. Unfortunately, there is still one case to handle where the split creates a zero sized buffer. In this case we merge the buffers and unmark the split. This happens when apply is zero and user pushed data beyond encrypt buffer. This fixes the original case as well because the split allocated an encrypt buffer larger than the plaintext buffer and the merge simply moves the pointers around so we now have a reference to the new (larger) encrypt buffer. Perhaps its not ideal but it seems the best solution for a fixes branch and avoids handling these two cases, (a) apply that needs split and (b) non apply case. The are edge cases anyways so optimizing them seems not necessary unless someone wants later in next branches. Fixes: d3b18ad31f93d ("tls: add bpf support to sk_msg handling") Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- net/tls/tls_sw.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)