Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fork() safety #5

Open
zackw opened this issue Aug 28, 2013 · 4 comments
Open

fork() safety #5

zackw opened this issue Aug 28, 2013 · 4 comments

Comments

@zackw
Copy link
Contributor

zackw commented Aug 28, 2013

The global state object should be automatically invalidated on the child side of a fork, to ensure that the child does not produce the same random stream as the parent from then on (see http://emboss.github.io/blog/2013/08/21/openssl-prng-is-not-really-fork-safe/ for discussion of this issue wrt OpenSSL).

The obvious way to do this is with pthread_atfork; conveniently, ottery_wipe already has the right signature for that. It does raise a bunch of portability issues, but I don't see a better approach.

Dealing with this for application-allocated ottery_state objects is probably up to the application, but we should at least document the problem.

@nmathewson
Copy link
Owner

Well, libottery currently does a "gitpid() != st->pid" check, which doesn't have the same flaw as openssl's "MD_Update(...cur_pid)" operation. No matter how many times you fork a process, the child's pid will never be the same as the parent's.

There is a possible flaw here, though: if we fork, and then the parent dies, and then the child forks again without invoking any ottery functions in the meantime, we could wind up with the same PID as the original process, which would keep us from noticing that the PID had changed.

There's an old-ish branch "topic/pthread_atfork" that tries to do the obvious pthread_atfork workaround. I'm not completely happy with it, but I do think we should take some approach like that at some point.

@nmathewson
Copy link
Owner

The branch topic/pthread_atfork_v2 now has some sketched-out work here, but I don't much care for it. It uses both an atfork-based implementation and a getpid-based implementation, but doing both of these slows down small requests by more than I'd like... and this is before making the atfork-counter thread-safe. (Atomic, not locked, I hope!) If we can really trust the pthread_atfork thing, we could get the time back, but I'm worried to do so without a lot of thinking and testing.

@zackw
Copy link
Contributor Author

zackw commented Sep 10, 2013

I don't see any pthread_atfork_v2 branch...?

@nmathewson
Copy link
Owner

oops. Should be there now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants