Bug 793831: Add socket validity checks to RIL IPC; r=cjones
authorKyle Machulis <kyle@nonpolynomial.com>
Tue, 25 Sep 2012 11:46:54 -0700
changeset 108180 c88496e8454fdcf57b9c69282cecc89aefb57ee1
parent 108179 b3e493a66d853878eeb4e89e9770219373a46999
child 108181 c544d6a5e993a66385d9579575662222284ad00c
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewerscjones
bugs793831
milestone18.0a1
Bug 793831: Add socket validity checks to RIL IPC; r=cjones
ipc/ril/Ril.cpp
--- a/ipc/ril/Ril.cpp
+++ b/ipc/ril/Ril.cpp
@@ -121,30 +121,34 @@ private:
 CancelableTask* RilReconnectTask::sTask;
 
 void RilReconnectTask::Run() {
     // NB: the order of these two statements is important!  sTask must
     // always run, whether we've been canceled or not, to avoid
     // leading a dangling pointer in sTask.
     sTask = nullptr;
     if (mCanceled) {
-        return;
-    }
-
-    if (sClient->OpenSocket()) {
-        return;
+        if (sClient->OpenSocket()) {
+            return;
+        }
     }
     Enqueue(1000);
 }
 
-class RilWriteTask : public Task {
+class RilWriteTask : public Task
+{
     virtual void Run();
 };
 
-void RilWriteTask::Run() {
+void RilWriteTask::Run()
+{
+    if(sClient->mSocket.get() < 0) {
+        NS_WARNING("Trying to write to non-open socket!");
+        return;
+    }
     sClient->OnFileCanWriteWithoutBlocking(sClient->mSocket.rwget());
 }
 
 static void
 ConnectToRil(Monitor* aMonitor, bool* aSuccess)
 {
     MOZ_ASSERT(!sClient);
 
@@ -156,79 +160,80 @@ ConnectToRil(Monitor* aMonitor, bool* aS
         lock.Notify();
     }
     // aMonitor may have gone out of scope by now, don't touch it
 }
 
 bool
 RilClient::OpenSocket()
 {
+    ScopedClose sck;
 #if defined(MOZ_WIDGET_GONK)
     // Using a network socket to test basic functionality
     // before we see how this works on the phone.
     struct sockaddr_un addr;
     socklen_t alen;
     size_t namelen;
     int err;
     memset(&addr, 0, sizeof(addr));
     strcpy(addr.sun_path, RIL_SOCKET_NAME);
     addr.sun_family = AF_LOCAL;
-    mSocket.reset(socket(AF_LOCAL, SOCK_STREAM, 0));
+    sck = socket(AF_LOCAL, SOCK_STREAM, 0);
     alen = strlen(RIL_SOCKET_NAME) + offsetof(struct sockaddr_un, sun_path) + 1;
 #else
     struct hostent *hp;
     struct sockaddr_in addr;
     socklen_t alen;
 
     hp = gethostbyname("localhost");
     if (hp == 0) return false;
 
     memset(&addr, 0, sizeof(addr));
     addr.sin_family = hp->h_addrtype;
     addr.sin_port = htons(RIL_TEST_PORT);
     memcpy(&addr.sin_addr, hp->h_addr, hp->h_length);
-    mSocket.reset(socket(hp->h_addrtype, SOCK_STREAM, 0));
+    sck = socket(hp->h_addrtype, SOCK_STREAM, 0);
     alen = sizeof(addr);
 #endif
 
-    if (mSocket.get() < 0) {
+    if (sck < 0) {
         LOG("Cannot create socket for RIL!\n");
         return false;
     }
 
-    if (connect(mSocket.get(), (struct sockaddr *) &addr, alen) < 0) {
+    if (connect(sck, (struct sockaddr *) &addr, alen) < 0) {
 #if defined(MOZ_WIDGET_GONK)
         LOG("Cannot open socket for RIL!\n");
 #endif
-        mSocket.dispose();
         return false;
     }
 
     // Set close-on-exec bit.
-    int flags = fcntl(mSocket.get(), F_GETFD);
+    int flags = fcntl(sck, F_GETFD);
     if (-1 == flags) {
         return false;
     }
 
     flags |= FD_CLOEXEC;
-    if (-1 == fcntl(mSocket.get(), F_SETFD, flags)) {
+    if (-1 == fcntl(sck, F_SETFD, flags)) {
         return false;
     }
 
     // Select non-blocking IO.
-    if (-1 == fcntl(mSocket.get(), F_SETFL, O_NONBLOCK)) {
+    if (-1 == fcntl(sck, F_SETFL, O_NONBLOCK)) {
         return false;
     }
-    if (!mIOLoop->WatchFileDescriptor(mSocket.get(),
+    if (!mIOLoop->WatchFileDescriptor(sck,
                                       true,
                                       MessageLoopForIO::WATCH_READ,
                                       &mReadWatcher,
                                       this)) {
         return false;
     }
+    mSocket.reset(sck.forget());
     LOG("Socket open for RIL\n");
     return true;
 }
 
 void
 RilClient::OnFileCanReadWithoutBlocking(int fd)
 {
     // Keep reading data until either
@@ -256,17 +261,17 @@ RilClient::OnFileCanReadWithoutBlocking(
                     // else fall through to error handling on other errno's
                 }
                 LOG("Cannot read from network, error %d\n", ret);
                 // At this point, assume that we can't actually access
                 // the socket anymore, and start a reconnect loop.
                 mIncoming.forget();
                 mReadWatcher.StopWatchingFileDescriptor();
                 mWriteWatcher.StopWatchingFileDescriptor();
-                close(mSocket.get());
+                mSocket.reset(-1);
                 RilReconnectTask::Enqueue();
                 return;
             }
             mIncoming->mSize = ret;
             sConsumer->MessageReceived(mIncoming.forget());
             if (ret < ssize_t(RilRawData::MAX_DATA_SIZE)) {
                 return;
             }