Revert^2 "libandroidfw hardening for IncFs"
55ef6167a2c235bd88c7216238b2001b46795b79
Change-Id: I02d4890d181655dfd0a14c188468db512559d27b
diff --git a/libs/androidfw/Asset.cpp b/libs/androidfw/Asset.cpp
index cd30c18..4fbe4a3 100644
--- a/libs/androidfw/Asset.cpp
+++ b/libs/androidfw/Asset.cpp
@@ -298,34 +298,18 @@
/*
* Create a new Asset from a memory mapping.
*/
-/*static*/ Asset* Asset::createFromUncompressedMap(FileMap* dataMap, AccessMode mode)
+/*static*/ std::unique_ptr<Asset> Asset::createFromUncompressedMap(incfs::IncFsFileMap&& dataMap,
+ AccessMode mode,
+ base::unique_fd fd)
{
- _FileAsset* pAsset;
- status_t result;
+ auto pAsset = util::make_unique<_FileAsset>();
- pAsset = new _FileAsset;
- result = pAsset->openChunk(dataMap, base::unique_fd(-1));
- if (result != NO_ERROR) {
- delete pAsset;
- return NULL;
- }
-
- pAsset->mAccessMode = mode;
- return pAsset;
-}
-
-/*static*/ std::unique_ptr<Asset> Asset::createFromUncompressedMap(std::unique_ptr<FileMap> dataMap,
- base::unique_fd fd, AccessMode mode)
-{
- std::unique_ptr<_FileAsset> pAsset = util::make_unique<_FileAsset>();
-
- status_t result = pAsset->openChunk(dataMap.get(), std::move(fd));
+ status_t result = pAsset->openChunk(std::move(dataMap), std::move(fd));
if (result != NO_ERROR) {
return NULL;
}
// We succeeded, so relinquish control of dataMap
- (void) dataMap.release();
pAsset->mAccessMode = mode;
return std::move(pAsset);
}
@@ -333,35 +317,18 @@
/*
* Create a new Asset from compressed data in a memory mapping.
*/
-/*static*/ Asset* Asset::createFromCompressedMap(FileMap* dataMap,
- size_t uncompressedLen, AccessMode mode)
+/*static*/ std::unique_ptr<Asset> Asset::createFromCompressedMap(incfs::IncFsFileMap&& dataMap,
+ size_t uncompressedLen,
+ AccessMode mode)
{
- _CompressedAsset* pAsset;
- status_t result;
+ auto pAsset = util::make_unique<_CompressedAsset>();
- pAsset = new _CompressedAsset;
- result = pAsset->openChunk(dataMap, uncompressedLen);
- if (result != NO_ERROR) {
- delete pAsset;
- return NULL;
- }
-
- pAsset->mAccessMode = mode;
- return pAsset;
-}
-
-/*static*/ std::unique_ptr<Asset> Asset::createFromCompressedMap(std::unique_ptr<FileMap> dataMap,
- size_t uncompressedLen, AccessMode mode)
-{
- std::unique_ptr<_CompressedAsset> pAsset = util::make_unique<_CompressedAsset>();
-
- status_t result = pAsset->openChunk(dataMap.get(), uncompressedLen);
+ status_t result = pAsset->openChunk(std::move(dataMap), uncompressedLen);
if (result != NO_ERROR) {
return NULL;
}
// We succeeded, so relinquish control of dataMap
- (void) dataMap.release();
pAsset->mAccessMode = mode;
return std::move(pAsset);
}
@@ -414,7 +381,7 @@
* Constructor.
*/
_FileAsset::_FileAsset(void)
- : mStart(0), mLength(0), mOffset(0), mFp(NULL), mFileName(NULL), mFd(-1), mMap(NULL), mBuf(NULL)
+ : mStart(0), mLength(0), mOffset(0), mFp(NULL), mFileName(NULL), mFd(-1), mBuf(NULL)
{
// Register the Asset with the global list here after it is fully constructed and its
// vtable pointer points to this concrete type. b/31113965
@@ -441,7 +408,7 @@
status_t _FileAsset::openChunk(const char* fileName, int fd, off64_t offset, size_t length)
{
assert(mFp == NULL); // no reopen
- assert(mMap == NULL);
+ assert(!mMap.has_value());
assert(fd >= 0);
assert(offset >= 0);
@@ -484,15 +451,15 @@
/*
* Create the chunk from the map.
*/
-status_t _FileAsset::openChunk(FileMap* dataMap, base::unique_fd fd)
+status_t _FileAsset::openChunk(incfs::IncFsFileMap&& dataMap, base::unique_fd fd)
{
assert(mFp == NULL); // no reopen
- assert(mMap == NULL);
+ assert(!mMap.has_value());
assert(dataMap != NULL);
- mMap = dataMap;
+ mMap = std::move(dataMap);
mStart = -1; // not used
- mLength = dataMap->getDataLength();
+ mLength = mMap->length();
mFd = std::move(fd);
assert(mOffset == 0);
@@ -528,10 +495,15 @@
if (!count)
return 0;
- if (mMap != NULL) {
+ if (mMap.has_value()) {
/* copy from mapped area */
//printf("map read\n");
- memcpy(buf, (char*)mMap->getDataPtr() + mOffset, count);
+ const auto readPos = mMap->data().offset(mOffset).convert<char>();
+ if (!readPos.verify(count)) {
+ return -1;
+ }
+
+ memcpy(buf, readPos.unsafe_ptr(), count);
actual = count;
} else if (mBuf != NULL) {
/* copy from buffer */
@@ -594,10 +566,6 @@
*/
void _FileAsset::close(void)
{
- if (mMap != NULL) {
- delete mMap;
- mMap = NULL;
- }
if (mBuf != NULL) {
delete[] mBuf;
mBuf = NULL;
@@ -624,16 +592,21 @@
* level and we'd be using a different object, but we didn't, so we
* deal with it here.
*/
-const void* _FileAsset::getBuffer(bool wordAligned)
+const void* _FileAsset::getBuffer(bool aligned)
+{
+ return getIncFsBuffer(aligned).unsafe_ptr();
+}
+
+incfs::map_ptr<void> _FileAsset::getIncFsBuffer(bool aligned)
{
/* subsequent requests just use what we did previously */
if (mBuf != NULL)
return mBuf;
- if (mMap != NULL) {
- if (!wordAligned) {
- return mMap->getDataPtr();
+ if (mMap.has_value()) {
+ if (!aligned) {
+ return mMap->data();
}
- return ensureAlignment(mMap);
+ return ensureAlignment(*mMap);
}
assert(mFp != NULL);
@@ -671,47 +644,44 @@
mBuf = buf;
return mBuf;
} else {
- FileMap* map;
-
- map = new FileMap;
- if (!map->create(NULL, fileno(mFp), mStart, mLength, true)) {
- delete map;
+ incfs::IncFsFileMap map;
+ if (!map.Create(fileno(mFp), mStart, mLength, NULL /* file_name */ )) {
return NULL;
}
ALOGV(" getBuffer: mapped\n");
- mMap = map;
- if (!wordAligned) {
- return mMap->getDataPtr();
+ mMap = std::move(map);
+ if (!aligned) {
+ return mMap->data();
}
- return ensureAlignment(mMap);
+ return ensureAlignment(*mMap);
}
}
int _FileAsset::openFileDescriptor(off64_t* outStart, off64_t* outLength) const
{
- if (mMap != NULL) {
+ if (mMap.has_value()) {
if (mFd.ok()) {
- *outStart = mMap->getDataOffset();
- *outLength = mMap->getDataLength();
- const int fd = dup(mFd);
- if (fd < 0) {
- ALOGE("Unable to dup fd (%d).", mFd.get());
- return -1;
- }
- lseek64(fd, 0, SEEK_SET);
- return fd;
+ *outStart = mMap->offset();
+ *outLength = mMap->length();
+ const int fd = dup(mFd);
+ if (fd < 0) {
+ ALOGE("Unable to dup fd (%d).", mFd.get());
+ return -1;
+ }
+ lseek64(fd, 0, SEEK_SET);
+ return fd;
}
- const char* fname = mMap->getFileName();
+ const char* fname = mMap->file_name();
if (fname == NULL) {
fname = mFileName;
}
if (fname == NULL) {
return -1;
}
- *outStart = mMap->getDataOffset();
- *outLength = mMap->getDataLength();
+ *outStart = mMap->offset();
+ *outLength = mMap->length();
return open(fname, O_RDONLY | O_BINARY);
}
if (mFileName == NULL) {
@@ -722,16 +692,21 @@
return open(mFileName, O_RDONLY | O_BINARY);
}
-const void* _FileAsset::ensureAlignment(FileMap* map)
+incfs::map_ptr<void> _FileAsset::ensureAlignment(const incfs::IncFsFileMap& map)
{
- void* data = map->getDataPtr();
- if ((((size_t)data)&0x3) == 0) {
+ const auto data = map.data();
+ if (util::IsFourByteAligned(data)) {
// We can return this directly if it is aligned on a word
// boundary.
ALOGV("Returning aligned FileAsset %p (%s).", this,
getAssetSource());
return data;
}
+
+ if (!data.convert<uint8_t>().verify(mLength)) {
+ return NULL;
+ }
+
// If not aligned on a word boundary, then we need to copy it into
// our own buffer.
ALOGV("Copying FileAsset %p (%s) to buffer size %d to make it aligned.", this,
@@ -741,7 +716,8 @@
ALOGE("alloc of %ld bytes failed\n", (long) mLength);
return NULL;
}
- memcpy(buf, data, mLength);
+
+ memcpy(buf, data.unsafe_ptr(), mLength);
mBuf = buf;
return buf;
}
@@ -757,7 +733,7 @@
*/
_CompressedAsset::_CompressedAsset(void)
: mStart(0), mCompressedLen(0), mUncompressedLen(0), mOffset(0),
- mMap(NULL), mFd(-1), mZipInflater(NULL), mBuf(NULL)
+ mFd(-1), mZipInflater(NULL), mBuf(NULL)
{
// Register the Asset with the global list here after it is fully constructed and its
// vtable pointer points to this concrete type. b/31113965
@@ -786,7 +762,7 @@
int compressionMethod, size_t uncompressedLen, size_t compressedLen)
{
assert(mFd < 0); // no re-open
- assert(mMap == NULL);
+ assert(!mMap.has_value());
assert(fd >= 0);
assert(offset >= 0);
assert(compressedLen > 0);
@@ -815,20 +791,20 @@
*
* Nothing is expanded until the first read call.
*/
-status_t _CompressedAsset::openChunk(FileMap* dataMap, size_t uncompressedLen)
+status_t _CompressedAsset::openChunk(incfs::IncFsFileMap&& dataMap, size_t uncompressedLen)
{
assert(mFd < 0); // no re-open
- assert(mMap == NULL);
+ assert(!mMap.has_value());
assert(dataMap != NULL);
- mMap = dataMap;
+ mMap = std::move(dataMap);
mStart = -1; // not used
- mCompressedLen = dataMap->getDataLength();
+ mCompressedLen = mMap->length();
mUncompressedLen = uncompressedLen;
assert(mOffset == 0);
if (uncompressedLen > StreamingZipInflater::OUTPUT_CHUNK_SIZE) {
- mZipInflater = new StreamingZipInflater(dataMap, uncompressedLen);
+ mZipInflater = new StreamingZipInflater(&(*mMap), uncompressedLen);
}
return NO_ERROR;
}
@@ -901,11 +877,6 @@
*/
void _CompressedAsset::close(void)
{
- if (mMap != NULL) {
- delete mMap;
- mMap = NULL;
- }
-
delete[] mBuf;
mBuf = NULL;
@@ -940,8 +911,8 @@
goto bail;
}
- if (mMap != NULL) {
- if (!ZipUtils::inflateToBuffer(mMap->getDataPtr(), buf,
+ if (mMap.has_value()) {
+ if (!ZipUtils::inflateToBuffer(mMap->data(), buf,
mUncompressedLen, mCompressedLen))
goto bail;
} else {
@@ -976,3 +947,6 @@
return mBuf;
}
+incfs::map_ptr<void> _CompressedAsset::getIncFsBuffer(bool aligned) {
+ return incfs::map_ptr<void>(getBuffer(aligned));
+}