David Goulet: > Hi everyone, > > About a week ago I've sent an email to the Torsocks maintainers to > address some concerns I had about important issues of the current code > base. For the reasons found below, I proposed a rewrite that I am > willing to do and submit it to the community once I feel ok with it. > > Here is the repository I've created and started working on the rewrite. > It has been branched from upstream master at commit > e0bf65b5d3ca0e28b827c08d80b0fe1841d5a149 > > https://github.com/dgoulet/torsocks/tree/rewrite > > Torsocks rewrite reasons: > > 1) The code needs to be easier to maintain, so that it isn't > sporadically maintained. Considering the number of issues (see below), > it needs more love on a regular basis right now. > > 2) It has never been safe from the start so a rewrite will NOT impact > the current state of security. There is not much tests right now. > > 4) One of the biggest technical issues is that it's not thread safe and > fixing it right now would add an important impact on performance adding > locks to multiple global data structures. A clean rewrite would allow to > take into account this *very* important feature from the start without > any ugly hacks in the current code base. Please be extra careful with locking, especially since you're going to redesign this. I haven't personally reviewed torsock's current data structure use, but in general, when somebody looks at a codebase and says "this code is not threadsafe - it needs more locking around its data structure use", that person is usually wrong, or at best only half right. It's almost always better to get the locking out of your critical path and use thread local storage, message passing, and/or job scheduling instead of direct lock acquisition+release for commonly used data structures. This is not just for performance reasons. Deadlocks are forever. Library call hooking like this is especially tricky, because it could easily lead you to subtle instances of lock order reversal deadlock if you attempt to acquire your data structure lock before an OS/app lock via one socket call path, but after it in another (for example, due to OS/app abstractions that cause certain socket calls to be merely wrappers for combinations of others). Such app-specific and race-prone deadlock conditions will be very annoying to track down. Careful design of how you deploy locking and concurrency is a very wise time investment for this reason. You should probably post a design doc and ask for review of it. > 5) This is a very, and I can't stress this enough, _VERY_ intrusive > library so extra care MUST be put in making it bullet proof in terms of > error handling. Right now, there is multiple call sites that can fail on > error. -- Mike Perry
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ tor-dev mailing list tor-dev@xxxxxxxxxxxxxxxxxxxx https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev