| Age | Commit message (Collapse) | Author |
|---|
|
Old behavior:
Read partition, for each block:
Update hasher
Update verity writer
before reading hashtree/verity:
write hashtree/verity to disk
Read the last verity blocks.
Finalize hasher, verity hashes.
The old bahvior tries to minimize fs read by only read once and feed
data to hasher and verity writer. However, in VABC, reading/writing are
handled very differently. Read can be done via regular fd, but writes
must go through special COW API. As we have seen in b/186196758, using
COW API in filesystem hashing can lead to inconsistent read and boot
failure. Therefore, we've decided to write verity first using COW API,
then read/hash partition using regular fd. This does mean that we need
to read everything twice, but we think this is a worth while tradeoff.
As verity writes can take 5 minutes, but reading the entire partition
again only takes <10 seconds.
New behavior:
Read partition, for each block:
Update verity writer
Finalize verity writer, write verity to disk
launch snapuserd, open a regular fd.
Read partition, for each block:
Update hasher
Finaliaze hasher, verity hashes.
Test: th
Test: Manual testing on pixel of the following scenario:
1. Verity enabled, VABC enabled, pause/resume multiple times
2. Verity disabled, VABC enabled, pause/resume multiple times
3. Verity Enabled, VABC enabled, pause/resume multiple times
Bug: 186196758
Change-Id: I2477c2dc4da5b921e84b48a54d0d8a877c1a52ef
|
|
b/186196758 could be detected by keep running fs verification. The 2nd
attempt will cause verity data to be discarded, but since data is still
visible during the 2nd attempt, bug is only visible after device reboot,
or running fs verification for the 3rd time.
Bug: 186196758
Test: th
Change-Id: I7415665fa030b68acc3903499750702d8df5626e
|
|
Test: th
Change-Id: I7db1874cffdacf93bbee8243dc45bb1bcc8b04ee
|
|
b/186196758 is triggered by the following sequence of events:
1. update_engine finish writing all install ops, emits kEndOfInstall
label
2. update_engine opens cow in append mode, invokes
InitialiazeAppend(kEndOfInstall)
3. update_engine writes verity data, invokes SnapshotWriter::Finalize()
4. update_engine repeats step 2, but does not write any data after
opening SnapshotWriter. Instead, it reads verity and make sure the hash
matches what's specified in OTA payload.
5. Reboot device, verity data corrupted, device rollback to slot _a.
This is because, during step 4, when calling
InitializeAppend(kEndOfInstall), the SnapshotWriter only reads up to the
given label. But OpenReader() completely disregards the resume label and
reads all ops. Therefore, update_engine sees the verity data, and
determines that everything is fine. However, when calling
SnapshotWriter::Finalize(), data after resume label are discarded,
therefore verity data is gone.
Test: th
Bug: 186196758
Change-Id: I0166271b64eb7b574434d617ce730f345ca93ff1
|
|
If update_engine opens CowWriterFileDescriptor w/o writing anything,
data past the resume label is readable while fd is open, but will
be discarded once the fd is closed. Such "phantom read" causes
inconsistency. This CL contains two changes to address the above bug:
1. When device reboots after update, all I/O are served by snapuserd.
update_engine should use snapuserd for verification to emulate bahvior
of device after reboot.
2. When a CowWriterFd is opened, don't call Finalize() if no verity is
written. Since past-the-end data is discarded when we call Finalize()
Test: th
Bug: 186196758
Change-Id: Ia1d31b671c16fded7319677fe0397f1288457201
|
|
Test: th
Change-Id: I4e461b03d4008d484eafe601d3de2f4b06bf585d
|
|
Test: generate an OTA with verity enabled, install it
Bug: 173432386
Change-Id: I14cccb1bb339c9824a95b8e42ac5144cb8b75c3f
|
|
We can return a FileDescriptor object, which encapsulates logic needed
to write to a COW. This way, filesystem verfication action can use the
turend value directly for computing verity and hash the partition.
Test: th
Change-Id: Iafe9699ef0cc15961641fc94f8ad2820230a56e1
|
|
Verity that we read everything in hashtree_data_extent before writing
hash tree.
Bug: 173432386
Test: th
Change-Id: I00ab8053de71b13991adaa243b6cb6c7efd6e60f
|
|
When DynamicPartitionControlAndroid is constructed, it initializes both
source and target slot to -1. These values get updated during
PreparePartitionsForUpdate call. And we only
PreparePartitionsForUpdate() when applying an OTA or applocating space
for an OTA(not when verifying OTA metadata). Which means if
VerifyPayloadApplicable() is called before any call two other APIs, we
could be using an "Uninitialiazed" dynamic partition control.
To mitigate this problem, we pass in source_slot at ctor of
DynamicPartitionControl, also make IsDynamicPartition() api take in a
slot number to avoid reading uninitialized member fields.
Bug: 181643302
Test: apply an OTA, abort, restart update_engine, verify a payload
Change-Id: I9a8a0fe8a9aca48e91241e15bdec33a1c1228553
|
|
Test: th
Change-Id: Ia04f92d46da34fcd28d7e97c24b6e02fd676e1c7
|
|
If a slot doesn't support dynamic partitions, it's impossible to list
dynamic partitions on that slot. And we should just fall back to the
regular A/B in this case.
Bug: 180025432
Test: apply a retrofit package
Change-Id: I16c457b591e8c1d0cf1077a7be50dd9d8f61b8eb
|
|
Test: treehuggre
Change-Id: I03f69b7add96eaa481b1152a1425f4cb669d1113
|
|
During FileSystemVerify stage, update_engine needs to read from source
or target partition to verify hashes && write verity. Previously we use
brillow's file stream to implement async reads. WIth Virtual AB
Compression, reading from target partition must go through libsnapshot's
interface(FileDescriptorPtr). So we replace brillo::FileStream with
FileDescriptorPtr for ease of integrating with VABC.
Test: serve an OTA update, verify: slot switch resume, regular resume
Change-Id: Id8531757468f60e3e21667b7761b83f7c2af2dbf
|
|
Some of the unit tests have been leaking temp files because they don't
properly unlink them. In this CL, we did some rearrangement of the
ScopedTempFile class and moved it into the utils.h (instead of testing
only location) so it can be used everywhere and more efficiently. Also
added functionality to open an file descriptor too so users don't have
to keep a different object for the file descriptor.
BUG=b:162766400
TEST=cros_workon_make --board reef --test; Then looked at the
/build/reef/tmp directory and no files were leaked.
Change-Id: Id64a2923d30f27628120497fdefe16bf65fa3fb0
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2500772
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
|
|
For partial updates, the metadata for untouched dynamic partitions
are just copied over to the target slot. So, verifying the extents
of these partitions in the target metadata should be sufficient for
correctness. This saves the work to read & hash the bytes on these
partitions for each resumed update.
Bug: 151088567
Test: unit tests pass, apply a partial update
Change-Id: I9d40ed2643e145a1546ea17b146fcdcfb91f213f
|
|
We just did a AOSP merge, so it is a good time to clean things up.
BUG=none
TEST=unittest
Change-Id: I4fe9cef5eb8709344d6b78bc298c0f1c03308ffc
Reviewed-on: https://chromium-review.googlesource.com/1407540
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>
|
|
Since libchrome in AOSP is ahead of CrOS I had to guard against BASE_VER in a
few places to satisfy older libchromes.
file_fetcher.cc is now needed in delta_generator.
A few unittests need to be run as root.
BUG=chromium:916593
TEST=unittest
TEST=cros_generate_update_payload
TEST=cros flash
CQ-DEPEND=CL:1399261
Change-Id: If3497549e88e559f8ecc38f414259b9c774f4a44
|
|
This reverts commit 71818c8409812c5a08124627c19aa8ea0625a72e.
This patch was created because the upstream update_engine was using
the new version of libchrome and the Chrome OS one was not. Now
that we are upreving libchrome on Chrome OS we can revert this.
BUG=b:37434548
TEST=Build
CQ-DEPEND=CL:1240033
Change-Id: I98b7d124212087292500701782de08b3d3ecc559
Reviewed-on: https://chromium-review.googlesource.com/1239818
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
|
|
Computing FEC on device could take up to 3 minutes depending on
partition size, we should skip it if it's already written.
This is similar to how we skip postinstall for postponed OTA, but we
don't require passing additional header here because we can verify the
correctness of the verity data within update_engine itself.
Bug: 28171891
Test: update_engine_unittests
Change-Id: Ie9883e2260d95c05aec169dd1fde12beea0bdade
|
|
After hash tree is written, we re-read the partition to encode FEC,
this cannot be done incrementally for each Update() like the hash
tree, because the data needed to encode each rs block are spreaded
across entire partition, and we can not afford to use that much
memory.
For each round, we encode block_size number of rs blocks, which will
produce block_size * fec_roots bytes of FEC data, this will allow us
to read one block at a time instead of one byte.
Bug: 28171891
Test: update_engine_unittests
Test: brillo_update_payload generate
Test: brillo_update_payload verify
Change-Id: I35ba7e0647b9ee5a97b972dc480deef60d813676
|
|
Calculate verity hash tree in FilesystemVerifierAction based on configs
specified in protobuf, and write it to target partition before reading
from those blocks.
A new error code kVerityCalculationError was added to report error if
it fails.
Bug: 28171891
Test: update_engine_unittests
Change-Id: I492885a0655bf51043902f578720fffd87e6a3cf
|
|
The following change is reverted because aosp has newer libchrome.
71818c84 Partially Revert 2b9d241
Added stub override for ReportInternalErrorCode().
Fixed RunPosinstallAction typo.
Bug: 112326236
Test: update_engine_unittests
Change-Id: Ieaae0eef425cbb1278067a48aa19b14ed056317a
|
|
in CL:1065113 we forgot to remove the FileSystemVerfierAction instance from the
FilesystemVerifierActionDelegate. Hence, it will be removed from memory while
FilesystemVerifierActionDelegate still keeps a pointer to it and access it. This
causes a possible race condition on ExitMainLoop() function. This patch fixes
the issue by removing that instance and the unnecessary checks for the
cleanup. Once the Action is deleted by the ActionProcessor it will be cleaned up
(which will happen when the action is terminated too). So there is not really a
need for testing the cleanup method.
BUG=chromium:867815
TEST=unittests pass
Change-Id: Id9d3f361eb916007e95dbb1d0adb9603ceae00b9
Reviewed-on: https://chromium-review.googlesource.com/1151902
Commit-Ready: Lann Martin <lannm@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
|
|
Currently, an object that uses an ActionProcessor for processing one or more
actions has to own the Actions. This is problematic, because if we want to
create an action on the fly and use an ActionProcessor to perform it, we have to
own the Action until it is finished. Furthermore, if someone forget to own the
action, there will be memory leaks because ActionProcessor does not delete the
Action.
This patch passes the ownership of the Actions to the ActionProcessor through
unique pointers. If an object wants to have access to the Action, it can get it
when ActionComplete() is called.
BUG=chromium:807976
TEST=unittests
TEST=cros flash
TEST=precq
Change-Id: I28f7e9fd3425f17cc51b4db4a4abc130a7d6ef8f
Reviewed-on: https://chromium-review.googlesource.com/1065113
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>
|
|
Replace these 3 lines of code:
string path;
ASSERT_TRUE(utils::MakeTempFile("name-XXXXXX", &path, nullptr));
ScopedPathUnlinker path_unlinker(path);
with one liner:
test_utils::ScopedTempFile file("name-XXXXXX");
Bug: None
Test: unit test
Change-Id: Ic5be7dc8339842270023055bcc3a97e526953f04
|
|
2b9d2417722cd4052b0e22494886f93c5b4ef042 update_engine: Update libchrome APIS to r456626.
The above commit changes the libchrome API used in update_engine to r456626. But
the libchrome has not been upreved fully in the CrOS yet with the exception of
some changes represented in UE like CL:882543. So, now we need to revert the
changes partially untill the libchrome is updated.
BUG=chromium:815356
TEST=unittests, precq
Change-Id: If2207f0672c7b9f6dab84e676d9fb8423a047372
Reviewed-on: https://chromium-review.googlesource.com/965266
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
|
|
The new libchrome has been ported from Chromium and some APIs
have changed. Make necessary changes at call sites.
BUG=chromium:724678
CQ-DEPEND=CL:480928
Test: Build.
Change-Id: I01b70da87521d0884ed21acbd7ed3e0ff1e94357
Merged-In: I4dbaea4a2a19031375a8bf2415645a4f226dab57
|
|
update_engine now only runs FilesystemVerifierAction after DownloadAction,
the partition paths are already set in InstallPlan, so we no longer need
BootControlInterface to get partition paths in FilesystemVerifierAction.
Test: ./update_engine_unittests
Test: applied an update in edison
Bug: 26972259
Change-Id: I9d439688a21e4e42be88a4c5accf731ce64d2d6f
|
|
This mode was used to calculate the source partition hash before download
the payload, and we will verify it against the hash in the payload.
Now that we are using per-operation source hash, this mode is no longer
needed.
Test: ./update_engine_unittests
Test: cros_workon_make update_engine --test
Bug: 26972259
Change-Id: Ie30a38cfd9f94e4efe02dfc8664e6785018261f6
|
|
Now that we switched to minor version 3, those code is dead.
The disabled unittest is using CreateExtImageAtPath which I'm about to
remove, so I'm removing this code now.
Test: mma
Bug: 26972259
Change-Id: Ic6a9b9e87e5e2aa09011cf005a4dee78391c6901
|
|
Postinstall unittests were creating and mounting an image on each test
run. This patch adds several test scripts to one of the pre-generated
images and uses that image during postinstall testing instead.
To workaround problems with mount/umount of loop devices on Android,
this patch rewrites the `losetup` logic to make the appropriate
syscalls and create the loop device with mknod if it doesn't exists.
The tests require some extra SELinux policies to run in enforcing mode.
Bug: 26955860
TEST=Ran all Postinstall unittests.
Change-Id: I47a56b80b97596bc65ffe30cbc8118f05faff0ae
|
|
Many unittests do not pass for simple reasons that will be addressed
later. This CL includes all the changes to make the unittests build.
In particular, the generated DBus mocks, required to build several
unittests are now included here.
The dbus-constants.h files were moved to the system_api repo, so they
can be removed from here.
The unittest build is only enabled for Brillo targets, since non-Brillo
targets don't even build DBus.
Bug: 26955860
TEST=`mmma` on edison-eng (and aosp_arm-eng).
Change-Id: Ib38241f0a6eb99b1d60d72db6bcfd125d38e3fad
|
|
The "is_full_update" flag in the InstallPlan is required to decide
whether we should run a FilesystemVerification step before start
downloading the payload (for delta payloads) or not (for full payloads).
This step is done before start downloading the payload and not after
downloading the metadata to avoid long delays in the connection which
would then drop and require a retry.
Since the not so recent inclusion of the source_data_hash field in the
delta operations, the source data is verified on each operation, so the
install plan field and the pre-download FilesystemVerification is not
needed anymore.
To help deprecate this process, which is not included in the non-Brillo
version, this patch changes the is_full_update field to a payload_state
enum with a third "unknown" state that will be changed to delta or full
once the payload metadata is parsed.
Bug: 25631949
TEST=unittests updated.
TEST=Pushed a delta update to edison-eng and a non-Brillo target.
Change-Id: I17d8bf58990d8465bb8487adc66601f1c1dfca6d
|
|
Bug: 23182225
TEST=cros_workon_make update_engine --test
Change-Id: I9e440fa4adc79e8a0b6426b9b7aefd0d4209699a
|
|
In minor version 3, we use per-operation source hash to verify the source
instead of whole filesystem hash, but if target partition doesn't match,
we still need to check the source partition to see if it is the cause.
Bug: 23182225
TEST=Applied a minor version 3 delta payload.
TEST=cros_workon_make update_engine --test
Change-Id: I1f32c292d2938540b63f086cf4988d64620702a5
|
|
This patch splits from the main libupdate_engine code the part that
is strictly used to download and apply a payload into a new static
library, moving the code to subdirectories. The new library is divided
in two subdirectories: common/ and payload_consumer/, and should not
depend on other update_engine files outside those two subdirectories.
The main difference between those two is that the common/ tools are more
generic and not tied to the payload consumer process, but otherwise they
are both compiled together.
There are still dependencies from the new libpayload_consumer library
into the main directory files and DBus generated files. Those will be
addressed in follow up CLs.
Bug: 25197634
Test: FEATURES=test emerge-link update_engine; `mm` on Brillo.
Change-Id: Id8d0204ea573627e6e26ca9ea17b9592ca95bc23
|
