From: Tomas Hozza <thozza@redhat.com>
Date: Mon, 28 Jan 2013 05:43:11 +0000 (-0700)
Subject: Fix various Disk I/O issues in all modules
X-Git-Tag: SQUID_3_2_7~17
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9bf3d517cb8eeb3485bb5c81959087856e55ff4d;p=thirdparty%2Fsquid.git

Fix various Disk I/O issues in all modules

* Uninitialized class members.

* Handle NULL potential after several dynamic_cast.

* Better error result handling from several system functions
  lseek(), fcntl() can report errors which need handling.

* diskd explicit NULL dereference on broken input.
  Extremely unlikely, but worth protecting against.

 Detected by Coverity Scan. Issues 740510, 740358, 740359,
  740511, 740317, 740360, 740513, 740318, 740514
---

diff --git a/src/DiskIO/AIO/AIODiskIOStrategy.cc b/src/DiskIO/AIO/AIODiskIOStrategy.cc
index ef1777093d..7efb1fa436 100644
--- a/src/DiskIO/AIO/AIODiskIOStrategy.cc
+++ b/src/DiskIO/AIO/AIODiskIOStrategy.cc
@@ -49,9 +49,12 @@
 #include "DiskIO/ReadRequest.h"
 #include "DiskIO/WriteRequest.h"
 
-AIODiskIOStrategy::AIODiskIOStrategy()
+AIODiskIOStrategy::AIODiskIOStrategy() :
+        fd(-1)
 {
+    aq.aq_state = AQ_STATE_NONE;
     aq.aq_numpending = 0;
+    memset(&aq.aq_queue, 0, sizeof(aq.aq_queue));
 }
 
 AIODiskIOStrategy::~AIODiskIOStrategy()
diff --git a/src/DiskIO/DiskDaemon/DiskdFile.cc b/src/DiskIO/DiskDaemon/DiskdFile.cc
index 5946c8d0ab..e461f753e8 100644
--- a/src/DiskIO/DiskDaemon/DiskdFile.cc
+++ b/src/DiskIO/DiskDaemon/DiskdFile.cc
@@ -69,8 +69,11 @@ DiskdFile::operator delete(void *address)
     cbdataFree(t);
 }
 
-DiskdFile::DiskdFile(char const *aPath, DiskdIOStrategy *anIO) : errorOccured (false), IO(anIO),
-        inProgressIOs (0)
+DiskdFile::DiskdFile(char const *aPath, DiskdIOStrategy *anIO) :
+        errorOccured(false),
+        IO(anIO),
+        mode(0),
+        inProgressIOs(0)
 {
     assert (aPath);
     debugs(79, 3, "DiskdFile::DiskdFile: " << aPath);
@@ -382,8 +385,10 @@ DiskdFile::readDone(diomsg * M)
     debugs(79, 3, "DiskdFile::readDone: status " << M->status);
     assert (M->requestor);
     ReadRequest::Pointer readRequest = dynamic_cast<ReadRequest *>(M->requestor);
+
     /* remove the free protection */
-    readRequest->RefCountDereference();
+    if (readRequest != NULL)
+        readRequest->RefCountDereference();
 
     if (M->status < 0) {
         ++diskd_stats.read.fail;
@@ -407,7 +412,8 @@ DiskdFile::writeDone(diomsg *M)
     assert (M->requestor);
     WriteRequest::Pointer writeRequest = dynamic_cast<WriteRequest *>(M->requestor);
     /* remove the free protection */
-    writeRequest->RefCountDereference();
+    if (writeRequest != NULL)
+        writeRequest->RefCountDereference();
 
     if (M->status < 0) {
         errorOccured = true;
diff --git a/src/DiskIO/DiskDaemon/diskd.cc b/src/DiskIO/DiskDaemon/diskd.cc
index e38bcbfc82..60f844eba2 100644
--- a/src/DiskIO/DiskDaemon/diskd.cc
+++ b/src/DiskIO/DiskDaemon/diskd.cc
@@ -266,6 +266,10 @@ msg_handle(diomsg * r, int rl, diomsg * s)
 
     if (s->shm_offset > -1)
         buf = shmbuf + s->shm_offset;
+    else {
+        fprintf(stderr, "%d UNLNK id(%u) Error: no filename in shm buffer\n", (int) mypid, s->id);
+        return;
+    }
 
     switch (r->mtype) {
 
@@ -370,7 +374,10 @@ main(int argc, char *argv[])
 
     hash = hash_create(fsCmp, 1 << 4, fsHash);
     assert(hash);
-    fcntl(0, F_SETFL, SQUID_NONBLOCK);
+    if (fcntl(0, F_SETFL, SQUID_NONBLOCK) < 0) {
+        perror(xstrerror());
+        return 1;
+    }
     memset(&sa, '\0', sizeof(sa));
     sa.sa_handler = alarm_handler;
     sa.sa_flags = SA_RESTART;
diff --git a/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc b/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc
index 296e3f4ca2..81d2cd2190 100644
--- a/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc
+++ b/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc
@@ -190,7 +190,10 @@ DiskThreadsIOStrategy::sync()
     debugs(32, 2, "aioSync: done");
 }
 
-DiskThreadsIOStrategy::DiskThreadsIOStrategy() :  initialised (false) {}
+DiskThreadsIOStrategy::DiskThreadsIOStrategy() :
+        initialised(false),
+        squidaio_ctrl_pool(NULL)
+{}
 
 void
 DiskThreadsIOStrategy::aioStats(StoreEntry * sentry)
diff --git a/src/DiskIO/DiskThreads/aiops.cc b/src/DiskIO/DiskThreads/aiops.cc
index 6fa7be16c3..b04fb422d0 100644
--- a/src/DiskIO/DiskThreads/aiops.cc
+++ b/src/DiskIO/DiskThreads/aiops.cc
@@ -729,8 +729,10 @@ squidaio_read(int fd, char *bufp, size_t bufs, off_t offset, int whence, squidai
 static void
 squidaio_do_read(squidaio_request_t * requestp)
 {
-    lseek(requestp->fd, requestp->offset, requestp->whence);
-    requestp->ret = read(requestp->fd, requestp->bufferp, requestp->buflen);
+    if (lseek(requestp->fd, requestp->offset, requestp->whence) >= 0)
+        requestp->ret = read(requestp->fd, requestp->bufferp, requestp->buflen);
+    else
+        requestp->ret = -1;
     requestp->err = errno;
 }
 
diff --git a/src/DiskIO/IpcIo/IpcIoFile.cc b/src/DiskIO/IpcIo/IpcIoFile.cc
index b27a5d088d..b0386b887c 100644
--- a/src/DiskIO/IpcIo/IpcIoFile.cc
+++ b/src/DiskIO/IpcIo/IpcIoFile.cc
@@ -584,9 +584,14 @@ IpcIoFile::getFD() const
 /* IpcIoMsg */
 
 IpcIoMsg::IpcIoMsg():
-        requestId(0), offset(0), len(0), command(IpcIo::cmdNone), xerrno(0)
+        requestId(0),
+        offset(0),
+        len(0),
+        command(IpcIo::cmdNone),
+        xerrno(0)
 {
     start.tv_sec = 0;
+    start.tv_usec = 0;
 }
 
 /* IpcIoPendingRequest */