#25202: Check the calculations in cc_stats_refill_bucket using non fatal assertions --------------------------+------------------------------------ Reporter: teor | Owner: (none) Type: defect | Status: needs_review Priority: Medium | Milestone: Tor: 0.3.3.x-final Component: Core Tor/Tor | Version: Severity: Normal | Resolution: Keywords: tor-dos | Actual Points: Parent ID: #24902 | Points: 0.1 Reviewer: | Sponsor: --------------------------+------------------------------------ Changes (by teor):
* status: new => needs_review Old description: > In #25128, we removed an incorrect non-fatal assertion in > cc_stats_refill_bucket() to silence a warning: > {{{ > /* This function is not allowed to make the bucket count smaller */ > tor_assert_nonfatal(new_circuit_bucket_count >= stats->circuit_bucket); > }}} > > But we could have fixed the check instead, and added another check: > {{{ > /* This function is not allowed to make the bucket count larger than > the burst value */ > tor_assert_nonfatal(new_circuit_bucket_count <= dos_cc_circuit_burst); > /* This function is not allowed to make the bucket count smaller, > unless it is decreasing it to a newly configured, lower burst value. We > allow the bucket to stay the same size, in case the circuit rate is zero. > */ > tor_assert_nonfatal(new_circuit_bucket_count >= stats->circuit_bucket > || new_circuit_bucket_count == dos_cc_circuit_burst); > }}} > > We could be even more clever, and skip parts of the function if the rate > is zero. That's probably unnecessary. I'll think about it. > > I should get a chance to turn this into a proper branch over the next > week or so. If someone else wants to do it before then, go for it! New description: In #25128, we removed an incorrect non-fatal assertion in cc_stats_refill_bucket() to silence a warning: {{{ /* This function is not allowed to make the bucket count smaller */ tor_assert_nonfatal(new_circuit_bucket_count >= stats->circuit_bucket); }}} But we could have fixed the check instead, and added another check: {{{ /* This function is not allowed to make the bucket count larger than the burst value */ tor_assert_nonfatal(new_circuit_bucket_count <= dos_cc_circuit_burst); /* This function is not allowed to make the bucket count smaller, unless it is decreasing it to a newly configured, lower burst value. We allow the bucket to stay the same size, in case the circuit rate is zero. */ tor_assert_nonfatal(new_circuit_bucket_count >= stats->circuit_bucket || new_circuit_bucket_count == dos_cc_circuit_burst); }}} We could be even more clever, and skip parts of the function if the rate is zero. That's probably unnecessary. I'll think about it. I should get a chance to turn this into a proper branch over the next week or so. If someone else wants to do it before then, go for it! -- Comment: Spacing, and someone else might as well look at this for review -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25202#comment:1> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs