Bug 793831: Add socket validity checks to RIL IPC; r=cjones
authorKyle Machulis <kyle@nonpolynomial.com>
Sun, 30 Sep 2012 22:03:43 -0700
changeset 108822 1983304d2e60e8de09205c7e1f724109afdcc606
parent 108821 04cebae27c5e58509c053ad0f4965f8cbec75cba
child 108823 e8e7e0cf43d85ddca5dcde5054df6c5abe02a481
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
@@ -88,17 +88,21 @@ class RilReconnectTask : public Cancelab
     virtual void Run();
     virtual void Cancel() { mCanceled = true; }
 
     bool mCanceled;
 
 public:
     static void Enqueue(int aDelayMs = 0) {
         MessageLoopForIO* ioLoop = MessageLoopForIO::current();
-        MOZ_ASSERT(ioLoop && sClient->mIOLoop == ioLoop);
+        if (!ioLoop) {
+            NS_WARNING("No IOLoop to attach to, cancelling self!");
+            CancelIt();
+            return;
+        }
         if (sTask) {
             return;
         }
         sTask = new RilReconnectTask();
         if (aDelayMs) {
             ioLoop->PostDelayedTask(FROM_HERE, sTask, aDelayMs);
         } else {
             ioLoop->PostTask(FROM_HERE, sTask);
@@ -135,16 +139,20 @@ void RilReconnectTask::Run() {
     Enqueue(1000);
 }
 
 class RilWriteTask : public Task {
     virtual void 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 +164,82 @@ ConnectToRil(Monitor* aMonitor, bool* aS
         lock.Notify();
     }
     // aMonitor may have gone out of scope by now, don't touch it
 }
 
 bool
 RilClient::OpenSocket()
 {
+
+    ScopedClose skt;
 #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));
+    skt.reset(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));
+    skt.reset(socket(hp->h_addrtype, SOCK_STREAM, 0));
     alen = sizeof(addr);
 #endif
 
-    if (mSocket.get() < 0) {
+    if (skt.get() < 0) {
         LOG("Cannot create socket for RIL!\n");
         return false;
     }
 
-    if (connect(mSocket.get(), (struct sockaddr *) &addr, alen) < 0) {
+    if (connect(skt.get(), (struct sockaddr *) &addr, alen) < 0) {
 #if defined(MOZ_WIDGET_GONK)
         LOG("Cannot open socket for RIL!\n");
 #endif
-        mSocket.dispose();
+        skt.dispose();
         return false;
     }
 
     // Set close-on-exec bit.
-    int flags = fcntl(mSocket.get(), F_GETFD);
+    int flags = fcntl(skt.get(), F_GETFD);
     if (-1 == flags) {
         return false;
     }
 
     flags |= FD_CLOEXEC;
-    if (-1 == fcntl(mSocket.get(), F_SETFD, flags)) {
+    if (-1 == fcntl(skt.get(), F_SETFD, flags)) {
         return false;
     }
 
     // Select non-blocking IO.
-    if (-1 == fcntl(mSocket.get(), F_SETFL, O_NONBLOCK)) {
+    if (-1 == fcntl(skt.get(), F_SETFL, O_NONBLOCK)) {
         return false;
     }
-    if (!mIOLoop->WatchFileDescriptor(mSocket.get(),
+    if (!mIOLoop->WatchFileDescriptor(skt.get(),
                                       true,
                                       MessageLoopForIO::WATCH_READ,
                                       &mReadWatcher,
                                       this)) {
         return false;
     }
+    mSocket = skt.forget();
     LOG("Socket open for RIL\n");
     return true;
 }
 
 void
 RilClient::OnFileCanReadWithoutBlocking(int fd)
 {
     // Keep reading data until either
@@ -256,17 +267,19 @@ 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());
+                // ScopedClose will close our old socket on a reset.
+                // Setting to -1 means writes will fail with message.
+                mSocket.reset(-1);
                 RilReconnectTask::Enqueue();
                 return;
             }
             mIncoming->mSize = ret;
             sConsumer->MessageReceived(mIncoming.forget());
             if (ret < ssize_t(RilRawData::MAX_DATA_SIZE)) {
                 return;
             }