Commit graph

40 commits

Author SHA1 Message Date
Elliott Hughes
da675a945f Use the hermetic gzip instead of minigzip.
Bug: http://b/288169261
Test: treehugger
Change-Id: I7cb3d5468a79faedcae13d75913f11f5a8ea9e80
2023-06-21 10:10:42 -07:00
Kelvin Zhang
d1ba38f7c9 Check for overflow before allocating memory fore decompression.
On 32bit devices, an ZipEntry64 may have size > 2^32, we should check
for such cases before attempting to allocate memory.

Test: mm -j
Change-Id: I0f916ef4b2a692f167719a74bd6ff2e887c6c2ce
2020-09-18 17:41:51 -04:00
Kelvin Zhang
4f81130039 Switch to zip64 in recovery
There's already library support for zip64 in libziparchive. We just need
to start using the new APIs.

Bug: 167951876
Test: Sideload a large ota package in recovery
Change-Id: I652741965f28de079d873c6822317ee9fa855201
2020-09-16 14:21:37 -04:00
Tianjie
78d1514173 Update language to comply with Android’s inclusive language guidance
https: //source.android.com/setup/contribute/respectful-code
Bug: 161896447
Test: Unit tests pass
Change-Id: I0f3f0333dbccc94241a096ca5d3d9bc28c281492
2020-07-23 14:02:21 -07:00
Tianjie Xu
fb08b015f2 Address the warnings in recovery code
The following warnngs generate when compile with WITH_TIDY=1

.../bootable/recovery/applypatch/imgdiff.cpp:968:7: warning: 'src_ranges' used after it was moved [bugprone-use-after-move]
      src_ranges.Clear();
      ^
.../bootable/recovery/applypatch/imgdiff.cpp:966:27: note: move occurred here
        split_src_ranges->push_back(std::move(src_ranges));

The logic itself seems correct since the class is meant to be cleared
after move. I feel the std::move in 966 is actually useful to call the
move constructor in RangeSet. So I just modify L968 to suppress the
warning.

Less important ones:
bootable/recovery/applypatch/applypatch_modes.cpp:79:34: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg]
bootable/recovery/applypatch/imgdiff.cpp:1038:30: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg]
bootable/recovery/applypatch/imgdiff.cpp:1054:48: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg]
bootable/recovery/updater/include/private/commands.h:310:16: warning: std::move of the variable 'patch' of the trivially-copyable type 'PatchInfo' has no effect; remove std::move() [performance-move-const-arg]
bootable/recovery/updater/install.cpp:663:43: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg]

Bug: 150955971
Test: build
Change-Id: Ieb75f0229c47d470d4f5ac93fab39c5698d3f914
2020-03-07 17:54:11 -08:00
Elliott Hughes
88d8001e75 Move off the Next ZipString overload.
Bug: http://b/129068177
Test: treehugger
Change-Id: I3c8f70b0d8cc5dc6b3b4439dbe0b9a5bd85003c4
2019-05-22 18:52:29 -07:00
Elliott Hughes
143a03fa03 Track libziparchive API change.
Bug: http://b/129068177
Test: treehugger
Change-Id: I618bbcf38914dd81e042e0cfd1976ff26274dc30
2019-05-08 17:28:22 -07:00
Tianjie Xu
cc61cf6a9f Convert deflate image chunks to raw if the raw data is smaller
The imgpatch will fail on empty deflates because the bspatch won't call
the sink function if the target length is zero.

Instead of compressing an empty string, it's cleaner to not generate such
empty deflate chunks in the patch. Therefore, we can just convert the
chunk type to raw if the target length is smaller than the patch data.

Also adjust some unit tests and add the testdata gzipped_source &
gzipped_target. These two files are ~1K each and are generated by
gzipping two slightly different regular files.

Bug: 79265132
Test: unit tests pass, imgpatch applys successfully on the given src/tgt
Change-Id: I6bfff3251918137f6762a6f9e9551642371a1124
2018-05-24 10:49:54 -07:00
Tianjie Xu
572abbb81c Remove the assumption of target chunk size in imgdiff
In the split mode of imgdiff, we used to assume that the size of a split
target chunk is always greater than the blocksize i.e. 4096. This may
lead to the following assertion failure:
I0221 04:57:33.451323 818464 common.py:205 imgdiff F 02-21 04:57:33 821203 821203 imgdiff.cpp:999]
Check failed: tgt_size >= BLOCK_SIZE (tgt_size=476, BLOCK_SIZE=4096)

This CL removes the assumption and handles the edge cases.

Test: generate and verify the incremental update for TFs in the bug; unit test passes

Bug: 73757557
Bug: 73711365
Change-Id: Iadbb4ee658995f5856cd488f3793980881a59620
2018-02-23 10:44:05 -08:00
Tianjie Xu
6e293c99c1 Switch imgdiff to libbase logging
Also add a verbose option. And we won't print messages of 'info'
severity unless '-v' is present.

Test: run imgdiff and check the logs.
Change-Id: I1b90874baea8e72e2a2323a0b63bc5d35e653e6b
2017-11-15 22:35:39 -08:00
Treehugger Robot
3d24b7bca6 Merge "Use SuffixArrayIndexInterface opaque type instead of the underlying data pointer." 2017-10-24 17:20:14 +00:00
Alex Deymo
fa188268e4 Use SuffixArrayIndexInterface opaque type instead of the underlying data pointer.
bsdiff interface is changing such that it hides the suffix array
pointer from the public interface. This allows to use a different
suffix array data size depending on the input size, running much faster
in the normal case.

Bug: 34220646
Test: `make checkbuild`; Ran an incremental update generation on a non-A/B device.
Change-Id: I78e766da56cf28bc7774b8c8e58527bc11d919fb
2017-10-24 14:00:31 +02:00
Tao Bao
4568582002 otautil: Move RangeSet implementation into rangeset.cpp.
Since it has grown much larger, users of the header shouldn't compile
and carry their full copies.

Also add missing header includes in imgdiff.cpp and imgdiff_test.cpp.

Test: mmma bootable/recovery
Test: recovery_unit_test; recovery_component_test; recovery_host_test
Change-Id: I88ca54171765e5606ab0d61580fbc1ada578fd7d
2017-10-16 11:28:18 -07:00
Tao Bao
09e468f84c Move rangeset.h and print_sha1.h into otautil.
Also drop the "bootable/recovery" path in LOCAL_C_INCLUDES from
applypatch modules.

Test: lunch aosp_{angler,bullhead,fugu,dragon,sailfish}-userdebug;
      mmma bootable/recovery
Change-Id: Idd602a796894f971ee4f8fa3eafe36c42d9de986
2017-10-10 15:52:11 -07:00
Tianjie Xu
82582b4562 Output split information for imgdiff when handling large apks
Add a mandatory option in imgdiff to write the split info (i.e.
patch_size, tgt_size, src_ranges) to file when handling large apks.
Therefore, the caller of imgdiff can create split transfers based on
the info.

Bug: 63542719
Test: unit tests pass
Change-Id: I853d55d1f999fd576474faa81077f7307f4d856d
2017-09-21 11:42:16 -07:00
Tianjie Xu
2903cddb58 Improve imgdiff for large zip files
Due to the cache size limit for OTA generation, we used to split large
zip files linearly into pieces and do bsdiff on them. As a result, i) we
lose the advantage of imgdiff; ii) if there's an accidental order change
of some huge files inside the zip, we'll create an insanely large patch.

This patch splits the src&tgt more smartly based on the zip entry_name.
If the entry_name is empty or no matching source is found for a target
chunk, we'll skip adding its source and later do a bsdiff against the
whole split source image (this rarely happens in our use cases except
for the metadata inside a ziparchive).

After the split, the target pieces are continuous and block aligned,
while the sources pieces are mutually exclusive. (Some of the source
blocks may not be used if there's no matching entry_name in the target.)
Then we will generate patches accordingly between each split image
pairs.

Afterwards, if we apply imgpatch to each pair of split source/target
images and add up the patched result, we can get back the original
target image.

For example:
Input: [src_image, tgt_image]
Split: [src-0,tgt-0; src-1,tgt-1, src-2,tgt-2]
Diff:  [  patch-0;     patch-1;    patch-2]

Patch: [(src-0,patch-0)=tgt-0; (src-1,patch-1)=tgt-1;
(src-2,patch-2)=tgt-2;]
Append: [tgt-0 + tgt-1 + tgt-2 = tgt_image]

Peformance:
For the small package in b/34220646, we decrease the patch size of
chrome.apk dramatically from 30M to 400K due to the order change of
two big .so files.

On two versions of angler, I also observe decent patch size decrease.
For chrome.apk, we reduced the size from 5.9M to 3.2M; and for
vevlet.apk from 8.0M to 6.5M.

Bug: 34220646
Test: recovery component test && apply imgdiff & imgpatch on two
chrome.apk
Change-Id: I145d802984fa805efbbac9d01a2e64d82ef9728b
2017-09-05 15:09:58 -07:00
Tianjie Xu
57dd961995 Move Image/ImageChunk/PatchChunk declaration into header files
1. Move the declaration of the Image classes to the header file to make
testing easier.
2. Also move rangeset.h to bootable/recovery to allow access in imgdiff.

Test: recovery component test
Change-Id: I68a863e60a3f2e7ae46ee48f48eb15391f5f4330
2017-08-18 17:56:22 -07:00
Tianjie Xu
d82a2ed50b Add a new PatchChunk class in imgdiff
This way we can keep the input images const when calling
genetatepatches().

Test: recovery component test; diff and patch on chrome.apk; generate
recovery-from-boot.p for angler.
Change-Id: I65b5689b88f6719c6ede46bb82def0c4caeb8a61
2017-08-12 22:07:43 -07:00
Tianjie Xu
6b03ba7902 Refactor the imgdiff
This helps us to add a new mode to handle large APKs in the follow
up CL.

Changes include:
1. Create a new interface class 'Image'
1. Create subclasses 'ZipModeImage' and 'ImageModeImage' and move the
   related functions there.

Bug: 63542719
Test: recovery_component_test passes
Change-Id: I7729b0ba39b19a9c84811636a60dd0a0b1acc2f0
2017-08-02 14:29:45 -07:00
Tianjie Xu
14ebc1e5ae Fix a rare failure for imgdiff when random data equals gzip header
In a rare case, a random chunk will pass both the gzip header check
and the inflation process; but fail the uncompressed length check in the
footer. This leads to a imgdiff failure. So, we should treat this chunk
as 'normal' instead of 'inflated' while generating the patch.

Bug: 63334984
Test: imgdiff generates patch successfully on previous failing images.
Change-Id: Ice84f22d3653bce9756bda91e70528c0d2f264a0
2017-07-06 16:49:11 -07:00
Tianjie Xu
12b90553d7 More cleanup to imgdiff & imgpatch
Also remove the utils in applypatch and replace them with the
corresponding libbase functions.

Test: recovery tests pass.
Change-Id: I77254c141bd3e7d3d6894c23b60e866009516f81
2017-03-16 12:09:49 -07:00
Tianjie Xu
1ea84d6da9 Refractor the code for imgdiff
Put ImageChunk and some helper functions into a class. Also switch to
using std::vector instead of malloc.

Bug: 18606652
Test: imgdiff_test passed on host. Also generate a complete incremental OTA package.
The file content is the same and time consumption is similar.

Change-Id: Id603ada4e130ef521218400761a119001a86ca79
2017-03-06 15:41:02 -08:00
Tao Bao
7ab9c3812d Merge "bootable/recovery: cleanup compiler warnings (potential leak of memory)" 2017-02-01 05:10:15 +00:00
Sen Jiang
930edb6661 imgdiff: cache bsdiff suffix array in zip mode.
In zip mode, if a chunk is not deflate or its filename can't be found
in source chunks, the entire source file is used as old data for bsdiff,
To avoid repeatedly construct the suffix array used by bsdiff, we cache
the suffix array of the entire source file.

Bug: 34281147
Test: =time -v imgdiff -z Chrome-ORF74B.apk Chrome-ORF76B.apk Chrome.imgdiff
Change-Id: Ifd957ccecf7226fcb44dbf28c58969a06ef74f4b
2017-01-18 17:26:42 -08:00
Tao Bao
d37ce8f082 imgdiff: Fix an edge case that leads to infinite loop.
When the input image ends with the magic value sequence of 0x1f, 0x8b,
0x0b (optionally with 0x00), the image parsing code will be stuck in an
infinite loop.

Test: recovery_component_test passes.
Change-Id: Ie3629dfdc41360387b19cc3e0359c95ae4fb998e
2016-12-28 14:29:48 -08:00
Tao Bao
97555da4a6 Add tests for imgdiff.
Factor out libimgdiff static library for testing purpose.

This CL adds the imgdiff tests on host and on target both (similar to
libimgpatch). In practice, we only need imgdiff binary on host, and
libimgpatch on target. But they should build and pass tests on both
platforms.

Test: recovery_host_test passes; recovery_component_test passes.
Change-Id: I0eafb7faf727cdf70066310e845af6ee245d4f60
2016-12-19 16:53:03 -08:00
Rahul Chaudhry
8b640ffabc bootable/recovery: cleanup compiler warnings (potential leak of memory)
bootable/recovery/applypatch/imgdiff.cpp:195:5: warning: Potential leak
of memory pointed to by 'img' [clang-analyzer-unix.Malloc]

Bug: 26936282
Test: WITH_TIDY=1 WITH_STATIC_ANALYZER=1 mm
Change-Id: Ie79c780233ddfebf85686a24df3bf2561f831580
2016-12-06 15:10:41 -08:00
Tao Bao
ef3bea15b8 Merge "bootable/recovery: cleanup compiler warnings (unused value)" 2016-12-01 17:03:13 +00:00
Tao Bao
d255b661a1 Merge "bootable/recovery: cleanup compiler warnings (potential leak of memory)" 2016-11-30 21:19:42 +00:00
Rahul Chaudhry
a793c58208 bootable/recovery: cleanup compiler warnings (unused value)
bootable/recovery/applypatch/imgdiff.cpp:322:11: warning: Value stored to 'ret' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
bootable/recovery/applypatch/imgdiff.cpp:447:11: warning: Value stored to 'ret' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
bootable/recovery/applypatch/imgdiff.cpp:553:3: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]

Bug: 26936282
Test: WITH_TIDY=1 WITH_STATIC_ANALYZER=1 mm
Change-Id: I3f865e3e9b9d19e5ea5e8dfd2fe2c644254ffbb5
2016-11-30 11:18:32 -08:00
Rahul Chaudhry
b29f23f7e7 Use static_cast to cast pointers returned by malloc/calloc/realloc/mmap.
static_cast is preferable to reinterpret_cast when casting from void*
pointers returned by malloc/calloc/realloc/mmap calls.

Discovered while looking at compiler warnings (b/26936282).

Test: WITH_TIDY=1 WITH_STATIC_ANALYZER=1 mma
Change-Id: Iaffd537784aa857108f6981fdfd82d0496eb5592
Merged-In: I151642d5a60c94f312d0611576ad0143c249ba3d
2016-11-15 23:24:54 -08:00
Rahul Chaudhry
3a5177b16d bootable/recovery: cleanup compiler warnings (potential leak of memory)
bootable/recovery/applypatch/imgdiff.cpp:1065:3: warning: Potential leak of memory pointed to by 'patch_data'
bootable/recovery/applypatch/imgdiff.cpp:1065:3: warning: Potential leak of memory pointed to by 'patch_size'
bootable/recovery/applypatch/imgdiff.cpp:226:7: warning: Potential leak of memory pointed to by 'temp_entries'

Bug: 26936282
Test: WITH_TIDY=1 WITH_STATIC_ANALYZER=1 mm
Change-Id: I3cac945d7677d367934d5619ef7419daf6f48d6f
2016-11-15 16:18:46 -08:00
Tao Bao
a0c4011028 Revert "Fix memory/resource handling in imgdiff.cpp, using unique_ptr and vector."
This reverts commit 50a6f8c833.

A mix of new and free leads to memory corruptions.
2016-06-01 13:15:44 -07:00
Adam Buchbinder
50a6f8c833 Fix memory/resource handling in imgdiff.cpp, using unique_ptr and vector.
This fixes the following cppcheck reports:
[imgdiff.cpp:197]: (error) Memory leak: img
[imgdiff.cpp:386]: (error) Memory leak: img
[imgdiff.cpp:656]: (error) Memory leak: data
[imgdiff.cpp:664]: (error) Memory leak: data
[imgdiff.cpp:668]: (error) Memory leak: data
[imgdiff.cpp:668]: (error) Resource leak: f
[imgdiff.cpp:820]: (error) Memory leak: bonus_data
[imgdiff.cpp:824]: (error) Memory leak: bonus_data
[imgdiff.cpp:824]: (error) Resource leak: f
[imgdiff.cpp:847]: (error) Memory leak: bonus_data
[imgdiff.cpp:851]: (error) Memory leak: bonus_data
[imgdiff.cpp:856]: (error) Memory leak: bonus_data
[imgdiff.cpp:860]: (error) Memory leak: bonus_data
[imgdiff.cpp:227]: (error) Memory leak: temp_entries

Change-Id: I06f878d0b677a25328e0deb84f65f3b7212e24b1
(cherry picked from commit 51dc9444bc)
2016-06-01 10:28:45 -07:00
Sen Jiang
2fffcb174b applypatch: Use bsdiff from external/bsdiff.
external/bsdiff uses divsufsort which is much faster, and also include
some bug fixes.

Bug: 26982501
Test: ./imgdiff_test.sh

Change-Id: I089a301c291ee55d79938c7a3ca6d509895440d1
2016-05-03 16:39:51 -07:00
Sen Jiang
fa4f1b75b6 imgdiff: don't fail if gzip is corrupted.
Treat it as a normal chunk if inflate() fails.

Test: run imgdiff on corrupted gzip and apply the patch
Bug: 27153028
Change-Id: Idcbb3c1360ec0774e6c7861b21d99af8ee10604a
2016-02-12 12:57:03 -08:00
Sen Jiang
696692a3c9 applypatch: Add a Makefile to build imgdiff in Chrome OS.
Also fixed some warnings and added check for target_len.

Test: mma; emerge-peppy imgdiff; emerge-nyan imgdiff; sudo emerge imgdiff
Bug: 26866274

Change-Id: Ifbcd3afd6701c769ccf626e33ed94461706f7ee6
2016-02-09 15:26:57 -08:00
David Riley
0779fc9814 imgdiff: skip spurious gzip headers in image files
dragon kernel is compressed via lz4 for boot speed and bootloader
support reasons and recent prebuilts happen to include the gzip header
sequence which is causing imgdiff to fail.

Detect a spurious gzip header and treat the section as a normal section.

Bug: 26133184
Change-Id: I369d7d576fd7d2c579c0780fc5c669a5b6ea0d3d
(cherry picked from commit 0f2f6a746af517afca9e5e089a4a17be0a9766d6)
Signed-off-by: David Riley <davidriley@google.com>
2015-12-23 09:43:59 -08:00
Jeremy Compostella
a91c66d7c1 imgdiff: fix file descriptor leak
mkstemp() allocates a file description that is never released.  If
MakePatch() is called too many time, imgdiff reaches the Operating
System EMFILE (too many open files) limit.

Change-Id: Icbe1399f6f6d32cfa1830f879cacf7d75bbd9fc3
Signed-off-by: Jeremy Compostella <jeremy.compostella@intel.com>
Signed-off-by: Gaelle Nassiet <gaellex.nassiet@intel.com>
2015-09-08 19:15:09 +02:00
Tao Bao
ba9a42aa7e recovery: Switch applypatch/ and updater/ to cpp.
Mostly trivial changes to make cpp compiler happy.

Change-Id: I69bd1d96fcccf506007f6144faf37e11cfba1270
2015-07-13 17:21:31 -07:00
Renamed from applypatch/imgdiff.c (Browse further)