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

Race condition leading to tampering with a socket we no longer own in libboostasio.h #479

Open
devgs opened this issue Aug 31, 2022 · 0 comments

Comments

@devgs
Copy link

devgs commented Aug 31, 2022

Describe the bug
Due to an async nature of libboostasio.h we may get into a situation when Watcher outlives the call to monitor() that uninstalls the descriptor watch (flags == 0). Leading to a use of a socket that was closed by caller of monitor (TcpExtState::cleanup) and, immediately after, created by another entity (in POSIX, descriptor numbers are reused).

Specifically, problematic here is a use of _socket.release() that unsubscribes the socket's descriptor from a reactor (epoll/kqueue/...) when the file descriptor is in use by another asio socket, basically making it inoperable.

Expected behavior and actual behavior
Don't tamper with file descriptors that you no longer own.

Sample code

It's really hard to provide a sample. It's always hard to reproduce a race. And, as usual, it only happens in production :).
And obviously, to reproduce it you have to have at least two threads. In our case it's the other thread that manages to create a socket with a descriptor that was just close()-d by amqpcpp's thread, but before it was release() in a Wrapper's destructor.

It's easier to suggest a fix: either use a POSIX dup() or better: release the socket immediately, not relying for the Wrapper's destructor to be invoked:

            // the watcher does already exist, but we no longer have to watch this watcher
            iter->second.release(); // <------- HERE
            _watchers.erase(iter);

Obviously, you also have to add a release() function to Watcher.

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

1 participant