diff options
author | David Goulet <dgoulet@torproject.org> | 2017-12-11 15:42:28 -0500 |
---|---|---|
committer | David Goulet <dgoulet@torproject.org> | 2017-12-11 15:45:19 -0500 |
commit | 057139d3830bb94df8031bb6e8e385cef53352bc (patch) | |
tree | 5d24e5cebb0c95b7a04a7fa6de716f1b092ab010 | |
parent | d68abbe358af41b18c9c54444e49689e2626328d (diff) | |
download | tor-057139d3830bb94df8031bb6e8e385cef53352bc.tar.gz tor-057139d3830bb94df8031bb6e8e385cef53352bc.zip |
sched: Avoid integer overflow when computing tcp_space
In KIST, we could have a small congestion window value than the unacked
packets leading to a integer overflow which leaves the tcp_space value to be
humongous.
This has no security implications but it results in KIST scheduler allowing to
send cells on a potentially saturated connection.
Found by #24423. Fixes #24590.
Signed-off-by: David Goulet <dgoulet@torproject.org>
-rw-r--r-- | changes/bug24590 | 5 | ||||
-rw-r--r-- | src/or/scheduler_kist.c | 11 |
2 files changed, 12 insertions, 4 deletions
diff --git a/changes/bug24590 b/changes/bug24590 new file mode 100644 index 0000000000..77e039f8d2 --- /dev/null +++ b/changes/bug24590 @@ -0,0 +1,5 @@ + o Minor bugfixes (scheduler, KIST): + - Avoid a possible integer overflow when computing the available space on + the TCP buffer of a channel. This has no security implications but can + make KIST not behave properly by allowing more cells on a already + saturated connection. Fixes bug 24590; bugfix on 0.3.2.1-alpha. diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index 3d8f553ac2..9acd89b37f 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -264,10 +264,13 @@ update_socket_info_impl, (socket_table_ent_t *ent)) * ^ ((cwnd * mss) * factor) bytes */ - /* Assuming all these values from the kernel are uint32_t still, they will - * always fit into a int64_t tcp_space variable. */ - tcp_space = (ent->cwnd - ent->unacked) * (int64_t)ent->mss; - if (tcp_space < 0) { + /* 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 + * than the unacked packets, the remaining TCP space is set to 0 so we don't + * write more on this channel. */ + if (ent->cwnd >= ent->unacked) { + tcp_space = (ent->cwnd - ent->unacked) * (int64_t)(ent->mss); + } else { tcp_space = 0; } |