From 5c90ad451e16e69f3a73ff1541788aeec7a7aa1e Mon Sep 17 00:00:00 2001 From: benrubson Date: Wed, 9 Aug 2017 17:36:20 +0200 Subject: [PATCH] Make write return the number of bytes written --- encfs/BlockFileIO.cpp | 70 +++++++++++++++++++++++++----------------- encfs/BlockFileIO.h | 6 ++-- encfs/CipherFileIO.cpp | 27 ++++++++-------- encfs/CipherFileIO.h | 2 +- encfs/FileIO.h | 4 +-- encfs/FileNode.cpp | 11 +++++-- encfs/FileNode.h | 4 +-- encfs/MACFileIO.cpp | 14 ++++----- encfs/MACFileIO.h | 2 +- encfs/RawFileIO.cpp | 4 +-- encfs/RawFileIO.h | 2 +- encfs/encfs.cpp | 21 ++++++++----- 12 files changed, 95 insertions(+), 72 deletions(-) diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index 399d35e..5cf323c 100644 --- a/encfs/BlockFileIO.cpp +++ b/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,13 +97,13 @@ ssize_t BlockFileIO::cacheReadOneBlock(const IORequest &req) const { return result; } -int 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; - int res = writeOneBlock(req); + ssize_t res = writeOneBlock(req); if (res < 0) { clearCache(_cache, _blockSize); } @@ -121,7 +121,7 @@ int BlockFileIO::cacheWriteOneBlock(const IORequest &req) { 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; @@ -161,7 +161,7 @@ ssize_t BlockFileIO::read(const IORequest &req) const { } 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 @@ -188,10 +188,9 @@ ssize_t BlockFileIO::read(const IORequest &req) const { } /** - * Returns the very first result returned by RawFileIO::write so : - * 0 in case of success, or -errno in case of failure. + * Returns the number of bytes written, or -errno in case of failure. */ -int BlockFileIO::write(const IORequest &req) { +ssize_t BlockFileIO::write(const IORequest &req) { CHECK(_blockSize != 0); off_t fileSize = getSize(); @@ -201,11 +200,11 @@ int BlockFileIO::write(const IORequest &req) { // 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) { @@ -243,7 +242,7 @@ int BlockFileIO::write(const IORequest &req) { blockReq.data = nullptr; blockReq.dataLen = _blockSize; - int res = 0; + ssize_t res = 0; size_t size = req.dataLen; unsigned char *inPtr = req.data; while (size != 0u) { @@ -272,10 +271,12 @@ int BlockFileIO::write(const IORequest &req) { } else { // have to merge with existing block data.. blockReq.dataLen = _blockSize; - if ((blockReq.dataLen = cacheReadOneBlock(blockReq)) < 0) { - res = blockReq.dataLen; + ssize_t readSize = cacheReadOneBlock(blockReq); + if (readSize < 0) { + res = readSize; break; } + blockReq.dataLen = readSize; // extend data if necessary.. if (partialOffset + toCopy > blockReq.dataLen) { @@ -303,20 +304,22 @@ int BlockFileIO::write(const IORequest &req) { MemoryPool::release(mb); } - return res; + if (res < 0) { + return res; + } + return req.dataLen; } int BlockFileIO::blockSize() const { return _blockSize; } /** - * Returns the very first result returned by RawFileIO::write so : - * 0 in case of success, or -errno in case of failure. + * 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 res = 0; + int newBlockSize = newSize % _blockSize; //can be int as _blockSize is int + ssize_t res = 0; IORequest req; MemBlock mb; @@ -366,7 +369,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { // 2, pad zero blocks unless holes are allowed if (!_allowHoles) { - for (; !(res < 0) && (oldLastBlock != newLastBlock); ++oldLastBlock) { + for (; (res >= 0) && (oldLastBlock != newLastBlock); ++oldLastBlock) { VLOG(1) << "padding block " << oldLastBlock; req.offset = oldLastBlock * _blockSize; req.dataLen = _blockSize; @@ -376,7 +379,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { } // 3. only necessary if write is forced and block is non 0 length - if (!(res < 0) && forceWrite && newBlockSize) { + if ((res >= 0) && forceWrite && newBlockSize) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); @@ -388,11 +391,17 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { MemoryPool::release(mb); } - return res; + 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(); @@ -407,7 +416,7 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { } const bool forceWrite = true; - if (!(res < 0)) { + if (res == 0) { res = padFile(oldSize, size, forceWrite); } } else if (size == oldSize) { @@ -424,20 +433,23 @@ int BlockFileIO::truncateBase(off_t size, FileIO *base) { req.dataLen = _blockSize; req.data = mb.data; - ssize_t rdSz = cacheReadOneBlock(req); - if (rdSz < 0) { - res = rdSz; + ssize_t readSize = cacheReadOneBlock(req); + if (readSize < 0) { + res = readSize; } - // do the truncate else if (base != nullptr) { + // do the truncate res = base->truncate(size); } // write back out partial block req.dataLen = partialBlock; - if (!(res < 0)) { - res = cacheWriteOneBlock(req); + if (res == 0) { + ssize_t writeSize = cacheWriteOneBlock(req); + if (writeSize < 0) { + res = writeSize; + } } MemoryPool::release(mb); diff --git a/encfs/BlockFileIO.h b/encfs/BlockFileIO.h index ca02883..94dd71e 100644 --- a/encfs/BlockFileIO.h +++ b/encfs/BlockFileIO.h @@ -43,7 +43,7 @@ class BlockFileIO : public FileIO { // implemented in terms of blocks. virtual ssize_t read(const IORequest &req) const; - virtual int write(const IORequest &req); + virtual ssize_t write(const IORequest &req); virtual int blockSize() const; @@ -54,10 +54,10 @@ class BlockFileIO : public FileIO { // 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 int writeOneBlock(const IORequest &req) = 0; + virtual ssize_t writeOneBlock(const IORequest &req) = 0; ssize_t cacheReadOneBlock(const IORequest &req) const; - int cacheWriteOneBlock(const IORequest &req); + ssize_t cacheWriteOneBlock(const IORequest &req); int _blockSize; bool _allowHoles; diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index a83d821..44b41aa 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -233,9 +233,9 @@ int CipherFileIO::initHeader() { req.data = buf; req.dataLen = 8; - int res = base->write(req); - if (res < 0) { - return res; + ssize_t writeSize = base->write(req); + if (writeSize < 0) { + return writeSize; } } else { VLOG(1) << "base not writable, IV not written.."; @@ -333,14 +333,13 @@ 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) { @@ -353,9 +352,9 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { 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) { @@ -370,7 +369,7 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { return readSize; } -int CipherFileIO::writeOneBlock(const IORequest &req) { +ssize_t CipherFileIO::writeOneBlock(const IORequest &req) { if (haveHeader && fsConfig->reverseEncryption) { VLOG(1) @@ -390,12 +389,12 @@ int CipherFileIO::writeOneBlock(const IORequest &req) { 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 } - int res = 0; + ssize_t res = 0; if (ok) { if (haveHeader) { IORequest tmpReq = req; @@ -481,7 +480,7 @@ int CipherFileIO::truncate(off_t size) { // the wrong size.. res = BlockFileIO::truncateBase(size, nullptr); - if (!(res < 0)) { + if (res == 0) { res = base->truncate(size + HEADER_SIZE); } } @@ -531,7 +530,7 @@ 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 @@ -554,7 +553,7 @@ 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; } diff --git a/encfs/CipherFileIO.h b/encfs/CipherFileIO.h index 9378e88..b0ddb19 100644 --- a/encfs/CipherFileIO.h +++ b/encfs/CipherFileIO.h @@ -65,7 +65,7 @@ class CipherFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual int writeOneBlock(const IORequest &req); + virtual ssize_t writeOneBlock(const IORequest &req); virtual int generateReverseHeader(unsigned char *data); int initHeader(); diff --git a/encfs/FileIO.h b/encfs/FileIO.h index 73de8b7..c1060db 100644 --- a/encfs/FileIO.h +++ b/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 int write(const IORequest &req) = 0; + virtual ssize_t write(const IORequest &req) = 0; virtual int truncate(off_t size) = 0; diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index f23b3ae..de65d89 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -224,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; @@ -235,7 +235,7 @@ ssize_t FileNode::read(off_t offset, unsigned char *data, ssize_t size) const { return io->read(req); } -int 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; @@ -245,7 +245,12 @@ int 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) { diff --git a/encfs/FileNode.h b/encfs/FileNode.h index 6279c33..0eff109 100644 --- a/encfs/FileNode.h +++ b/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; - int write(off_t offset, unsigned char *data, ssize_t size); + ssize_t read(off_t offset, unsigned char *data, size_t size) const; + ssize_t write(off_t offset, unsigned char *data, size_t size); // truncate the file to a particular size int truncate(off_t size); diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index df7a753..cf12560 100644 --- a/encfs/MACFileIO.cpp +++ b/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); @@ -214,10 +214,10 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { return readSize; } -int 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); @@ -247,20 +247,20 @@ int MACFileIO::writeOneBlock(const IORequest &req) { } // now, we can let the next level have it.. - int res = base->write(newReq); + ssize_t writeSize = base->write(newReq); MemoryPool::release(mb); - return res; + 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)) { + if (res == 0) { res = base->truncate(locWithHeader(size, bs, headerSize)); } diff --git a/encfs/MACFileIO.h b/encfs/MACFileIO.h index eb80013..026a4c8 100644 --- a/encfs/MACFileIO.h +++ b/encfs/MACFileIO.h @@ -64,7 +64,7 @@ class MACFileIO : public BlockFileIO { private: virtual ssize_t readOneBlock(const IORequest &req) const; - virtual int writeOneBlock(const IORequest &req); + virtual ssize_t writeOneBlock(const IORequest &req); std::shared_ptr base; std::shared_ptr cipher; diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index d7dbc03..27aa021 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -212,7 +212,7 @@ ssize_t RawFileIO::read(const IORequest &req) const { return readSize; } -int RawFileIO::write(const IORequest &req) { +ssize_t RawFileIO::write(const IORequest &req) { rAssert(fd >= 0); rAssert(canWrite); @@ -253,7 +253,7 @@ int RawFileIO::write(const IORequest &req) { } } - return 0; //No matter how many bytes we wrote, we of course already know this. + return req.dataLen; } return true; diff --git a/encfs/RawFileIO.h b/encfs/RawFileIO.h index be69313..370edd5 100644 --- a/encfs/RawFileIO.h +++ b/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 int write(const IORequest &req); + virtual ssize_t write(const IORequest &req); virtual int truncate(off_t size); diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 0afc117..1ddbf46 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #ifdef linux #include #endif @@ -686,12 +687,17 @@ 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::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)); } @@ -708,16 +714,17 @@ 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) { - int res = fnode->write(offset, ptr, size); - if (res < 0) { - return res; - } - return size; +ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { + return fnode->write(offset, ptr, size); } 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::max()) { + RLOG(ERROR) << "tried to write too much data: " << size; + return -EIO; + } EncFS_Context *ctx = context(); if (isReadOnly(ctx)) { return -EROFS;