On Wed, Dec 2, 2009 at 8:27 PM, Johannes Sixt <j6t@xxxxxxxx> wrote: > On Mittwoch, 2. Dezember 2009, Erik Faye-Lund wrote: >> On Fri, Nov 27, 2009 at 8:59 PM, Johannes Sixt <j6t@xxxxxxxx> wrote: >> > "relatively small chance of stuff blowing up"? The docs of >> > TerminateThread: "... the kernel32 state for the thread's process could >> > be inconsistent." That's scary if we are talking about a process that >> > should run for days or weeks without interruption. >> >> I think there's a misunderstanding here. I thought your suggestion was >> to simply call die(), which would take down the main process. After >> reading this explanation, I think you're talking about giving an error >> and rejecting the connection instead. Which makes more sense than to >> risk crashing the main-process, indeed. > > Just rejecting a connection is certainly the simplest do to keep the daemon > process alive. But the server can be DoS-ed from a single source IP. > > Currently git-daemon can only be DDoS-ed because there is a maximum number of > connections, which are not closed if all of them originate from different > IPs. > After some testing I've found that git-daemon can very much be DoS-ed from a single IP in it's current form. This is for two reasons: 1) The clever xcalloc + memcmp trick has a fault; the port for each connection is different, so there will never be a match. I have a patch[1] for this that I plan to send out soon. 2) Even with this patch the effect of the DoS-protection is kind of limited. This is because it's a child process of the fork()'d process again that does all the heavy lifting, and kill(pid, SIGHUP) doesn't kill child processes. So, the connection gets to continue the action until upload-pack (or whatever the current command is) finish. This might be quite lengthy. As I said, I have a patch for 1), but I don't quite know how to fix 2). Perhaps this is a good use for process groups? I'm a Windows-guy; my POSIX isn't exactly super-awesome... I found these issues during my latest effort to port git-daemon to Windows. I managed to get this to work fine on Windows, by implementing a kill(x, SIGTERM) that terminated child-processes (because I was under the impression that this was what happened... I guess daemon.c lead me to believe that). [1]: http://repo.or.cz/w/git/kusma.git/commit/b1d286d32f42c57b90a1db9b7b8d6775a5d1ad7b -- Erik "kusma" Faye-Lund -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html