From 63ae33eca737264dfdad538fc4c17350ab7d8421 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Tue, 8 Jan 2013 23:43:25 +0100 Subject: [PATCH] Fix crash in AICurlPrivate::curlthread::PollSet::next Introduced in https://github.com/AlericInglewood/SingularityViewer/commit/6dcda3595e5ff176ec05c7156a68413390636ed1 (Add recovery for randomly closed socket desciptors.) By copying CurlSocketInfo* into mCopiedFileDescriptors, it was possible that we accessed a deleted CurlSocketInfo for it's filedescriptor, returning a random value, which, when passed to FD_ISSET could cause a SIGSEGV. I reverted this change and now store the literal socket descriptors in mCopiedFileDescriptors again. It is not a problem when libcurl deletes the corresponding CurlSocketInfo (through a callback to MultiHandle::socket_callback) in that case, because the fd_set we test against isn't updated as a result of that, not until we're down with mCopiedFileDescriptors (at the re-entry of select()). --- indra/llmessage/aicurlthread.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index 8b54fcf81..6b67733bd 100644 --- a/indra/llmessage/aicurlthread.cpp +++ b/indra/llmessage/aicurlthread.cpp @@ -353,8 +353,8 @@ class PollSet #if !WINDOWS_CODE curl_socket_t mMaxFd; // The largest filedescriptor in the array, or CURL_SOCKET_BAD when it is empty. curl_socket_t mMaxFdSet; // The largest filedescriptor set in mFdSet by refresh(), or CURL_SOCKET_BAD when it was empty. - std::vector mCopiedFileDescriptors; // Filedescriptors copied by refresh to mFdSet. - std::vector::iterator mIter; // Index into mCopiedFileDescriptors for next(); loop variable. + std::vector mCopiedFileDescriptors; // Filedescriptors copied by refresh to mFdSet. + std::vector::iterator mIter; // Index into mCopiedFileDescriptors for next(); loop variable. #else unsigned int mIter; // Index into fd_set::fd_array. #endif @@ -580,7 +580,7 @@ refresh_t PollSet::refresh(void) } FD_SET(mFileDescriptors[i]->getSocketFd(), &mFdSet); #if !WINDOWS_CODE - mCopiedFileDescriptors.push_back(mFileDescriptors[i]); + mCopiedFileDescriptors.push_back(mFileDescriptors[i]->getSocketFd()); #endif if (++i == mNrFds) { @@ -625,7 +625,7 @@ void PollSet::reset(void) else { mIter = mCopiedFileDescriptors.begin(); - if (!FD_ISSET((*mIter)->getSocketFd(), &mFdSet)) + if (!FD_ISSET(*mIter, &mFdSet)) next(); } #endif @@ -636,7 +636,7 @@ inline curl_socket_t PollSet::get(void) const #if WINDOWS_CODE return (mIter >= mFdSet.fd_count) ? CURL_SOCKET_BAD : mFdSet.fd_array[mIter]; #else - return (mIter == mCopiedFileDescriptors.end()) ? CURL_SOCKET_BAD : (*mIter)->getSocketFd(); + return (mIter == mCopiedFileDescriptors.end()) ? CURL_SOCKET_BAD : *mIter; #endif } @@ -647,7 +647,7 @@ void PollSet::next(void) ++mIter; #else llassert(mIter != mCopiedFileDescriptors.end()); // Only call next() if the last call to get() didn't return -1. - while (++mIter != mCopiedFileDescriptors.end() && !FD_ISSET((*mIter)->getSocketFd(), &mFdSet)); + while (++mIter != mCopiedFileDescriptors.end() && !FD_ISSET(*mIter, &mFdSet)); #endif }