From bdb0414add26b3b792d9c1d16c36a933069acabb Mon Sep 17 00:00:00 2001 From: samcake Date: Fri, 12 May 2017 18:04:22 -0700 Subject: [PATCH] Adding a validation step at runtime for the cached KTX file in order to regenerate them if anything seems wrong --- libraries/gpu/src/gpu/Texture.cpp | 7 ++++++- libraries/gpu/src/gpu/Texture_ktx.cpp | 7 +++++++ libraries/ktx/src/ktx/KTX.cpp | 6 ++++++ libraries/ktx/src/ktx/Reader.cpp | 19 ++++++++++++++++++- libraries/ktx/src/ktx/Writer.cpp | 3 ++- .../src/model-networking/KTXCache.cpp | 2 +- .../src/model-networking/TextureCache.cpp | 4 +++- libraries/networking/src/FileCache.cpp | 11 ++++++++--- libraries/networking/src/FileCache.h | 2 +- libraries/shared/src/shared/Storage.cpp | 4 ++++ 10 files changed, 56 insertions(+), 9 deletions(-) diff --git a/libraries/gpu/src/gpu/Texture.cpp b/libraries/gpu/src/gpu/Texture.cpp index 0f84d2a3c9..a94a0e1621 100755 --- a/libraries/gpu/src/gpu/Texture.cpp +++ b/libraries/gpu/src/gpu/Texture.cpp @@ -441,7 +441,10 @@ void Texture::assignStoredMip(uint16 level, storage::StoragePointer& storage) { // THen check that the mem texture passed make sense with its format Size expectedSize = evalStoredMipSize(level, getStoredMipFormat()); auto size = storage->size(); - if (storage->size() <= expectedSize) { + if (storage->size() < expectedSize) { + _storage->assignMipData(level, storage); + _stamp++; + } else if (size == expectedSize) { _storage->assignMipData(level, storage); _stamp++; } else if (size > expectedSize) { @@ -469,6 +472,8 @@ void Texture::assignStoredMipFace(uint16 level, uint8 face, storage::StoragePoin Size expectedSize = evalStoredMipFaceSize(level, getStoredMipFormat()); auto size = storage->size(); if (size <= expectedSize) { + _stamp++; + } else if (size == expectedSize) { _storage->assignMipFaceData(level, face, storage); _stamp++; } else if (size > expectedSize) { diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 3fc4e0d432..524fd0a88c 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -542,6 +542,13 @@ bool Texture::evalTextureFormat(const ktx::Header& header, Element& mipFormat, E } else { return false; } + } else if (header.getGLFormat() == ktx::GLFormat::RG && header.getGLType() == ktx::GLType::UNSIGNED_BYTE && header.getTypeSize() == 1) { + mipFormat = Format::VEC2NU8_XY; + if (header.getGLInternaFormat_Uncompressed() == ktx::GLInternalFormat_Uncompressed::RG8) { + texelFormat = Format::VEC2NU8_XY; + } else { + return false; + } } else if (header.getGLFormat() == ktx::GLFormat::COMPRESSED_FORMAT && header.getGLType() == ktx::GLType::COMPRESSED_TYPE) { if (header.getGLInternaFormat_Compressed() == ktx::GLInternalFormat_Compressed::COMPRESSED_SRGB_S3TC_DXT1_EXT) { mipFormat = Format::COLOR_COMPRESSED_SRGB; diff --git a/libraries/ktx/src/ktx/KTX.cpp b/libraries/ktx/src/ktx/KTX.cpp index 38bb91e5c2..ff80695280 100644 --- a/libraries/ktx/src/ktx/KTX.cpp +++ b/libraries/ktx/src/ktx/KTX.cpp @@ -114,6 +114,9 @@ size_t Header::evalFaceSize(uint32_t level) const { } size_t Header::evalImageSize(uint32_t level) const { auto faceSize = evalFaceSize(level); + if ((faceSize < 4) || ((faceSize & 0x3) != 0)) { + return 0; + } if (numberOfFaces == NUM_CUBEMAPFACES && numberOfArrayElements == 0) { return faceSize; } else { @@ -139,6 +142,9 @@ ImageDescriptors Header::generateImageDescriptors() const { size_t imageOffset = 0; for (uint32_t level = 0; level < numberOfMipmapLevels; ++level) { auto imageSize = static_cast(evalImageSize(level)); + if ((imageSize < 4) || ((imageSize & 0x3) != 0)) { + return ImageDescriptors(); + } if (imageSize == 0) { return ImageDescriptors(); } diff --git a/libraries/ktx/src/ktx/Reader.cpp b/libraries/ktx/src/ktx/Reader.cpp index 440e2f048c..49fc8bac70 100644 --- a/libraries/ktx/src/ktx/Reader.cpp +++ b/libraries/ktx/src/ktx/Reader.cpp @@ -148,12 +148,24 @@ namespace ktx { size_t imageSize = *reinterpret_cast(currentPtr); currentPtr += sizeof(uint32_t); + auto expectedImageSize = header.evalImageSize(images.size()); + if (imageSize != expectedImageSize) { + break; + } else if ((imageSize < 4) || (imageSize & 0x3)) { + break; + } + + // The image size is the face size, beware! + size_t faceSize = imageSize; + if (numFaces == NUM_CUBEMAPFACES) { + imageSize = NUM_CUBEMAPFACES * faceSize; + } + // If enough data ahead then capture the pointer if ((currentPtr - srcBytes) + imageSize <= (srcSize)) { auto padding = Header::evalPadding(imageSize); if (numFaces == NUM_CUBEMAPFACES) { - size_t faceSize = imageSize / NUM_CUBEMAPFACES; Image::FaceBytes faces(NUM_CUBEMAPFACES); for (uint32_t face = 0; face < NUM_CUBEMAPFACES; face++) { faces[face] = currentPtr; @@ -166,6 +178,7 @@ namespace ktx { currentPtr += imageSize + padding; } } else { + // Stop here break; } } @@ -190,6 +203,10 @@ namespace ktx { // populate image table result->_images = parseImages(result->getHeader(), result->getTexelsDataSize(), result->getTexelsData()); + if (result->_images.size() != result->getHeader().getNumberOfLevels()) { + // Fail if the number of images produced doesn't match the header number of levels + return nullptr; + } return result; } diff --git a/libraries/ktx/src/ktx/Writer.cpp b/libraries/ktx/src/ktx/Writer.cpp index 4226b8fa84..50f63767a0 100644 --- a/libraries/ktx/src/ktx/Writer.cpp +++ b/libraries/ktx/src/ktx/Writer.cpp @@ -210,7 +210,8 @@ namespace ktx { if (currentDataSize + sizeof(uint32_t) < allocatedImagesDataSize) { uint32_t imageOffset = currentPtr - destBytes; size_t imageSize = srcImages[l]._imageSize; - *(reinterpret_cast (currentPtr)) = (uint32_t) imageSize; + size_t imageFaceSize = srcImages[l]._faceSize; + *(reinterpret_cast (currentPtr)) = (uint32_t)imageFaceSize; // the imageSize written in the ktx is the FACE size currentPtr += sizeof(uint32_t); currentDataSize += sizeof(uint32_t); diff --git a/libraries/model-networking/src/model-networking/KTXCache.cpp b/libraries/model-networking/src/model-networking/KTXCache.cpp index 8ec1c4e41c..e0447af8e6 100644 --- a/libraries/model-networking/src/model-networking/KTXCache.cpp +++ b/libraries/model-networking/src/model-networking/KTXCache.cpp @@ -22,7 +22,7 @@ KTXCache::KTXCache(const std::string& dir, const std::string& ext) : } KTXFilePointer KTXCache::writeFile(const char* data, Metadata&& metadata) { - FilePointer file = FileCache::writeFile(data, std::move(metadata)); + FilePointer file = FileCache::writeFile(data, std::move(metadata), true); return std::static_pointer_cast(file); } diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index 9653cde7d8..47fc62a9b5 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -792,6 +792,8 @@ void ImageReader::read() { texture = gpu::Texture::unserialize(ktxFile->getFilepath()); if (texture) { texture = textureCache->cacheTextureByHash(hash, texture); + } else { + qCWarning(modelnetworking) << "Invalid cached KTX " << _url << " under hash " << hash.c_str() << ", recreating..."; } } } @@ -835,7 +837,7 @@ void ImageReader::read() { const char* data = reinterpret_cast(memKtx->_storage->data()); size_t length = memKtx->_storage->size(); auto& ktxCache = textureCache->_ktxCache; - networkTexture->_file = ktxCache.writeFile(data, KTXCache::Metadata(hash, length)); + networkTexture->_file = ktxCache.writeFile(data, KTXCache::Metadata(hash, length)); // if (!networkTexture->_file) { qCWarning(modelnetworking) << _url << "file cache failed"; } else { diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 43055e7ed6..8f3509d8f3 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -97,7 +97,7 @@ FilePointer FileCache::addFile(Metadata&& metadata, const std::string& filepath) return file; } -FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { +FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata, bool overwrite) { assert(_initialized); std::string filepath = getFilepath(metadata.key); @@ -107,8 +107,13 @@ FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { // if file already exists, return it FilePointer file = getFile(metadata.key); if (file) { - qCWarning(file_cache, "[%s] Attempted to overwrite %s", _dirname.c_str(), metadata.key.c_str()); - return file; + if (!overwrite) { + qCWarning(file_cache, "[%s] Attempted to overwrite %s", _dirname.c_str(), metadata.key.c_str()); + return file; + } else { + qCWarning(file_cache, "[%s] Overwriting %s", _dirname.c_str(), metadata.key.c_str()); + file.reset(); + } } QSaveFile saveFile(QString::fromStdString(filepath)); diff --git a/libraries/networking/src/FileCache.h b/libraries/networking/src/FileCache.h index f77db555bc..908ddcd285 100644 --- a/libraries/networking/src/FileCache.h +++ b/libraries/networking/src/FileCache.h @@ -80,7 +80,7 @@ protected: /// must be called after construction to create the cache on the fs and restore persisted files void initialize(); - FilePointer writeFile(const char* data, Metadata&& metadata); + FilePointer writeFile(const char* data, Metadata&& metadata, bool overwrite = false); FilePointer getFile(const Key& key); /// create a file diff --git a/libraries/shared/src/shared/Storage.cpp b/libraries/shared/src/shared/Storage.cpp index f6585e6ecb..7cff876aa0 100644 --- a/libraries/shared/src/shared/Storage.cpp +++ b/libraries/shared/src/shared/Storage.cpp @@ -21,6 +21,10 @@ ViewStorage::ViewStorage(const storage::StoragePointer& owner, size_t size, cons StoragePointer Storage::createView(size_t viewSize, size_t offset) const { auto selfSize = size(); + if ((viewSize < 4) || ((viewSize & 0x3) != 0)) { + throw std::runtime_error("Invalid mapping range"); + } + if (0 == viewSize) { viewSize = selfSize; }