Merge pull request #391 from vgough/pr/380

Return code rework

.travis.yml

@@ -5,7 +5,7 @@ matrix:
     - os: linux
       compiler: gcc
       dist: trusty
-      sudo: false
+      sudo: required
       addons:
         apt:
           sources:
@@ -17,12 +17,12 @@ matrix:
             - gettext
             - cmake3
       env:
-        - INTEGRATION=false
+        - SUDO_TESTS=true
 
     - os: linux
       compiler: clang
       dist: trusty
-      sudo: true
+      sudo: false
       addons:
         apt:
           sources:
@@ -37,14 +37,14 @@ matrix:
             - clang-4.0
             - clang-tidy-4.0
       env:
-        - CC=clang-4.0 CXX=clang++-4.0 CHECK=true INTEGRATION=true
+        - CC=clang-4.0 CXX=clang++-4.0 CHECK=true INTEGRATION=false CMAKE=/tmp/bin/cmake
 
     - os: osx
       compiler: clang
       osx_image: xcode8.3
-      sudo: true
+      sudo: required
       env:
-        - INTEGRATION=true
+        - SUDO_TESTS=true
 
 before_script:
   - ./ci/setup.sh

CMakeLists.txt

@@ -168,7 +168,21 @@ if (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} GREATER 3.5) # Need 3.6 or abo
     message(STATUS "clang-tidy not found.")
   else()
     message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}")
-    set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-checks=*,-modernize-loop-convert,-cppcoreguidelines-pro-*,-readability-inconsistent-declaration-parameter-name,-google-readability-casting,-cert-err58-cpp,-google-runtime-int,-readability-named-parameter,-google-build-using-namespace,-misc-unused-parameters,-google-runtime-references")
+    string(CONCAT TIDY_OPTS "-checks=*"
+      ",-cert-err58-cpp"
+      ",-cppcoreguidelines-pro-*"
+      ",-google-build-using-namespace"
+      ",-google-readability-casting"
+      ",-google-readability-todo"
+      ",-google-runtime-int"
+      ",-google-runtime-references"
+      ",-misc-misplaced-widening-cast"
+      ",-misc-unused-parameters"
+      ",-modernize-loop-convert"
+      ",-readability-inconsistent-declaration-parameter-name"
+      ",-readability-named-parameter"
+      )
+    set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" ${TIDY_OPTS})
     #set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-checks=-*,google-readability-redundant-smartptr-get")
   endif()
 else()

build.sh

@@ -1,9 +1,10 @@
 #!/bin/bash -eu
 
+: ${CMAKE:=cmake}
 : ${CHECK:=false}
 : ${INTEGRATION:=true}
 
-cmake --version
+${CMAKE} --version
 
 CFG=$*
 if [[ "$CHECK" == "true" ]]; then
@@ -20,7 +21,7 @@ then
 fi
 
 cd build
-cmake .. ${CFG}
+${CMAKE} .. ${CFG}
 make -j2
 make test
 if [[ "$INTEGRATION" == "true" ]]; then

ci/setup.sh

@@ -1,6 +1,6 @@
 #!/bin/bash -eu
 
-: ${INTEGRATION:=false}
+: ${INTEGRATION:=true}
 : ${CHECK:=false}
 
 if [[ "$INTEGRATION" == "true" ]]; then
@@ -15,8 +15,8 @@ if [[ "$CHECK" == "true" ]]; then
   if uname -s | grep -q Linux; then
     wget https://cmake.org/files/v3.9/cmake-3.9.1-Linux-x86_64.tar.gz -O /tmp/cmake.tar.gz
     tar -C /tmp/ -xf /tmp/cmake.tar.gz
-    sudo rm -f $(which cmake)
+    mkdir /tmp/bin
-    sudo ln -s $(ls -1 /tmp/cmake*/bin/cmake) /bin/
+    ln -s $(ls -1 /tmp/cmake*/bin/cmake) /tmp/bin/
   fi
 fi
 

encfs/BlockFileIO.cpp

@@ -69,7 +69,7 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const {
    * the lower file may have changed behind our back. */
   if ((!_noCache) && (req.offset == _cache.offset) && (_cache.dataLen != 0)) {
     // satisfy request from cache
-    int len = req.dataLen;
+    size_t len = req.dataLen;
     if (_cache.dataLen < len) {
       len = _cache.dataLen;  // Don't read past EOF
     }
@@ -97,17 +97,17 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const {
   return result;
 }
 
-bool BlockFileIO::cacheWriteOneBlock(const IORequest &req) {
+ssize_t BlockFileIO::cacheWriteOneBlock(const IORequest &req) {
   // cache results of write (before pass-thru, because it may be modified
   // in-place)
   memcpy(_cache.data, req.data, req.dataLen);
   _cache.offset = req.offset;
   _cache.dataLen = req.dataLen;
-  bool ok = writeOneBlock(req);
+  ssize_t res = writeOneBlock(req);
-  if (!ok) {
+  if (res < 0) {
     clearCache(_cache, _blockSize);
   }
-  return ok;
+  return res;
 }
 
 /**
@@ -116,11 +116,13 @@ bool BlockFileIO::cacheWriteOneBlock(const IORequest &req) {
  * data from the front of the first block if the request is not aligned.
  * Always requests aligned data of the size of one block or less from the
  * lower layer.
+ * Returns the number of bytes read, or -errno in case of failure.
  */
 ssize_t BlockFileIO::read(const IORequest &req) const {
   CHECK(_blockSize != 0);
 
-  int partialOffset = req.offset % _blockSize;
+  int partialOffset =
+      req.offset % _blockSize;  // can be int as _blockSize is int
   off_t blockNum = req.offset / _blockSize;
   ssize_t result = 0;
 
@@ -154,11 +156,15 @@ ssize_t BlockFileIO::read(const IORequest &req) const {
     }
 
     ssize_t readSize = cacheReadOneBlock(blockReq);
+    if (readSize < 0) {
+      result = readSize;
+      break;
+    }
     if (readSize <= partialOffset) {
       break;  // didn't get enough bytes
     }
 
-    int cpySize = min((size_t)(readSize - partialOffset), size);
+    size_t cpySize = min((size_t)(readSize - partialOffset), size);
     CHECK(cpySize <= readSize);
 
     // if we read to a temporary buffer, then move the data
@@ -184,21 +190,25 @@ ssize_t BlockFileIO::read(const IORequest &req) const {
   return result;
 }
 
-bool BlockFileIO::write(const IORequest &req) {
+/**
+ * Returns the number of bytes written, or -errno in case of failure.
+ */
+ssize_t BlockFileIO::write(const IORequest &req) {
   CHECK(_blockSize != 0);
 
   off_t fileSize = getSize();
   if (fileSize < 0) {
-    return false;
+    return fileSize;
   }
 
   // where write request begins
   off_t blockNum = req.offset / _blockSize;
-  int partialOffset = req.offset % _blockSize;
+  int partialOffset =
+      req.offset % _blockSize;  // can be int as _blockSize is int
 
   // last block of file (for testing write overlaps with file boundary)
   off_t lastFileBlock = fileSize / _blockSize;
-  ssize_t lastBlockSize = fileSize % _blockSize;
+  size_t lastBlockSize = fileSize % _blockSize;
 
   off_t lastNonEmptyBlock = lastFileBlock;
   if (lastBlockSize == 0) {
@@ -208,7 +218,10 @@ bool BlockFileIO::write(const IORequest &req) {
   if (req.offset > fileSize) {
     // extend file first to fill hole with 0's..
     const bool forceWrite = false;
-    padFile(fileSize, req.offset, forceWrite);
+    int res = padFile(fileSize, req.offset, forceWrite);
+    if (res < 0) {
+      return res;
+    }
   }
 
   // check against edge cases where we can just let the base class handle the
@@ -233,7 +246,7 @@ bool BlockFileIO::write(const IORequest &req) {
   blockReq.data = nullptr;
   blockReq.dataLen = _blockSize;
 
-  bool ok = true;
+  ssize_t res = 0;
   size_t size = req.dataLen;
   unsigned char *inPtr = req.data;
   while (size != 0u) {
@@ -258,11 +271,16 @@ bool BlockFileIO::write(const IORequest &req) {
 
       if (blockNum > lastNonEmptyBlock) {
         // just pad..
-        blockReq.dataLen = toCopy + partialOffset;
+        blockReq.dataLen = partialOffset + toCopy;
       } else {
         // have to merge with existing block data..
         blockReq.dataLen = _blockSize;
-        blockReq.dataLen = cacheReadOneBlock(blockReq);
+        ssize_t readSize = cacheReadOneBlock(blockReq);
+        if (readSize < 0) {
+          res = readSize;
+          break;
+        }
+        blockReq.dataLen = readSize;
 
         // extend data if necessary..
         if (partialOffset + toCopy > blockReq.dataLen) {
@@ -274,8 +292,8 @@ bool BlockFileIO::write(const IORequest &req) {
     }
 
     // Finally, write the damn thing!
-    if (!cacheWriteOneBlock(blockReq)) {
+    res = cacheWriteOneBlock(blockReq);
-      ok = false;
+    if (res < 0) {
       break;
     }
 
@@ -290,15 +308,22 @@ bool BlockFileIO::write(const IORequest &req) {
     MemoryPool::release(mb);
   }
 
-  return ok;
+  if (res < 0) {
+    return res;
+  }
+  return req.dataLen;
 }
 
 int BlockFileIO::blockSize() const { return _blockSize; }
 
-void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) {
+/**
+ * Returns 0 in case of success, or -errno in case of failure.
+ */
+int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) {
   off_t oldLastBlock = oldSize / _blockSize;
   off_t newLastBlock = newSize / _blockSize;
-  int newBlockSize = newSize % _blockSize;
+  int newBlockSize = newSize % _blockSize;  // can be int as _blockSize is int
+  ssize_t res = 0;
 
   IORequest req;
   MemBlock mb;
@@ -317,9 +342,10 @@ void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) {
 
       if (outSize != 0) {
         memset(mb.data, 0, outSize);
-        cacheReadOneBlock(req);
+        if ((res = cacheReadOneBlock(req)) >= 0) {
-        req.dataLen = outSize;
+          req.dataLen = outSize;
-        cacheWriteOneBlock(req);
+          res = cacheWriteOneBlock(req);
+        }
       }
     } else
       VLOG(1) << "optimization: not padding last block";
@@ -338,39 +364,48 @@ void BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) {
     if (req.dataLen != 0) {
       VLOG(1) << "padding block " << oldLastBlock;
       memset(mb.data, 0, _blockSize);
-      cacheReadOneBlock(req);
+      if ((res = cacheReadOneBlock(req)) >= 0) {
-      req.dataLen = _blockSize;  // expand to full block size
+        req.dataLen = _blockSize;  // expand to full block size
-      cacheWriteOneBlock(req);
+        res = cacheWriteOneBlock(req);
+      }
       ++oldLastBlock;
     }
 
     // 2, pad zero blocks unless holes are allowed
     if (!_allowHoles) {
-      for (; oldLastBlock != newLastBlock; ++oldLastBlock) {
+      for (; (res >= 0) && (oldLastBlock != newLastBlock); ++oldLastBlock) {
         VLOG(1) << "padding block " << oldLastBlock;
         req.offset = oldLastBlock * _blockSize;
         req.dataLen = _blockSize;
         memset(mb.data, 0, req.dataLen);
-        cacheWriteOneBlock(req);
+        res = cacheWriteOneBlock(req);
       }
     }
 
     // 3. only necessary if write is forced and block is non 0 length
-    if (forceWrite && (newBlockSize != 0)) {
+    if ((res >= 0) && forceWrite && (newBlockSize != 0)) {
       req.offset = newLastBlock * _blockSize;
       req.dataLen = newBlockSize;
       memset(mb.data, 0, req.dataLen);
-      cacheWriteOneBlock(req);
+      res = cacheWriteOneBlock(req);
     }
   }
 
   if (mb.data != nullptr) {
     MemoryPool::release(mb);
   }
+
+  if (res < 0) {
+    return res;
+  }
+  return 0;
 }
 
+/**
+ * Returns 0 in case of success, or -errno in case of failure.
+ */
 int BlockFileIO::truncateBase(off_t size, FileIO *base) {
-  int partialBlock = size % _blockSize;
+  int partialBlock = size % _blockSize;  // can be int as _blockSize is int
   int res = 0;
 
   off_t oldSize = getSize();
@@ -381,11 +416,13 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) {
     // do the truncate so that the underlying filesystem can allocate
     // the space, and then we'll fill it in padFile..
     if (base != nullptr) {
-      base->truncate(size);
+      res = base->truncate(size);
     }
 
     const bool forceWrite = true;
-    padFile(oldSize, size, forceWrite);
+    if (res == 0) {
+      res = padFile(oldSize, size, forceWrite);
+    }
   } else if (size == oldSize) {
     // the easiest case, but least likely....
   } else if (partialBlock != 0) {
@@ -400,21 +437,23 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) {
     req.dataLen = _blockSize;
     req.data = mb.data;
 
-    ssize_t rdSz = cacheReadOneBlock(req);
+    ssize_t readSize = cacheReadOneBlock(req);
+    if (readSize < 0) {
+      res = readSize;
+    }
 
-    // do the truncate
+    else if (base != nullptr) {
-    if (base != nullptr) {
+      // do the truncate
       res = base->truncate(size);
     }
 
     // write back out partial block
     req.dataLen = partialBlock;
-    bool wrRes = cacheWriteOneBlock(req);
+    if (res == 0) {
-
+      ssize_t writeSize = cacheWriteOneBlock(req);
-    if ((rdSz < 0) || (!wrRes)) {
+      if (writeSize < 0) {
-      // rwarning - unlikely to ever occur..
+        res = writeSize;
-      RLOG(WARNING) << "truncate failure: read " << rdSz
+      }
-                    << " bytes, partial block of " << partialBlock;
     }
 
     MemoryPool::release(mb);

encfs/BlockFileIO.h

@@ -43,21 +43,21 @@ class BlockFileIO : public FileIO {
 
   // implemented in terms of blocks.
   virtual ssize_t read(const IORequest &req) const;
-  virtual bool write(const IORequest &req);
+  virtual ssize_t write(const IORequest &req);
 
   virtual int blockSize() const;
 
  protected:
   int truncateBase(off_t size, FileIO *base);
-  void padFile(off_t oldSize, off_t newSize, bool forceWrite);
+  int padFile(off_t oldSize, off_t newSize, bool forceWrite);
 
   // same as read(), except that the request.offset field is guarenteed to be
   // block aligned, and the request size will not be larger then 1 block.
   virtual ssize_t readOneBlock(const IORequest &req) const = 0;
-  virtual bool writeOneBlock(const IORequest &req) = 0;
+  virtual ssize_t writeOneBlock(const IORequest &req) = 0;
 
   ssize_t cacheReadOneBlock(const IORequest &req) const;
-  bool cacheWriteOneBlock(const IORequest &req);
+  ssize_t cacheWriteOneBlock(const IORequest &req);
 
   int _blockSize;
   bool _allowHoles;

encfs/BlockNameIO.cpp

@@ -163,8 +163,12 @@ int BlockNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv,
   encodedName[0] = (mac >> 8) & 0xff;
   encodedName[1] = (mac)&0xff;
 
-  _cipher->blockEncode((unsigned char *)encodedName + 2, length + padding,
+  bool ok;
-                       (uint64_t)mac ^ tmpIV, _key);
+  ok = _cipher->blockEncode((unsigned char *)encodedName + 2, length + padding,
+                            (uint64_t)mac ^ tmpIV, _key);
+  if (!ok) {
+    throw Error("block encode failed in filename encode");
+  }
 
   // convert to base 64 ascii
   int encodedStreamLen = length + 2 + padding;
@@ -219,8 +223,12 @@ int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv,
     tmpIV = *iv;
   }
 
-  _cipher->blockDecode((unsigned char *)tmpBuf + 2, decodedStreamLen,
+  bool ok;
-                       (uint64_t)mac ^ tmpIV, _key);
+  ok = _cipher->blockDecode((unsigned char *)tmpBuf + 2, decodedStreamLen,
+                            (uint64_t)mac ^ tmpIV, _key);
+  if (!ok) {
+    throw Error("block decode failed in filename decode");
+  }
 
   // find out true string length
   int padding = (unsigned char)tmpBuf[2 + decodedStreamLen - 1];

encfs/CipherFileIO.cpp

@@ -98,20 +98,22 @@ bool CipherFileIO::setIV(uint64_t iv) {
   } else if (haveHeader) {
     // we have an old IV, and now a new IV, so we need to update the fileIV
     // on disk.
+    // ensure the file is open for read/write..
+    int newFlags = lastFlags | O_RDWR;
+    int res = base->open(newFlags);
+    if (res < 0) {
+      if (res == -EISDIR) {
+        // duh -- there are no file headers for directories!
+        externalIV = iv;
+        return base->setIV(iv);
+      }
+      VLOG(1) << "setIV failed to re-open for write";
+      return false;
+    }
     if (fileIV == 0) {
-      // ensure the file is open for read/write..
+      if (initHeader() < 0) {
-      int newFlags = lastFlags | O_RDWR;
-      int res = base->open(newFlags);
-      if (res < 0) {
-        if (res == -EISDIR) {
-          // duh -- there are no file headers for directories!
-          externalIV = iv;
-          return base->setIV(iv);
-        }
-        VLOG(1) << "writeHeader failed to re-open for write";
         return false;
       }
-      initHeader();
     }
 
     uint64_t oldIV = externalIV;
@@ -172,7 +174,7 @@ off_t CipherFileIO::getSize() const {
   return size;
 }
 
-void CipherFileIO::initHeader() {
+int CipherFileIO::initHeader() {
   // check if the file has a header, and read it if it does..  Otherwise,
   // create one.
   off_t rawSize = base->getSize();
@@ -185,9 +187,14 @@ void CipherFileIO::initHeader() {
     req.offset = 0;
     req.data = buf;
     req.dataLen = 8;
-    base->read(req);
+    ssize_t readSize = base->read(req);
+    if (readSize < 0) {
+      return readSize;
+    }
 
-    cipher->streamDecode(buf, sizeof(buf), externalIV, key);
+    if (!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) {
+      return -EBADMSG;
+    }
 
     fileIV = 0;
     for (int i = 0; i < 8; ++i) {
@@ -201,7 +208,8 @@ void CipherFileIO::initHeader() {
     unsigned char buf[8] = {0};
     do {
       if (!cipher->randomize(buf, 8, false)) {
-        throw Error("Unable to generate a random file IV");
+        RLOG(ERROR) << "Unable to generate a random file IV";
+        return -EBADMSG;
       }
 
       fileIV = 0;
@@ -215,31 +223,28 @@ void CipherFileIO::initHeader() {
     } while (fileIV == 0);  // don't accept 0 as an option..
 
     if (base->isWritable()) {
-      cipher->streamEncode(buf, sizeof(buf), externalIV, key);
+      if (!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) {
+        return -EBADMSG;
+      }
 
       IORequest req;
       req.offset = 0;
       req.data = buf;
       req.dataLen = 8;
 
-      base->write(req);
+      ssize_t writeSize = base->write(req);
+      if (writeSize < 0) {
+        return writeSize;
+      }
     } else {
       VLOG(1) << "base not writable, IV not written..";
     }
   }
   VLOG(1) << "initHeader finished, fileIV = " << fileIV;
+  return 0;
 }
 
 bool CipherFileIO::writeHeader() {
-  if (!base->isWritable()) {
-    // open for write..
-    int newFlags = lastFlags | O_RDWR;
-    if (base->open(newFlags) < 0) {
-      VLOG(1) << "writeHeader failed to re-open for write";
-      return false;
-    }
-  }
-
   if (fileIV == 0) {
     RLOG(ERROR) << "Internal error: fileIV == 0 in writeHeader!!!";
   }
@@ -251,16 +256,16 @@ bool CipherFileIO::writeHeader() {
     fileIV >>= 8;
   }
 
-  cipher->streamEncode(buf, sizeof(buf), externalIV, key);
+  if (!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) {
+    return false;
+  }
 
   IORequest req;
   req.offset = 0;
   req.data = buf;
   req.dataLen = 8;
 
-  base->write(req);
+  return (base->write(req) >= 0);
-
-  return true;
 }
 
 /**
@@ -275,7 +280,7 @@ bool CipherFileIO::writeHeader() {
  * the IV. This guarantees unpredictability and prevents watermarking
  * attacks.
  */
-void CipherFileIO::generateReverseHeader(unsigned char *headerBuf) {
+int CipherFileIO::generateReverseHeader(unsigned char *headerBuf) {
 
   struct stat stbuf;
   int res = getAttr(&stbuf);
@@ -309,7 +314,10 @@ void CipherFileIO::generateReverseHeader(unsigned char *headerBuf) {
   VLOG(1) << "fileIV=" << fileIV;
 
   // Encrypt externally-visible header
-  cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key);
+  if (!cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key)) {
+    return -EBADMSG;
+  }
+  return 0;
 }
 
 /**
@@ -321,76 +329,90 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const {
   int bs = blockSize();
   off_t blockNum = req.offset / bs;
 
-  ssize_t readSize = 0;
   IORequest tmpReq = req;
 
   // adjust offset if we have a file header
   if (haveHeader && !fsConfig->reverseEncryption) {
     tmpReq.offset += HEADER_SIZE;
   }
-  readSize = base->read(tmpReq);
+  ssize_t readSize = base->read(tmpReq);
 
   bool ok;
   if (readSize > 0) {
     if (haveHeader && fileIV == 0) {
-      const_cast<CipherFileIO *>(this)->initHeader();
+      int res = const_cast<CipherFileIO *>(this)->initHeader();
+      if (res < 0) {
+        return res;
+      }
     }
 
     if (readSize != bs) {
       VLOG(1) << "streamRead(data, " << readSize << ", IV)";
-      ok = streamRead(tmpReq.data, (int)readSize, blockNum ^ fileIV);
+      ok = streamRead(tmpReq.data, (int)readSize,
+                      blockNum ^ fileIV);  // cast works because we work on a
+                                           // block and blocksize fit an int
     } else {
-      ok = blockRead(tmpReq.data, (int)readSize, blockNum ^ fileIV);
+      ok = blockRead(tmpReq.data, (int)readSize,
+                     blockNum ^ fileIV);  // cast works because we work on a
+                                          // block and blocksize fit an int
     }
 
     if (!ok) {
       VLOG(1) << "decodeBlock failed for block " << blockNum << ", size "
               << readSize;
-      readSize = -1;
+      readSize = -EBADMSG;
     }
-  } else {
+  } else if (readSize == 0) {
     VLOG(1) << "readSize zero for offset " << req.offset;
   }
 
   return readSize;
 }
 
-bool CipherFileIO::writeOneBlock(const IORequest &req) {
+ssize_t CipherFileIO::writeOneBlock(const IORequest &req) {
 
   if (haveHeader && fsConfig->reverseEncryption) {
     VLOG(1)
         << "writing to a reverse mount with per-file IVs is not implemented";
-    return false;
+    return -EPERM;
   }
 
   int bs = blockSize();
   off_t blockNum = req.offset / bs;
 
   if (haveHeader && fileIV == 0) {
-    initHeader();
+    int res = initHeader();
+    if (res < 0) {
+      return res;
+    }
   }
 
   bool ok;
   if (req.dataLen != bs) {
-    ok = streamWrite(req.data, (int)req.dataLen, blockNum ^ fileIV);
+    ok = streamWrite(req.data, (int)req.dataLen,
+                     blockNum ^ fileIV);  // cast works because we work on a
+                                          // block and blocksize fit an int
   } else {
-    ok = blockWrite(req.data, (int)req.dataLen, blockNum ^ fileIV);
+    ok = blockWrite(req.data, (int)req.dataLen,
+                    blockNum ^ fileIV);  // cast works because we work on a
+                                         // block and blocksize fit an int
   }
 
+  ssize_t res = 0;
   if (ok) {
     if (haveHeader) {
       IORequest tmpReq = req;
       tmpReq.offset += HEADER_SIZE;
-      ok = base->write(tmpReq);
+      res = base->write(tmpReq);
     } else {
-      ok = base->write(req);
+      res = base->write(req);
     }
   } else {
     VLOG(1) << "encodeBlock failed for block " << blockNum << ", size "
             << req.dataLen;
-    ok = false;
+    res = -EBADMSG;
   }
-  return ok;
+  return res;
 }
 
 bool CipherFileIO::blockWrite(unsigned char *buf, int size,
@@ -439,27 +461,38 @@ bool CipherFileIO::streamRead(unsigned char *buf, int size,
 
 int CipherFileIO::truncate(off_t size) {
   int res = 0;
+  int reopen = 0;
+  // well, we will truncate, so we need a write access to the file
+  if (!base->isWritable()) {
+    int newFlags = lastFlags | O_RDWR;
+    int res = base->open(newFlags);
+    if (res < 0) {
+      VLOG(1) << "truncate failed to re-open for write";
+      base->open(lastFlags);
+      return res;
+    }
+    reopen = 1;
+  }
   if (!haveHeader) {
     res = BlockFileIO::truncateBase(size, base.get());
   } else {
     if (0 == fileIV) {
       // empty file.. create the header..
-      if (!base->isWritable()) {
+      res = initHeader();
-        // open for write..
-        int newFlags = lastFlags | O_RDWR;
-        if (base->open(newFlags) < 0) {
-          VLOG(1) << "writeHeader failed to re-open for write";
-        }
-      }
-      initHeader();
     }
-
     // can't let BlockFileIO call base->truncate(), since it would be using
     // the wrong size..
-    res = BlockFileIO::truncateBase(size, nullptr);
-
     if (res == 0) {
-      base->truncate(size + HEADER_SIZE);
+      res = BlockFileIO::truncateBase(size, nullptr);
+    }
+    if (res == 0) {
+      res = base->truncate(size + HEADER_SIZE);
+    }
+  }
+  if (reopen == 1) {
+    reopen = base->open(lastFlags);
+    if (res < 0) {
+      res = reopen;
     }
   }
   return res;
@@ -484,7 +517,10 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const {
   // generate the file IV header
   // this is needed in any case - without IV the file cannot be decoded
   unsigned char headerBuf[HEADER_SIZE];
-  const_cast<CipherFileIO *>(this)->generateReverseHeader(headerBuf);
+  int res = const_cast<CipherFileIO *>(this)->generateReverseHeader(headerBuf);
+  if (res < 0) {
+    return res;
+  }
 
   // Copy the request so we can modify it without affecting the caller
   IORequest req = origReq;
@@ -505,7 +541,8 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const {
     VLOG(1) << "Adding " << headerBytes << " header bytes";
 
     // copy the header bytes into the data
-    int headerOffset = HEADER_SIZE - headerBytes;
+    int headerOffset =
+        HEADER_SIZE - headerBytes;  // can be int as HEADER_SIZE is int
     memcpy(req.data, &headerBuf[headerOffset], headerBytes);
 
     // the read does not want data beyond the header
@@ -528,7 +565,8 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const {
   if (readBytes < 0) {
     return readBytes;  // Return error code
   }
-  ssize_t sum = headerBytes + readBytes;
+  ssize_t sum =
+      headerBytes + readBytes;  // could be size_t, but as we return ssize_t...
   VLOG(1) << "returning sum=" << sum;
   return sum;
 }

encfs/CipherFileIO.h

@@ -65,10 +65,10 @@ class CipherFileIO : public BlockFileIO {
 
  private:
   virtual ssize_t readOneBlock(const IORequest &req) const;
-  virtual bool writeOneBlock(const IORequest &req);
+  virtual ssize_t writeOneBlock(const IORequest &req);
-  virtual void generateReverseHeader(unsigned char *data);
+  virtual int generateReverseHeader(unsigned char *data);
 
-  void initHeader();
+  int initHeader();
   bool writeHeader();
   bool blockRead(unsigned char *buf, int size, uint64_t iv64) const;
   bool streamRead(unsigned char *buf, int size, uint64_t iv64) const;

encfs/Context.cpp

@@ -111,7 +111,8 @@ void EncFS_Context::renameNode(const char *from, const char *to) {
 
 // putNode stores "node" under key "path" in the "openFiles" map. It
 // increments the reference count if the key already exists.
-void EncFS_Context::putNode(const char *path, std::shared_ptr<FileNode> node) {
+void EncFS_Context::putNode(const char *path,
+                            const std::shared_ptr<FileNode> &node) {
   Lock lock(contextMutex);
   auto &list = openFiles[std::string(path)];
   // The length of "list" serves as the reference count.
@@ -122,7 +123,7 @@ void EncFS_Context::putNode(const char *path, std::shared_ptr<FileNode> node) {
 // eraseNode is called by encfs_release in response to the RELEASE
 // FUSE-command we get from the kernel.
 void EncFS_Context::eraseNode(const char *path,
-                              std::shared_ptr<FileNode> fnode) {
+                              const std::shared_ptr<FileNode> &fnode) {
   Lock lock(contextMutex);
 
   auto it = openFiles.find(std::string(path));
@@ -151,7 +152,7 @@ void EncFS_Context::eraseNode(const char *path,
 
 // nextFuseFh returns the next unused uint64 to serve as the FUSE file
 // handle for the kernel.
-uint64_t EncFS_Context::nextFuseFh(void) {
+uint64_t EncFS_Context::nextFuseFh() {
   // This is thread-safe because currentFuseFh is declared as std::atomic
   return currentFuseFh++;
 }

encfs/Context.h

@@ -48,9 +48,9 @@ class EncFS_Context {
 
   void getAndResetUsageCounter(int *usage, int *openCount);
 
-  void putNode(const char *path, std::shared_ptr<FileNode> node);
+  void putNode(const char *path, const std::shared_ptr<FileNode> &node);
 
-  void eraseNode(const char *path, std::shared_ptr<FileNode> fnode);
+  void eraseNode(const char *path, const std::shared_ptr<FileNode> &fnode);
 
   void renameNode(const char *oldName, const char *newName);
 

encfs/DirNode.cpp

@@ -149,7 +149,7 @@ class RenameOp {
 
   ~RenameOp();
 
-  operator bool() const { return renameList != nullptr; }
+  explicit operator bool() const { return renameList != nullptr; }
 
   bool apply();
   void undo();
@@ -181,8 +181,9 @@ bool RenameOp::apply() {
 
       // rename on disk..
       if (::rename(last->oldCName.c_str(), last->newCName.c_str()) == -1) {
+        int eno = errno;
         RLOG(WARNING) << "Error renaming " << last->oldCName << ": "
-                      << strerror(errno);
+                      << strerror(eno);
         dn->renameNode(last->newPName.c_str(), last->oldPName.c_str(), false);
         return false;
       }
@@ -351,7 +352,8 @@ DirTraverse DirNode::openDir(const char *plaintextPath) {
 
   DIR *dir = ::opendir(cyName.c_str());
   if (dir == nullptr) {
-    VLOG(1) << "opendir error " << strerror(errno);
+    int eno = errno;
+    VLOG(1) << "opendir error " << strerror(eno);
     return DirTraverse(shared_ptr<DIR>(), 0, std::shared_ptr<NameIO>());
   }
   std::shared_ptr<DIR> dp(dir, DirDeleter());
@@ -584,8 +586,7 @@ int DirNode::rename(const char *fromPlaintext, const char *toPlaintext) {
   }
 
   if (res != 0) {
-    VLOG(1) << "rename failed: " << strerror(errno);
+    VLOG(1) << "rename failed: " << strerror(-res);
-    res = -errno;
   }
 
   return res;
@@ -726,7 +727,7 @@ int DirNode::unlink(const char *plaintextName) {
     res = ::unlink(fullName.c_str());
     if (res == -1) {
       res = -errno;
-      VLOG(1) << "unlink error: " << strerror(errno);
+      VLOG(1) << "unlink error: " << strerror(-res);
     }
   }
 

encfs/FileIO.h

@@ -34,7 +34,7 @@ struct IORequest {
   off_t offset;
 
   // amount of bytes to read/write.
-  int dataLen;
+  size_t dataLen;
   unsigned char *data;
 
   IORequest();
@@ -68,7 +68,7 @@ class FileIO {
   virtual off_t getSize() const = 0;
 
   virtual ssize_t read(const IORequest &req) const = 0;
-  virtual bool write(const IORequest &req) = 0;
+  virtual ssize_t write(const IORequest &req) = 0;
 
   virtual int truncate(off_t size) = 0;
 

encfs/FileNode.cpp

@@ -157,14 +157,16 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) {
   if (uid != 0) {
     olduid = setfsuid(uid);
     if (olduid == -1) {
-      RLOG(DEBUG) << "setfsuid error: " << strerror(errno);
+      int eno = errno;
+      RLOG(DEBUG) << "setfsuid error: " << strerror(eno);
       return -EPERM;
     }
   }
   if (gid != 0) {
     oldgid = setfsgid(gid);
     if (oldgid == -1) {
-      RLOG(DEBUG) << "setfsgid error: " << strerror(errno);
+      int eno = errno;
+      RLOG(DEBUG) << "setfsgid error: " << strerror(eno);
       return -EPERM;
     }
   }
@@ -185,6 +187,12 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) {
     res = ::mknod(_cname.c_str(), mode, rdev);
   }
 
+  if (res == -1) {
+    int eno = errno;
+    VLOG(1) << "mknod error: " << strerror(eno);
+    res = -eno;
+  }
+
   if (olduid >= 0) {
     setfsuid(olduid);
   }
@@ -192,12 +200,6 @@ int FileNode::mknod(mode_t mode, dev_t rdev, uid_t uid, gid_t gid) {
     setfsgid(oldgid);
   }
 
-  if (res == -1) {
-    int eno = errno;
-    VLOG(1) << "mknod error: " << strerror(eno);
-    res = -eno;
-  }
-
   return res;
 }
 
@@ -222,7 +224,7 @@ off_t FileNode::getSize() const {
   return res;
 }
 
-ssize_t FileNode::read(off_t offset, unsigned char *data, ssize_t size) const {
+ssize_t FileNode::read(off_t offset, unsigned char *data, size_t size) const {
   IORequest req;
   req.offset = offset;
   req.dataLen = size;
@@ -233,7 +235,7 @@ ssize_t FileNode::read(off_t offset, unsigned char *data, ssize_t size) const {
   return io->read(req);
 }
 
-bool FileNode::write(off_t offset, unsigned char *data, ssize_t size) {
+ssize_t FileNode::write(off_t offset, unsigned char *data, size_t size) {
   VLOG(1) << "FileNode::write offset " << offset << ", data size " << size;
 
   IORequest req;
@@ -243,7 +245,12 @@ bool FileNode::write(off_t offset, unsigned char *data, ssize_t size) {
 
   Lock _lock(mutex);
 
-  return io->write(req);
+  ssize_t res = io->write(req);
+  // Of course due to encryption we genrally write more than requested
+  if (res < 0) {
+    return res;
+  }
+  return size;
 }
 
 int FileNode::truncate(off_t size) {
@@ -266,8 +273,6 @@ int FileNode::sync(bool datasync) {
     }
 #else
     (void)datasync;
-    // no fdatasync support
-    // TODO: use autoconfig to check for it..
     res = fsync(fh);
 #endif
 

encfs/FileNode.h

@@ -79,8 +79,8 @@ class FileNode {
   int getAttr(struct stat *stbuf) const;
   off_t getSize() const;
 
-  ssize_t read(off_t offset, unsigned char *data, ssize_t size) const;
+  ssize_t read(off_t offset, unsigned char *data, size_t size) const;
-  bool write(off_t offset, unsigned char *data, ssize_t size);
+  ssize_t write(off_t offset, unsigned char *data, size_t size);
 
   // truncate the file to a particular size
   int truncate(off_t size);

encfs/MACFileIO.cpp

@@ -147,7 +147,7 @@ off_t MACFileIO::getSize() const {
 ssize_t MACFileIO::readOneBlock(const IORequest &req) const {
   int headerSize = macBytes + randBytes;
 
-  int bs = blockSize() + headerSize;
+  int bs = blockSize() + headerSize;  // ok, should clearly fit into an int
 
   MemBlock mb = MemoryPool::allocate(bs);
 
@@ -194,7 +194,7 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const {
         RLOG(WARNING) << "MAC comparison failure in block " << blockNum;
         if (!warnOnly) {
           MemoryPool::release(mb);
-          throw Error(_("MAC comparison failure, refusing to read"));
+          return -EBADMSG;
         }
       }
     }
@@ -214,10 +214,10 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const {
   return readSize;
 }
 
-bool MACFileIO::writeOneBlock(const IORequest &req) {
+ssize_t MACFileIO::writeOneBlock(const IORequest &req) {
   int headerSize = macBytes + randBytes;
 
-  int bs = blockSize() + headerSize;
+  int bs = blockSize() + headerSize;  // ok, should clearly fit into an int
 
   // we have the unencrypted data, so we need to attach a header to it.
   MemBlock mb = MemoryPool::allocate(bs);
@@ -231,7 +231,7 @@ bool MACFileIO::writeOneBlock(const IORequest &req) {
   memcpy(newReq.data + headerSize, req.data, req.dataLen);
   if (randBytes > 0) {
     if (!cipher->randomize(newReq.data + macBytes, randBytes, false)) {
-      return false;
+      return -EBADMSG;
     }
   }
 
@@ -247,21 +247,21 @@ bool MACFileIO::writeOneBlock(const IORequest &req) {
   }
 
   // now, we can let the next level have it..
-  bool ok = base->write(newReq);
+  ssize_t writeSize = base->write(newReq);
 
   MemoryPool::release(mb);
 
-  return ok;
+  return writeSize;
 }
 
 int MACFileIO::truncate(off_t size) {
   int headerSize = macBytes + randBytes;
-  int bs = blockSize() + headerSize;
+  int bs = blockSize() + headerSize;  // ok, should clearly fit into an int
 
   int res = BlockFileIO::truncateBase(size, nullptr);
 
   if (res == 0) {
-    base->truncate(locWithHeader(size, bs, headerSize));
+    res = base->truncate(locWithHeader(size, bs, headerSize));
   }
 
   return res;

encfs/MACFileIO.h

@@ -64,7 +64,7 @@ class MACFileIO : public BlockFileIO {
 
  private:
   virtual ssize_t readOneBlock(const IORequest &req) const;
-  virtual bool writeOneBlock(const IORequest &req);
+  virtual ssize_t writeOneBlock(const IORequest &req);
 
   std::shared_ptr<FileIO> base;
   std::shared_ptr<Cipher> cipher;

encfs/RawFileIO.cpp

@@ -90,6 +90,8 @@ Interface RawFileIO::interface() const { return RawFileIO_iface; }
     This works around the problem described in
    https://github.com/vgough/encfs/issues/181
     Without this, "umask 0777 ; echo foo > bar" fails.
+
+    Sets errno when -1 is returned.
 */
 static int open_readonly_workaround(const char *path, int flags) {
   int fd = -1;
@@ -97,13 +99,11 @@ static int open_readonly_workaround(const char *path, int flags) {
   memset(&stbuf, 0, sizeof(struct stat));
   if (lstat(path, &stbuf) != -1) {
     // make sure user has read/write permission..
-    chmod(path, stbuf.st_mode | 0600);
+    if (chmod(path, stbuf.st_mode | 0600) != -1) {
-    fd = ::open(path, flags);
+      fd = ::open(path, flags);
-    chmod(path, stbuf.st_mode);
+      chmod(path, stbuf.st_mode);
-  } else {
+    }
-    RLOG(INFO) << "can't stat file " << path;
   }
-
   return fd;
 }
 
@@ -120,50 +120,53 @@ int RawFileIO::open(int flags) {
   bool requestWrite = (((flags & O_RDWR) != 0) || ((flags & O_WRONLY) != 0));
   VLOG(1) << "open call, requestWrite = " << requestWrite;
 
-  int result = 0;
-
   // if we have a descriptor and it is writable, or we don't need writable..
   if ((fd >= 0) && (canWrite || !requestWrite)) {
     VLOG(1) << "using existing file descriptor";
-    result = fd;  // success
+    return fd;  // success
-  } else {
+  }
-    int finalFlags = requestWrite ? O_RDWR : O_RDONLY;
+
+  int finalFlags = requestWrite ? O_RDWR : O_RDONLY;
 
 #if defined(O_LARGEFILE)
-    if ((flags & O_LARGEFILE) != 0) {
+  if ((flags & O_LARGEFILE) != 0) {
-      finalFlags |= O_LARGEFILE;
+    finalFlags |= O_LARGEFILE;
-    }
+  }
 #else
 #warning O_LARGEFILE not supported
 #endif
 
-    int newFd = ::open(name.c_str(), finalFlags);
+  int eno;
-
+  int newFd = ::open(name.c_str(), finalFlags);
-    VLOG(1) << "open file with flags " << finalFlags << ", result = " << newFd;
+  if (newFd < 0) {
-
+    eno = errno;
-    if ((newFd == -1) && (errno == EACCES)) {
-      VLOG(1) << "using readonly workaround for open";
-      newFd = open_readonly_workaround(name.c_str(), finalFlags);
-    }
-
-    if (newFd >= 0) {
-      if (oldfd >= 0) {
-        RLOG(ERROR) << "leaking FD?: oldfd = " << oldfd << ", fd = " << fd
-                    << ", newfd = " << newFd;
-      }
-
-      // the old fd might still be in use, so just keep it around for
-      // now.
-      canWrite = requestWrite;
-      oldfd = fd;
-      result = fd = newFd;
-    } else {
-      result = -errno;
-      RLOG(DEBUG) << "::open error: " << strerror(errno);
-    }
   }
 
-  return result;
+  VLOG(1) << "open file with flags " << finalFlags << ", result = " << newFd;
+
+  if ((newFd == -1) && (eno == EACCES)) {
+    VLOG(1) << "using readonly workaround for open";
+    newFd = open_readonly_workaround(name.c_str(), finalFlags);
+    eno = errno;
+  }
+
+  if (newFd < 0) {
+    RLOG(DEBUG) << "::open error: " << strerror(eno);
+    return -eno;
+  }
+
+  if (oldfd >= 0) {
+    RLOG(ERROR) << "leaking FD?: oldfd = " << oldfd << ", fd = " << fd
+                << ", newfd = " << newFd;
+  }
+
+  // the old fd might still be in use, so just keep it around for
+  // now.
+  canWrite = requestWrite;
+  oldfd = fd;
+  fd = newFd;
+
+  return fd;
 }
 
 int RawFileIO::getAttr(struct stat *stbuf) const {
@@ -192,8 +195,9 @@ off_t RawFileIO::getSize() const {
       const_cast<RawFileIO *>(this)->knownSize = true;
       return fileSize;
     }
-    RLOG(ERROR) << "getSize on " << name << " failed: " << strerror(errno);
+    int eno = errno;
-    return -1;
+    RLOG(ERROR) << "getSize on " << name << " failed: " << strerror(eno);
+    return -eno;
   }
   return fileSize;
 }
@@ -204,44 +208,59 @@ ssize_t RawFileIO::read(const IORequest &req) const {
   ssize_t readSize = pread(fd, req.data, req.dataLen, req.offset);
 
   if (readSize < 0) {
+    int eno = errno;
     RLOG(WARNING) << "read failed at offset " << req.offset << " for "
-                  << req.dataLen << " bytes: " << strerror(errno);
+                  << req.dataLen << " bytes: " << strerror(eno);
+    return -eno;
   }
 
   return readSize;
 }
 
-bool RawFileIO::write(const IORequest &req) {
+ssize_t RawFileIO::write(const IORequest &req) {
   rAssert(fd >= 0);
   rAssert(canWrite);
 
-  int retrys = 10;
+  // int retrys = 10;
   void *buf = req.data;
   ssize_t bytes = req.dataLen;
   off_t offset = req.offset;
 
-  while ((bytes != 0) && retrys > 0) {
+  /*
+   * Let's write while pwrite() writes, to avoid writing only a part of the
+   * request,
+   * whereas it could have been fully written. This to avoid inconsistencies /
+   * corruption.
+   */
+  // while ((bytes != 0) && retrys > 0) {
+  while (bytes != 0) {
     ssize_t writeSize = ::pwrite(fd, buf, bytes, offset);
 
     if (writeSize < 0) {
+      int eno = errno;
       knownSize = false;
       RLOG(WARNING) << "write failed at offset " << offset << " for " << bytes
-                    << " bytes: " << strerror(errno);
+                    << " bytes: " << strerror(eno);
-      return false;
+      // pwrite is not expected to return 0, so eno should always be set, but we
+      // never know...
+      return -eno;
+    }
+    if (writeSize == 0) {
+      return -EIO;
     }
 
     bytes -= writeSize;
     offset += writeSize;
     buf = (void *)((char *)buf + writeSize);
-    --retrys;
   }
 
-  if (bytes != 0) {
+  // if (bytes != 0) {
-    RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of "
+  //   RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of
-                << req.dataLen << ", max retries reached";
+  //   "
-    knownSize = false;
+  //               << req.dataLen << ", max retries reached";
-    return false;
+  //   knownSize = false;
-  }
+  //   return (eno) ? -eno : -EIO;
+  // }
   if (knownSize) {
     off_t last = req.offset + req.dataLen;
     if (last > fileSize) {
@@ -249,7 +268,7 @@ bool RawFileIO::write(const IORequest &req) {
     }
   }
 
-  return true;
+  return req.dataLen;
 }
 
 int RawFileIO::truncate(off_t size) {

encfs/RawFileIO.h

@@ -46,7 +46,7 @@ class RawFileIO : public FileIO {
   virtual off_t getSize() const;
 
   virtual ssize_t read(const IORequest &req) const;
-  virtual bool write(const IORequest &req);
+  virtual ssize_t write(const IORequest &req);
 
   virtual int truncate(off_t size);
 

encfs/SSL_Cipher.cpp

@@ -106,7 +106,7 @@ int BytesToKey(int keyLen, int ivLen, const EVP_MD *md,
       memcpy(iv, mdBuf + offset, toCopy);
       iv += toCopy;
       niv -= toCopy;
-      offset += toCopy;
+      // offset += toCopy;
     }
     if ((nkey == 0) && (niv == 0)) {
       break;
@@ -170,12 +170,14 @@ static Range CAMELLIABlockRange(64, 4096, 16);
 
 static std::shared_ptr<Cipher> NewCAMELLIACipher(const Interface &iface,
                                                  int keyLen) {
-  if (keyLen <= 0) keyLen = 192;
+  if (keyLen <= 0) {
+    keyLen = 192;
+  }
 
   keyLen = CAMELLIAKeyRange.closest(keyLen);
 
-  const EVP_CIPHER *blockCipher = 0;
+  const EVP_CIPHER *blockCipher = nullptr;
-  const EVP_CIPHER *streamCipher = 0;
+  const EVP_CIPHER *streamCipher = nullptr;
 
   switch (keyLen) {
     case 128:
@@ -503,7 +505,7 @@ CipherKey SSL_Cipher::newRandomKey() {
     compute a 64-bit check value for the data using HMAC.
 */
 static uint64_t _checksum_64(SSLKey *key, const unsigned char *data,
-                             int dataLen, uint64_t *chainedIV) {
+                             int dataLen, uint64_t *const chainedIV) {
   rAssert(dataLen > 0);
   Lock lock(key->mutex);
 
@@ -810,6 +812,7 @@ bool SSL_Cipher::streamEncode(unsigned char *buf, int size, uint64_t iv64,
   if (dstLen != size) {
     RLOG(ERROR) << "encoding " << size << " bytes, got back " << dstLen << " ("
                 << tmpLen << " in final_ex)";
+    return false;
   }
 
   return true;
@@ -846,6 +849,7 @@ bool SSL_Cipher::streamDecode(unsigned char *buf, int size, uint64_t iv64,
   if (dstLen != size) {
     RLOG(ERROR) << "decoding " << size << " bytes, got back " << dstLen << " ("
                 << tmpLen << " in final_ex)";
+    return false;
   }
 
   return true;
@@ -861,7 +865,8 @@ bool SSL_Cipher::blockEncode(unsigned char *buf, int size, uint64_t iv64,
   // data must be integer number of blocks
   const int blockMod = size % EVP_CIPHER_CTX_block_size(key->block_enc);
   if (blockMod != 0) {
-    throw Error("Invalid data size, not multiple of block size");
+    RLOG(ERROR) << "Invalid data size, not multiple of block size";
+    return false;
   }
 
   Lock lock(key->mutex);
@@ -879,6 +884,7 @@ bool SSL_Cipher::blockEncode(unsigned char *buf, int size, uint64_t iv64,
   if (dstLen != size) {
     RLOG(ERROR) << "encoding " << size << " bytes, got back " << dstLen << " ("
                 << tmpLen << " in final_ex)";
+    return false;
   }
 
   return true;
@@ -894,7 +900,8 @@ bool SSL_Cipher::blockDecode(unsigned char *buf, int size, uint64_t iv64,
   // data must be integer number of blocks
   const int blockMod = size % EVP_CIPHER_CTX_block_size(key->block_dec);
   if (blockMod != 0) {
-    throw Error("Invalid data size, not multiple of block size");
+    RLOG(ERROR) << "Invalid data size, not multiple of block size";
+    return false;
   }
 
   Lock lock(key->mutex);
@@ -912,6 +919,7 @@ bool SSL_Cipher::blockDecode(unsigned char *buf, int size, uint64_t iv64,
   if (dstLen != size) {
     RLOG(ERROR) << "decoding " << size << " bytes, got back " << dstLen << " ("
                 << tmpLen << " in final_ex)";
+    return false;
   }
 
   return true;

encfs/XmlReader.cpp

@@ -103,7 +103,7 @@ bool XmlValue::readB64(const char *path, unsigned char *data,
 
   std::string s = value->text();
   s.erase(std::remove_if(s.begin(), s.end(), ::isspace), s.end());
-  s.erase(s.find_last_not_of("=") + 1);
+  s.erase(s.find_last_not_of('=') + 1);
 
   int decodedSize = B64ToB256Bytes(s.size());
   if (decodedSize != length) {

encfs/encfs.cpp

@@ -25,6 +25,7 @@
 #include <cstring>
 #include <ctime>
 #include <fcntl.h>
+#include <limits>
 #include <memory>
 #include <sys/stat.h>
 #include <sys/statvfs.h>
@@ -64,7 +65,7 @@ using namespace std::placeholders;
 
 namespace encfs {
 
-#define GET_FN(ctx, finfo) ctx->getNode((void *)(uintptr_t)finfo->fh)
+#define GET_FN(ctx, finfo) (ctx)->getNode((void *)(uintptr_t)(finfo)->fh)
 
 static EncFS_Context *context() {
   return (EncFS_Context *)fuse_get_context()->private_data;
@@ -78,9 +79,10 @@ static EncFS_Context *context() {
 static bool isReadOnly(EncFS_Context *ctx) { return ctx->opts->readOnly; }
 
 // helper function -- apply a functor to a cipher path, given the plain path
-static int withCipherPath(const char *opName, const char *path,
+static int withCipherPath(
-                          function<int(EncFS_Context *, const string &)> op,
+    const char *opName, const char *path,
-                          bool passReturnCode = false) {
+    const function<int(EncFS_Context *, const string &)> &op,
+    bool passReturnCode = false) {
   EncFS_Context *ctx = context();
 
   int res = -EIO;
@@ -109,7 +111,7 @@ static int withCipherPath(const char *opName, const char *path,
   return res;
 }
 
-static void checkCanary(std::shared_ptr<FileNode> fnode) {
+static void checkCanary(const std::shared_ptr<FileNode> &fnode) {
   if (fnode->canary == CANARY_OK) {
     return;
   }
@@ -408,11 +410,7 @@ int _do_readlink(EncFS_Context *ctx, const string &cyName, char *buf,
 
   buf[res] = '\0';  // ensure null termination
   string decodedName;
-  try {
+  decodedName = FSRoot->plainPath(buf);
-    decodedName = FSRoot->plainPath(buf);
-  } catch (...) {
-    VLOG(1) << "caught error decoding path";
-  }
 
   if (!decodedName.empty()) {
     strncpy(buf, decodedName.c_str(), size - 1);
@@ -690,12 +688,18 @@ int encfs_release(const char *path, struct fuse_file_info *finfo) {
   }
 }
 
-int _do_read(FileNode *fnode, unsigned char *ptr, size_t size, off_t off) {
+ssize_t _do_read(FileNode *fnode, unsigned char *ptr, size_t size, off_t off) {
   return fnode->read(off, ptr, size);
 }
 
 int encfs_read(const char *path, char *buf, size_t size, off_t offset,
                struct fuse_file_info *file) {
+  // Unfortunately we have to convert from ssize_t (pread) to int (fuse), so
+  // let's check this will be OK
+  if (size > std::numeric_limits<int>::max()) {
+    RLOG(ERROR) << "tried to read too much data: " << size;
+    return -EIO;
+  }
   return withFileNode("read", path, file,
                       bind(_do_read, _1, (unsigned char *)buf, size, offset));
 }
@@ -712,15 +716,19 @@ int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) {
   return withFileNode("fsync", path, file, bind(_do_fsync, _1, dataSync));
 }
 
-int _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) {
+ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size,
-  if (fnode->write(offset, ptr, size)) {
+                  off_t offset) {
-    return size;
+  return fnode->write(offset, ptr, size);
-  }
-  return -EIO;
 }
 
 int encfs_write(const char *path, const char *buf, size_t size, off_t offset,
                 struct fuse_file_info *file) {
+  // Unfortunately we have to convert from ssize_t (pwrite) to int (fuse), so
+  // let's check this will be OK
+  if (size > std::numeric_limits<int>::max()) {
+    RLOG(ERROR) << "tried to write too much data: " << size;
+    return -EIO;
+  }
   EncFS_Context *ctx = context();
   if (isReadOnly(ctx)) {
     return -EROFS;

encfs/encfs.h

@@ -41,7 +41,8 @@ static __inline int setfsuid(uid_t uid) {
   uid_t olduid = geteuid();
 
   if (seteuid(uid) != 0) {
-    VLOG(1) << "seteuid error: " << errno;
+    int eno = errno;
+    VLOG(1) << "seteuid error: " << strerror(eno);
   }
 
   return olduid;
@@ -51,7 +52,8 @@ static __inline int setfsgid(gid_t gid) {
   gid_t oldgid = getegid();
 
   if (setegid(gid) != 0) {
-    VLOG(1) << "setfsgid error: " << errno;
+    int eno = errno;
+    VLOG(1) << "setfsgid error: " << strerror(eno);
   }
 
   return oldgid;

encfs/main.cpp

@@ -534,7 +534,7 @@ void *encfs_init(fuse_conn_info *conn) {
     if (res != 0) {
       RLOG(ERROR) << "error starting idle monitor thread, "
                      "res = "
-                  << res << ", errno = " << errno;
+                  << res << ", " << strerror(res);
     }
   }
 

integration/normal.t.pl

@@ -2,7 +2,7 @@
 
 # Test EncFS normal and paranoid mode
 
-use Test::More tests => 122;
+use Test::More tests => 132;
 use File::Path;
 use File::Copy;
 use File::Temp;
@@ -78,6 +78,8 @@ sub runTests
     &grow;
     &umask0777;
     &create_unmount_remount;
+    &checkReadError;
+    &checkWriteError;
 
     &configFromPipe;
     &cleanup;
@@ -109,7 +111,7 @@ sub corruption
     ok( open(IN, "< $crypt/corrupt"), "open corrupted file");
     my $content;
     $result = read(IN, $content, 20);
-    ok(! defined $result, "corrupted file with MAC returns read error: $!");
+    ok($!{EBADMSG} && (! defined $result), "corrupted file with MAC returns read error: $!");
 }
 
 # Test internal modification
@@ -430,3 +432,38 @@ sub create_unmount_remount
 
     portable_unmount($mnt);
 }
+
+# Test that read errors are correctly thrown up to us
+sub checkReadError
+{
+    # Not sure how to implement this, so feel free !
+    ok(1, "read error");
+}
+
+# Test that write errors are correctly thrown up to us
+sub checkWriteError
+{
+    # No OSX impl, and requires sudo which is inconvenient outside of CI.
+    if($^O eq "darwin" || !defined($ENV{'SUDO_TESTS'})) {
+        ok(1, "write error");
+        ok(1, "write error");
+        ok(1, "write error");
+        ok(1, "write error");
+    }
+    else {
+            my $crypt = "$workingDir/checkWriteError.crypt";
+            my $mnt = "$workingDir/checkWriteError.mnt";
+            mkdir($crypt) || BAIL_OUT($!);
+            mkdir($mnt)  || BAIL_OUT($!);
+            system("sudo mount -t tmpfs -o size=1m tmpfs $crypt");
+            ok( $? == 0, "mount command returns 0") || return;
+            system("./build/encfs --standard --extpass=\"echo test\" $crypt $mnt 2>&1");
+            ok( $? == 0, "encfs command returns 0") || return;
+            ok(open(OUT , "> $mnt/file"), "write content");
+            while(print OUT "0123456789") {}
+            ok ($!{ENOSPC}, "write returned $! instead of ENOSPC");
+            close OUT;
+            portable_unmount($mnt);
+            system("sudo umount $crypt");
+    }
+}