Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a720635b authored by Fabien Sanglard's avatar Fabien Sanglard
Browse files

Fix zipalign alignment error 

Problem: Zipalign operates over several false assumptions. First it
assumes that zip entries are in the same order in the body and in
the Central Direcotry. Second, it assumes there are not space
between entries. This makes alignment incorrect when these asserts
are not true.

Solution: Don't align entries by tracking bias based on input zip
entry location. Calculate the expected alignment based on the out-
put zip file and correct with extra padding.

Fixes: 162117652
Test: Units Tests
Change-Id: Ia179338f658cab18a377cba2c7c8e629089a2785
parent 6f83343e

tools/zipalign/Android.bp

+2 −0
Original line number Diff line number Diff line
@@ -63,6 +63,8 @@ cc_test_host { 
        "libgmock",

    ],

    data: [

         "tests/data/diffOrders.zip",

         "tests/data/holes.zip",

         "tests/data/unaligned.zip",

    ],

    defaults: ["zipalign_defaults"],

tools/zipalign/ZipAlign.cpp

+1 −12
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ static int copyAndAlign(ZipFile* pZin, ZipFile* pZout, int alignment, bool zopfl 
{

    int numEntries = pZin->getNumEntries();

    ZipEntry* pEntry;

    int bias = 0;

    status_t status;



    for (int i = 0; i < numEntries; i++) {
@@ -68,30 +67,20 @@ static int copyAndAlign(ZipFile* pZin, ZipFile* pZout, int alignment, bool zopfl 


            if (zopfli) {

                status = pZout->addRecompress(pZin, pEntry, &pNewEntry);

                bias += pNewEntry->getCompressedLen() - pEntry->getCompressedLen();

            } else {

                status = pZout->add(pZin, pEntry, padding, &pNewEntry);

            }

        } else {

            const int alignTo = getAlignment(pageAlignSharedLibs, alignment, pEntry);



            /*

             * Copy the entry, adjusting as required.  We assume that the

             * file position in the new file will be equal to the file

             * position in the original.

             */

            off_t newOffset = pEntry->getFileOffset() + bias;

            padding = (alignTo - (newOffset % alignTo)) % alignTo;



            //printf("--- %s: orig at %ld(+%d) len=%ld, adding pad=%d\n",

            //    pEntry->getFileName(), (long) pEntry->getFileOffset(),

            //    bias, (long) pEntry->getUncompressedLen(), padding);

            status = pZout->add(pZin, pEntry, padding, &pNewEntry);

            status = pZout->add(pZin, pEntry, alignTo, &pNewEntry);

        }



        if (status != OK)

            return 1;

        bias += padding;

        //printf(" added '%s' at %ld (pad=%d)\n",

        //    pNewEntry->getFileName(), (long) pNewEntry->getFileOffset(),

        //    padding);

tools/zipalign/ZipFile.cpp

+31 −6
Original line number Diff line number Diff line
@@ -502,6 +502,32 @@ bail: 
    return result;

}



/*

 * Based on the current position in the output zip, assess where the entry

 * payload will end up if written as-is. If alignment is not satisfactory,

 * add some padding in the extra field.

 *

 */

status_t ZipFile::alignEntry(android::ZipEntry* pEntry, uint32_t alignTo){

    if (alignTo == 0 || alignTo == 1)

        return OK;



    // Calculate where the entry payload offset will end up if we were to write

    // it as-is.

    uint64_t expectedPayloadOffset = ftell(mZipFp) +

        android::ZipEntry::LocalFileHeader::kLFHLen +

        pEntry->mLFH.mFileNameLength +

        pEntry->mLFH.mExtraFieldLength;



    // If the alignment is not what was requested, add some padding in the extra

    // so the payload ends up where is requested.

    uint64_t alignDiff = alignTo - (expectedPayloadOffset % alignTo);

    if (alignDiff == 0)

        return OK;



    return pEntry->addPadding(alignDiff);

}



/*

 * Add an entry by copying it from another zip file.  If "padding" is

 * nonzero, the specified number of bytes will be added to the "extra"
@@ -510,7 +536,7 @@ bail: 
 * If "ppEntry" is non-NULL, a pointer to the new entry will be returned.

 */

status_t ZipFile::add(const ZipFile* pSourceZip, const ZipEntry* pSourceEntry,

    int padding, ZipEntry** ppEntry)

    int alignTo, ZipEntry** ppEntry)

{

    ZipEntry* pEntry = NULL;

    status_t result;
@@ -537,11 +563,10 @@ status_t ZipFile::add(const ZipFile* pSourceZip, const ZipEntry* pSourceEntry, 
    result = pEntry->initFromExternal(pSourceEntry);

    if (result != OK)

        goto bail;

    if (padding != 0) {

        result = pEntry->addPadding(padding);



    result = alignEntry(pEntry, alignTo);

    if (result != OK)

      goto bail;

    }



    /*

     * From here on out, failures are more interesting.

tools/zipalign/ZipFile.h

+6 −4
Original line number Diff line number Diff line
@@ -102,14 +102,14 @@ public: 
    }



    /*

     * Add an entry by copying it from another zip file.  If "padding" is

     * nonzero, the specified number of bytes will be added to the "extra"

     * field in the header.

     * Add an entry by copying it from another zip file.  If "alignment" is

     * nonzero, an appropriate number of bytes will be added to the "extra"

     * field in the header so the entry payload is aligned.

     *

     * If "ppEntry" is non-NULL, a pointer to the new entry will be returned.

     */

    status_t add(const ZipFile* pSourceZip, const ZipEntry* pSourceEntry,

        int padding, ZipEntry** ppEntry);

        int alignment, ZipEntry** ppEntry);



    /*

     * Add an entry by copying it from another zip file, recompressing with
@@ -163,6 +163,8 @@ private: 
    ZipFile(const ZipFile& src);

    ZipFile& operator=(const ZipFile& src);



    status_t alignEntry(android::ZipEntry* pEntry, uint32_t alignTo);



    class EndOfCentralDir {

    public:

        EndOfCentralDir(void) :

tools/zipalign/tests/data/diffOrders.zip

0 → 100644
+220 B

File added.

No diff preview for this file type.

View file