Commit Graph

195 Commits

Author SHA1 Message Date
Tao Bao
e02cbaaa62 Merge "applypatch: Restrict applypatch_check to eMMC targets." 2018-07-10 06:19:19 +00:00
Tao Bao
7c1d426dbc applypatch: Restrict applypatch_check to eMMC targets.
Also fix an error-pone behavior in previous code when verifying an eMMC
target. As long as it loads the partition content successfully according
to the SHAs embedded in the filename, it shouldn't further check against
the SHAs given in the second argument. Because the loaded contents
relate to a specific partition size.

For example:

  apply_patch_check(
      "EMMC:/boot.img:src_size:src_hash:tgt_size:tgt_hash",
      "src_hash");

Assume "/boot.img" already has the desired hash of "tgt_hash", the
previous code would give wrong verification result. The issue can be
addressed by additionally listing "tgt_hash" as one of the desired SHAs
(or by applying this CL).

Bug: 110106408
Test: Run recovery_unit_test and recovery_component_test on marlin.
Change-Id: I8daafdbecd083f687e24d563ab089caa25667633
2018-07-09 21:13:56 -07:00
Tao Bao
7ea515e6fc applypatch: Fix a potential nullptr dereferencing.
Note that the code exists in debugging path only, and won't be hit
unless device has flaky flash.

Test: Run recovery_unit_test and recovery_component_test on marlin.
Change-Id: I0c71adc271f08f00e3eabd9d14cd8af3186c5bae
2018-07-09 15:22:35 -07:00
Tao Bao
8dc704930b applypatch: {Load,Save}FileContents and ParseSha1 take std::string.
Test: mmma -j bootable/recovery
Test: Run recovery_component_test on marlin.
Change-Id: Ifcf244346a88dac833d91b169a4c2aee1fe677f1
2018-06-20 13:21:16 -07:00
Tao Bao
859bfc5659 applypatch: Move to libbase logging.
Test: mmma -j bootable/recovery
Test: Run recovery_component_test on marlin.
Change-Id: I3a4e32eb2c1e231c6690a50571edc66341b8d50a
2018-06-20 13:19:25 -07:00
Tao Bao
155771bafa applypatch: Clean up the function comments.
Also two minor changes (other than renaming some parameters):
 - Added constness to the first parameter of FindMatchingPatch();
 - Declared WriteToPartition() as static.

Bug: 110106408
Test: mmma -j bootable/recovery
Change-Id: I388958c944a23ce4a38a757ce2249f6a89dd4f03
2018-06-19 16:33:50 -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
3f638ee834 Save the target file when applypatch tests fail
Save the target file to tempfile upon unittest failures so that we can
try to decompress the deflate chunks in the flaky unittests. And print
the zlib version in case that gets changed.

Also the SHA1 of the uncompressed data seems correct; so only keep the
final SHA1 to double confirm.

Bug: 67849209
Test: recovery_component_test
Change-Id: Ic6447c2b75c29379d6844cd23a0ff1c4305694a0
2018-04-26 19:46:39 -07:00
Tianjie Xu
cc8587b508 Merge "Dump the uncompressed data's SHA1 to debug flaky tests" 2018-04-26 17:23:00 +00:00
Tao Bao
641fa97def Rename CacheLocation to Paths.
We have a general need for overriding more paths (e.g. "/tmp"), mostly
for testing purpose. Rename CacheLocation to Paths, and use that to
manage TEMPORARY_{INSTALL,LOG}_FILE.

Test: mmma -j bootable/recovery
Test: recovery_component_test
Change-Id: Ia8ce8e5695df37ca434f13ac4d3206de1e8e9396
2018-04-25 21:46:00 -07:00
Tianjie Xu
9e1ccd47b4 Dump the uncompressed data's SHA1 to debug flaky tests
Dump the SHA1 of the uncompressed data in applypatch to confirm if we
are at least doing the bspatch part correctly. (I expect so since the actual
length of the uncompressed data matches the expected length).

Also try to decompress the deflate chunk inside the recovery image for
these two flacky tests. In theory, there shouldn't be randomness in
zlib; so we would know if we process the data wrongly if the deflate fails
to decompress.

Bug: 67849209
Test: recovery_component_test
Change-Id: Id947522153b1eeb0d10d161298a96fb045f92018
2018-04-25 20:00:56 -07:00
Tianjie Xu
ffed57a7a3 Dump debug information for apply_patch unit tests
The apply patch test should have a deterministic way to append patch
data. Add debug logs to dump the length and SHA1 of each step to further
track down the flakiness.

Also redirect the debug logging to stdout in case the logcat becomes too
chatty.

Bug: 67849209
Test: Run recovery_component_test
Change-Id: I42bafef2d9dee599719ae57840b3d8c00d243ebd
2018-04-24 09:56:55 -07:00
Tao Bao
8b0b0f1f02 applypatch: Drop the SHA_CTX parameter in Apply{BSDiff,Image}Patch.
As they're accepting the SinkFn callback, it makes more sense to leave
the work to their callers.

Test: mmma -j bootable/recovery
Test: Run recovery_component_test on marlin.
Test: No other active user of the two functions.
Change-Id: I8d67b38ce037925442296f136b483e0c71983777
2018-04-20 09:27:50 -07:00
Tao Bao
4f83430471 applypatch: Dump patch info on mismatching patching result.
After splitting the previously flaky
ApplyPatchModesTest#PatchModeEmmcTarget tests,
PatchModeEmmcTargetWithMultiplePatches now becomes the sole victim. This
CL dumps additional info to narrow down the cause.

Bug: 67849209
Test: `recovery_component_test` on marlin.
Test: It dumps additional info after using corrupt bonus.file.
Change-Id: Ic5436de457cc882a51d03f49d5cee70077f7d3df
2018-04-19 12:46:22 -07:00
Tianjie Xu
d5fbcc1ba9 Remove the old log files if cache space is insufficient for OTA
We set the limit of the max stash size to 80% of cache size. But the
cache space can still be insufficient for the update if the log files
occupy a large chunk of /cache. So remove the old logs for now to make
room for the update.

Bug: 77528881
Test: unit tests pass
Change-Id: Ia8bcb0ace11f8164ad9290bfb360e08e31d282cb
2018-04-13 11:33:47 -07:00
Tianjie Xu
3bbb20f557 Add a singleton CacheLocation to replace the hard coded locations
This class allows us to set the following locations dynamically:
cache_temp_source, last_command_file, stash_directory_base.

In the updater's main function, we reset the values of these variables
to their default locations in /cache; while we can set them to temp
files in unit tests or host simulation.

Test: unit tests pass
Change-Id: I528652650caa41373617ab055d41b1f1a4ec0f87
2018-02-28 11:19:11 -08: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
7a4dacf724 Disable building libapplypatch on mac
The sdk_mac on build server fails with the error:
bootable/recovery/applypatch/freecache.cpp:23:10: fatal error: 'sys/statfs.h' file not found

So we will disable libapplypatch on mac.

Test: the library still builds on linux; and check the other host targets in the same cl.
Change-Id: Ie4a30708726e51c810f7ad7f1085d38154076cca
2018-02-17 22:06:31 -08:00
Tianjie Xu
e40c80d021 Skip the cache size check on host
When running the update simulation, we choose to skip the cache size
check for now due to the lack of "/cache" on host. And in later cls we
can implement a cache size estimator to make the check more constrained.

Also build the host version of support libraries.

Test: unit test pass
Change-Id: I3ed93c857fd02f7b62f5baba9130f75c3236e717
2018-02-12 15:19:37 -08:00
Tao Bao
47e5a8d085 applypatch: Remove the 'st' field from FileContents.
It used to keep track of the stat(2) info (e.g. st_mode/st_gid/st_uid)
while patching a file in file-based OTA.

Test: Build and use the new updater to apply an update on bullhead.
Change-Id: Ibf8f0f4b14298a9489bf24a2678bb279c5d9c8f3
2017-12-07 10:01:57 -08:00
Tianjie Xu
c7069cc922 Merge "Add libbrotli as a dependency when building applypatch binary" 2017-11-17 16:09:08 +00: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
Tao Bao
de07371b03 Merge "applypatch: Change the patch parameter to const Value& in Apply{BSDiff,Image}Patch." 2017-11-11 01:09:47 +00:00
Tao Bao
1e0941f4f6 applypatch: Change the patch parameter to const Value& in Apply{BSDiff,Image}Patch.
It used to be "const Value*", but nullptr won't be a valid input.

Test: recovery_host_test; recovery_component_test
Change-Id: I904b5689ac3e64504088bf0544c9fb5d45a52243
2017-11-10 12:18:34 -08:00
Alex Deymo
3afe5f5691 Include bspatch.h from bsdiff/
The function ApplyBSDiffPatch() defined in bspatch.cpp is declared in
applypatch.h, but it includes "bspatch.h" from the bsdiff/ project,
which is at least confusing. There is no "bspatch.h" in this repo, so
the include actually reffers to the one in bsdiff. This patch uses the
"bsdiff/bspatch.h" form instead to avoid confusion.

Bug: None
Test: It builds.

Change-Id: I6b6ffd6725b2b34ff644aed93683f69779103661
2017-11-08 12:29:58 +01:00
Tianjie Xu
3d843868ae Add libbrotli as a dependency when building applypatch binary
libbrotli is now used in libbspatch.

Bug: 34220646
Test: mma
Change-Id: I8d3527141539ffdae112d9042355fd705caeb250
2017-11-03 15:52:22 -07:00
Tianjie Xu
a88cc5440e Switch to bionic gtest in bootable/recovery
We encountered segfaults in Imgdiff host tests due to the failure to
reset states of getopt. The problem can be solved by switching to use
bionic's gtest where a new process is forked for each test.

Also modify the recovery_component_test to make sure it runs in parallel.
Changes include:
  1. Merge the writes to misc partition into one single test.
  2. Change the hard coded location "/cache/saved.file" into a configurable
  variable.

Bug: 67849209
Test: recovery tests pass

Change-Id: I165d313f32b83393fb7922c5078636ac40b50bc2
2017-11-03 14:09:11 -07:00
Tianjie Xu
8c28081039 Add libbrotli as a dependency for libbsdiff
Bug: 34220646
Test: mma
Change-Id: If00285943fff8226f1bc7239db5570a277739904
2017-11-01 15:30:05 -07:00
Treehugger Robot
1bc0117755 Merge "applypatch: Fix a memory leak in ApplyImagePatch()." 2017-10-24 21:59:26 +00:00
Tao Bao
fdec103335 applypatch: Fix a memory leak in ApplyImagePatch().
$ valgrind --leak-check=full out/host/linux-x86/nativetest64/recovery_host_test/recovery_host_test

==36755== 112 bytes in 1 blocks are definitely lost in loss record 4 of 16
==36755==    at 0x40307C4: malloc (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
==36755==    by 0x40C1669: operator new(unsigned long) (external/libcxxabi/src/cxa_new_delete.cpp:46)
==36755==    by 0x18D6A8: ApplyImagePatch(unsigned char const*, unsigned long, Value const*, std::__1::function<unsigned long (unsigned char const*, unsigned long)>, sha_state_st*, Value const*) (bootable/recovery/applypatch/imgpatch.cpp:62)
==36755==    by 0x18D02B: ApplyImagePatch(unsigned char const*, unsigned long, unsigned char const*, unsigned long, std::__1::function<unsigned long (unsigned char const*, unsigned long)>) (bootable/recovery/applypatch/imgpatch.cpp:134)
==36755==    by 0x160D15: GenerateTarget(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) (bootable/recovery/tests/component/imgdiff_test.cpp:85)
==36755==    by 0x11FA7D: verify_patched_image(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) (bootable/recovery/tests/component/imgdiff_test.cpp:96)
==36755==    by 0x12966C: ImgdiffTest_zip_mode_smoke_trailer_zeros_Test::TestBody() (bootable/recovery/tests/component/imgdiff_test.cpp:295)
==36755==    by 0x235EF9: testing::Test::Run() (external/googletest/googletest/src/gtest.cc:2455)
==36755==    by 0x236CBF: testing::TestInfo::Run() (external/googletest/googletest/src/gtest.cc:2653)
==36755==    by 0x2372D6: testing::TestCase::Run() (external/googletest/googletest/src/gtest.cc:2771)
==36755==    by 0x23EEE6: testing::internal::UnitTestImpl::RunAllTests() (external/googletest/googletest/src/gtest.cc:4648)
==36755==    by 0x23EB45: testing::UnitTest::Run() (external/googletest/googletest/src/gtest.cc:2455)

std::unique_ptr<z_stream, decltype(&deflateEnd)> strm(new z_stream(), deflateEnd);

Only the internally allocated buffers inside 'strm' would be free'd by
deflateEnd(), but not 'strm' itself.

This CL fixes the issue by moving 'strm' to stack variable. Note that we
only need to call deflateEnd() on successful return of deflateInit2().

Test: recovery_host_test && recovery_component_test
Change-Id: I39b9bdf62376b8029f95cab82c8542bfcb874009
2017-10-24 12:48:36 -07: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
b058c5c481 Merge "applypatch: Move to Soong." 2017-10-12 16:12:48 +00:00
Tao Bao
582b67850f applypatch: Move to Soong.
Test: mmma bootable/recovery
Change-Id: Ie163aff1c4c2b3b15bb705825779ada6bc38ad67
2017-10-11 16:26:06 -07:00
Tao Bao
526699fc67 applypatch: Remove the Makefile for building imgdiff in Chrome OS.
The former user system/update_engine has dropped the support for
imgdiff (https://android-review.googlesource.com/c/platform/system/update_engine/+/400427/).

Test: N/A
Change-Id: Ia8f770dc51800b45064c4affc3514cf513877edc
2017-10-11 15:59:28 -07:00
Tao Bao
44fc7b1e79 applypatch: Use shared lib for libbz.
We have shared lib target for libbz now (and libbz.so is already on
device because of /system/bin/bzip2).

Test: m applypatch
Change-Id: I5fe2468a8d535840245f081a92d436240dddbf6b
2017-10-11 11:35:08 -07:00
Tao Bao
46d8a3b604 Merge "applypatch: Use shared libs for libbase/libcrypto/liblog/libziparchive." 2017-10-11 18:31:36 +00: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
Tao Bao
3db3205b2a applypatch: Use shared libs for libbase/libcrypto/liblog/libziparchive.
This reduces the size of /system/bin/applypatch by ~69KB
(aosp_bullhead-userdebug).

Also remove the unneeded libcutils dependency.

Test: mmma bootable/recovery
Test: Check that /system/bin/install-recovery.sh successfully installs
      the recovery image.
Change-Id: I5063be9a9b7b8029d45ab5c2a7c45ef2cda81d26
2017-10-10 15:34:49 -07:00
Tao Bao
e6f7f95d34 edify: Export the header and move to Soong.
Also make matching changes to applypatch modules which include
edify/expr.h.

Test: mmma bootable/recovery
Change-Id: Ia72be3caa010d7f56a70add2da345e631b306378
2017-10-09 14:08:00 -07:00
Tao Bao
1fc5bf353a Revert "Revert "Move error_code.h into otautil.""
This reverts commit 26436d6d60 to re-land
"Move error_code.h into otautil.".

This way it stops requiring relative path ".." in LOCAL_C_INCLUDES
(uncrypt and edify). Soong doesn't accept non-local ".." in
"local_include_dirs".

This CL needs to land with device-specific module changes (e.g. adding
the dependency on libotautil).

Test: lunch aosp_{angler,bullhead,dragon,fugu,sailfish}-userdebug;
      mmma bootable/recovery
Change-Id: If193241801af2dae73eccd31ce57cd2b81c9fd96
2017-10-09 14:07:54 -07:00
Tao Bao
38d78d19b9 applypatch: Forward declare struct Value.
And move '#include "edify/expr.h"' into .cpp files. This breaks the
transitive dependency on libedify. Modules that include
"applypatch/applypatch.h" don't need to add libedify into their
dependency list, unless they really need anything from libedify.

Build libedify static library for host, which is needed by
libimgpatch.

Test: mmma bootable/recovery
Change-Id: Ibb53d322579fcbf593438d058d9bcee240625941
2017-10-09 11:50:09 -07:00
Tao Bao
26436d6d60 Revert "Move error_code.h into otautil."
This reverts commit 623fe7e701.

Reason for revert: Need to address device-specific modules.

Change-Id: Ib7a4191e7f193dfff49b02d3de76dda856800251
2017-10-05 17:16:31 +00:00
Tao Bao
623fe7e701 Move error_code.h into otautil.
This way it stops requiring relative path ".." in LOCAL_C_INCLUDES
(uncrypt and edify). Soong doesn't accept non-local ".." in
"local_include_dirs".

Test: mmma bootable/recovery
Change-Id: Ia4649789cef2aaeb2785483660e9ea5a8b389c62
2017-10-04 08:55:24 -07:00
Tao Bao
d33b2f86b7 otafault: Move headers under otafault/.
Test: mmma bootable/recovery
Change-Id: I3ceb72f703c7c2857d656c137d71baa1fccd8238
2017-09-29 10:29:53 -07:00
Tao Bao
646b05a66c otafault: Remove the use of LOCAL_WHOLE_STATIC_LIBRARIES.
Commit d80a99883d has explanation of
potential issues.

Test: mmma bootable/recovery
Change-Id: I25ca9920952b7bbdd8a661d9dc90962431410bc4
2017-09-28 18:25:10 -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
c89d1e7e2a Turn on -Wall for recovery modules
Turn on -Wall for all modules. Also remove the obsolete file_cmp() in
apply_patch test and now() in wear_ui.

The only exception is lib_edify due to the unused functions in the
intermediate cpp files generated from the lex files. It will be handled
in a seperate CL.

Bug: 64939312
Test: mma, unit tests pass
Change-Id: Ic53f76b60b6401ab20db3d98130d674c08e3702f
2017-08-28 21:56:33 -07:00