sched: Consider extra_space even if negative in KIST

With extra_space negative, it means that the "notsent" queue is quite large so
we must consider that value with the current computed tcp_space. If we end up
to have negative space, we should not add more data to the kernel since the
notsent queue is just too filled up.

Fixes #24665

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2017-12-19 16:20:36 -05:00
parent 7e45720cf4
commit 885ba513ff
2 changed files with 17 additions and 11 deletions

6
changes/bug24665 Normal file
View file

@ -0,0 +1,6 @@
o Major bugfixes (KIST, scheduler):
- The KIST scheduler did not correctly account for data already enqueued
in each connection's send socket buffer, particularly in cases when the
TCP/IP congestion window was reduced between scheduler calls. This
situation lead to excessive per-connection buffering in the kernel, and
a potential memory DoS. Fixes bug 24665; bugfix on 0.3.2.1-alpha.

View file

@ -266,8 +266,7 @@ update_socket_info_impl, (socket_table_ent_t *ent))
/* These values from the kernel are uint32_t, they will always fit into a /* These values from the kernel are uint32_t, they will always fit into a
* int64_t tcp_space variable but if the congestion window cwnd is smaller * int64_t tcp_space variable but if the congestion window cwnd is smaller
* than the unacked packets, the remaining TCP space is set to 0 so we don't * than the unacked packets, the remaining TCP space is set to 0. */
* write more on this channel. */
if (ent->cwnd >= ent->unacked) { if (ent->cwnd >= ent->unacked) {
tcp_space = (ent->cwnd - ent->unacked) * (int64_t)(ent->mss); tcp_space = (ent->cwnd - ent->unacked) * (int64_t)(ent->mss);
} else { } else {
@ -276,20 +275,21 @@ update_socket_info_impl, (socket_table_ent_t *ent))
/* The clamp_double_to_int64 makes sure the first part fits into an int64_t. /* The clamp_double_to_int64 makes sure the first part fits into an int64_t.
* In fact, if sock_buf_size_factor is still forced to be >= 0 in config.c, * In fact, if sock_buf_size_factor is still forced to be >= 0 in config.c,
* then it will be positive for sure. Then we subtract a uint32_t. At worst * then it will be positive for sure. Then we subtract a uint32_t. Getting a
* we end up negative, but then we just set extra_space to 0 in the sanity * negative value is OK, see after how it is being handled. */
* check.*/
extra_space = extra_space =
clamp_double_to_int64( clamp_double_to_int64(
(ent->cwnd * (int64_t)ent->mss) * sock_buf_size_factor) - (ent->cwnd * (int64_t)ent->mss) * sock_buf_size_factor) -
ent->notsent; ent->notsent;
if (extra_space < 0) { if ((tcp_space + extra_space) < 0) {
extra_space = 0; /* This means that the "notsent" queue is just too big so we shouldn't put
} * more in the kernel for now. */
ent->limit = 0;
/* Finally we set the limit. Adding two positive int64_t together will always } else {
* fit in an uint64_t. */ /* Adding two positive int64_t together will always fit in an uint64_t.
* And we know this will always be positive. */
ent->limit = (uint64_t)tcp_space + (uint64_t)extra_space; ent->limit = (uint64_t)tcp_space + (uint64_t)extra_space;
}
return; return;
#else /* !(defined(HAVE_KIST_SUPPORT)) */ #else /* !(defined(HAVE_KIST_SUPPORT)) */