[CVS] pidfile support
rjs3 at andrew.cmu.edu
Mon Dec 16 11:22:36 EST 2002
On Sun, 15 Dec 2002, Henrique de Moraes Holschuh wrote:
> 1. There is no support for updating the pidfile. Thus, the lock is acquired
> only after forking, which means we lost the controlling terminal by the time
> we can complain about not being able to acquire a lock, and that we have
> already tried to go daemon and thus whatever is trying to start master is
> NOT told that we failed.
I don't think you can fix this in a perfect way, giving unix locking
semantics (well, atleast fcntl).
This is using the code from the original patch:
[A] lock and write pidfile
[A] fork and create [B] (which doesn't hold a lock on the file)
[B] attempt to reacquire lock on pidfile, FAIL since A doesn't have it
The parent to A thinks we successfully finished, when in reality we've
stepped on our own feet.
I've experementally verified that this is actually a problem in theory
(if not in practice).
I don't think the BSD flock() semantics even help us here (hold lock
across fork() but any close() will get rid of the lock), since then we
[A] lock and write the pidfile
[A] fork and create [B] (which shares the lock with [A])
[A] exit(success), which closes the file descriptor and releases the lock
[C] start a master process, lock and write the pidfile
[B] attempt to reacquire lock, and fail because [C] has the lock
(I'll note that I haven't experementally verified this sequence)
Now we've exited in failure, and not reported so to our parent.
But, I think this might be less of an issue than the first, since master
still starts successfully in the end (though, perhaps with a different
I'm willing to listen to suggestions.
> 2. Close on exec is not being set on the pidfile. Right now, this is not
> a bug. However, should we forget a close(pidfile) anywhere, it could
> somehow be the part of a bigger security hole...
This is doable I suppose.
Rob Siemborski * Andrew Systems Group * Cyert Hall 207 * 412-268-7456
Research Systems Programmer * /usr/contributed Gatekeeper
More information about the Info-cyrus