On Friday 21 March 2008 6:25:59 am Philip Guenther wrote:
> On Fri, Mar 21, 2008 at 1:44 AM, Tvrvk Edwin <[EMAIL PROTECTED]> wrote:
> > Philip Guenther wrote:
> >  > On Thu, Mar 20, 2008 at 3:01 PM, Tvrvk Edwin <[EMAIL PROTECTED]>
> wrote:
> >  >>  ClamAV has changed to call fork() after creating its local socket.
> >  >>  This causes weird behaviours when communicating on the socket [1]
> >  >>
> >  >>  If fork() is called before creating the socket() it works.
> >  >>
> >  >>  Is it safe to create a socket, fork(), and then call pthread_create()
> >  >>  and read from the socket?
> ...
> >  fork() is used to daemonize the process.
> 
> Okay, it's a bug in libpthread.  The user-space thread implementation
> of libpthread sets all the file descriptors to be non-blocking
> (O_NONBLOCK) so that it can catch blocking I/O and perform a context
> switch to another thread when that happens.  This is hidden from the
> application itself: if a threaded app calls fcntl(fd, F_GETFL), the
> library will hide the O_NONBLOCK flag unless the app actually called
> fcntl() to set it.  When a process exits, the library resets the fds
> back to blocking if they were only non-blocking for the library; this
> is so that other, non-threaded apps don't get confused when /dev/tty
> is left non-blocking, and things like that.
> 
> That last bit is the catch: when the parent exits after calling fork,
> the socket is reset to blocking and the child never sets it back to
> non-blocking again.

Your analysis is correct. 

> It's not clear to me how the child can reliably detect that this has
> occurred.  It could use a kqueue/kevent to detect when its parent has
> exited, but the reset could just as well be done by the child's
> grandparent (consider a double-fork daemonize).

Indeed this is a nasty limitation of userland threads that doesn't have
an easy solution.

> As a gross kludge, I think things would "work" if you added the
> following to the code called by the child after the fork.
>    sleep(1); /* make sure the parent has a chance to exit */
>    fcntl(sockfd, F_SETFL, fcntl(sockfd, F_GETFL) & ~O_NONBLOCK);
> 
> (With the correct 'sockfd' variable, of course).  That'll bring the
> kernel's O_NONBLOCK flag on the socket in sync with what libpthread
> thinks it should be.

To avoid the race in the child (and the sleep() call), the parrent can set the
fd's to non-blocking *before* the fork() and the child can set them back to
blocking directly after the fork. Obviously not ideal but it will work-around
the problem effectively.

> I'll note that I see a *bunch* of other fork() calls in the clamav
> source (0.92.1), some of which look very dubious in terms of safety.
> For example, dirscan() in clamd/scanner.c has a threadpool_t argument,
> so I presume it's called after threads have been spawned, yet it calls
> virusaction() which calls fork() and the child calls strdup(),
> malloc(), putenv(), system(), and exit(), none of which are
> async-signal safe.
> 
> <shrug>

Yuck

-Kurt

Reply via email to