[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #25573 [Core Tor/Tor]: Track half-closed stream IDs

#25573: Track half-closed stream IDs
------------------------------------------------+--------------------------
 Reporter:  mikeperry                           |          Owner:  (none)
     Type:  defect                              |         Status:
                                                |  needs_review
 Priority:  Medium                              |      Milestone:  Tor:
                                                |  0.3.4.x-final
Component:  Core Tor/Tor                        |        Version:
 Severity:  Normal                              |     Resolution:
 Keywords:  guard-discovery-stats 034-backport  |  Actual Points:
Parent ID:  #25574                              |         Points:
 Reviewer:  teor                                |        Sponsor:
                                                |  SponsorV-can
------------------------------------------------+--------------------------

Comment (by teor):

 Replying to [comment:17 mikeperry]:
 > Ok the following fixups have been pushed to mikeperry/ticket25573-v2:
 >
 > The space is fixed in c2dbb09dc3e4ed2b9d5a845a592a2d59d2d5e225.
 >
 > Using bsearch in get_unique_stream_id_by_circ() is fixed in
 99153878cb4b7ae309eb7f5109747e3a49ac00e9.

 These commits seem fine.

 > Tests for stream id wrapping and other get_unique_stream_id_by_circ()
 cases are in b8ae08caa059dc8f937a4afe9f929035b8beb712.

 I suggest you use tor's crypto_rand() for these tests.

 Here are some reasons from our ChangeLog:
 {{{
     - Use our own weak RNG when we need a weak RNG. Windows's rand() and
       Irix's random() only return 15 bits; Solaris's random() returns more
       bits but its RAND_MAX says it only returns 15, and so on. Motivated
       by the fix for bug 7801; bugfix on 0.2.2.20-alpha.
 }}}

 Also, I'm pretty sure some compilers on some OSes will warn if you use
 rand().

 > I have pushed a cleanly reorganized and squashed branch to
 mikeperry/ticket25573-v2-squashed2 (aka
 https://github.com/torproject/tor/pull/296). Its has 0 delta to
 mikeperry/ticket25573-v2, and everything has been squashed into 4 commits
 (a const for smartlist_bsearch; all half-stream tracking code+tests;
 prevent stream-id reuse code+tests; and truncate accounting code+tests).
 Only the last two commits change client behavior.

 The old branch was cancelled on appveyor, and the new branch timed out:
 * https://ci.appveyor.com/project/torproject/tor/build/1.0.701
 * https://ci.appveyor.com/project/torproject/tor/build/1.0.702

 So I pushed your branch to my github to trigger another build:
 * https://ci.appveyor.com/project/teor2345/tor/build/1.0.102

 And made myself an appveyor admin so I could trigger future builds more
 easily (#27372).

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25573#comment:18>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs