search

anestisb / android-prepare-vendor

Set of scripts to automate AOSP compatible vendor blobs generation from factory images
347 stars 156 forks source link

Output not deterministic for crosshatch: #142

Closed lrvick closed 5 years ago

lrvick commented 5 years ago

So comparing 2 output directories for crosshatch with diffoscope we get:

--- out1/crosshatch/pq1a.181205.006/vendor/google_devices/crosshatch
+++ out2/crosshatch/pq1a.181205.006/vendor/google_devices/crosshatch
├── file_signatures.txt
│ @@ -18,23 +18,23 @@
│  5e1b492874c221a7e1188fc3a5369d32b3e67969  vendor/google_devices/crosshatch/proprietary/etc/permissions/embms.xml
│  5aed30f29c7fbe91dcf7eef9dddbdf96ac6bcbee  vendor/google_devices/crosshatch/proprietary/etc/permissions/lpa.xml
│  0454d4d27fb5bbe87c1d6ebd4e9432508de76c9b  vendor/google_devices/crosshatch/proprietary/etc/permissions/LteDirectDiscovery.xml
│  9fc58a3d54d20ac1d53bab14608815121aefbce5  vendor/google_devices/crosshatch/proprietary/etc/permissions/qcrilhook.xml
│  3d95597dc6db9a071bfb06d55d1a0cdf309a2854  vendor/google_devices/crosshatch/proprietary/etc/permissions/RemoteSimlock.xml
│  b297c8a27443570377d78586b00198839ce326bb  vendor/google_devices/crosshatch/proprietary/etc/permissions/telephonyservice.xml
│  a2bb4abefd281692e6af2c1c5785c09a5ab89703  vendor/google_devices/crosshatch/proprietary/etc/permissions/UimService.xml
│ -4e437a4e0ddcdcbe37dd5f3621c7af57f76064f1  vendor/google_devices/crosshatch/proprietary/framework/com.qualcomm.qti.uceservice-V2.0-java.jar
│ +e4b3aa9415cf949ba7478385f111febf9f42c2aa  vendor/google_devices/crosshatch/proprietary/framework/com.qualcomm.qti.uceservice-V2.0-java.jar
│  180d4d6b65d989023364457eb1bf6e1d45f7e274  vendor/google_devices/crosshatch/proprietary/framework/embmslibrary.jar
│ -4d43b859b520f750364539f84fcba8db54d0eefc  vendor/google_devices/crosshatch/proprietary/framework/qcrilhook.jar
│ -a190c39bcc0c0af6ee41fa95395357884478b575  vendor/google_devices/crosshatch/proprietary/framework/QtiTelephonyServicelibrary.jar
│ -bb938b38da8286ad0faa6364c1c1fe4a00b16621  vendor/google_devices/crosshatch/proprietary/framework/uimlpalibrary.jar
│ -e74042e504122d2ded3a0fbcc7792ded6abbe2b0  vendor/google_devices/crosshatch/proprietary/framework/uimremotesimlocklibrary.jar
│ -7671e6429d92d9e4f70de05fae1836c2e7a6261f  vendor/google_devices/crosshatch/proprietary/framework/vendor.qti.hardware.alarm-V1.0-java.jar
│ -484d6c6d52217922c461e181a90e95dcf43fc8c8  vendor/google_devices/crosshatch/proprietary/framework/vendor.qti.hardware.data.latency-V1.0-java.jar
│ -4084d8127ada7cff2f8065f6201247ddaa51fe21  vendor/google_devices/crosshatch/proprietary/framework/vendor.qti.hardware.soter-V1.0-java.jar
│ +56294b43cc57ee63494d39c81751b64f5e2984f5  vendor/google_devices/crosshatch/proprietary/framework/qcrilhook.jar
│ +3e068a11f96d4165102337702ef13f652487c5e9  vendor/google_devices/crosshatch/proprietary/framework/QtiTelephonyServicelibrary.jar
│ +d1a277aeeb7aed24988b7f15f0cb27945903fd64  vendor/google_devices/crosshatch/proprietary/framework/uimlpalibrary.jar
│ +bfe9c4d52a15e86bd8c5dccbd8e25a24ce6bd0f6  vendor/google_devices/crosshatch/proprietary/framework/uimremotesimlocklibrary.jar
│ +6973d23fc5e71aed15ab8946623bdc982954c801  vendor/google_devices/crosshatch/proprietary/framework/vendor.qti.hardware.alarm-V1.0-java.jar
│ +6d11e76ed07055d10d341e803722a63f4a9356cd  vendor/google_devices/crosshatch/proprietary/framework/vendor.qti.hardware.data.latency-V1.0-java.jar
│ +6c7d6060b5da240d43a7375c1adf68fd4741f45d  vendor/google_devices/crosshatch/proprietary/framework/vendor.qti.hardware.soter-V1.0-java.jar
│  e8bafa7ba87211891ad43aa34ae82977b975e14c  vendor/google_devices/crosshatch/proprietary/lib64/libadsprpc_system.so
│  2c6181a5dec26e3b9cf40db28e572da0f6c44206  vendor/google_devices/crosshatch/proprietary/lib64/libcdsprpc_system.so
│  27f124237363f705cf5aa101f4030dfdcdb00842  vendor/google_devices/crosshatch/proprietary/lib64/libdiag_system.so
│  13659d690c39441a6b20a41cea0f72bdbb79007b  vendor/google_devices/crosshatch/proprietary/lib64/libGPQTEEC_system.so
│  7c59864d839499414fff7b6a62475cc81843d5cc  vendor/google_devices/crosshatch/proprietary/lib64/libGPTEE_system.so
│  42a3449245e77e32222e31fb12bcc119e5bcb00a  vendor/google_devices/crosshatch/proprietary/lib64/libimscamera_jni.so
│  c38a168d7c576456d9a3b3270303c352c92a1b9e  vendor/google_devices/crosshatch/proprietary/lib64/libimsmedia_jni.so
├── proprietary
│ ├── framework
│ │ ├── QtiTelephonyServicelibrary.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 40398 bytes, number of entries: 2
│ │ │ │  -rw----     2.0 fat       61 bX defN 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat    97160 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat    97160 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 97221 bytes uncompressed, 40122 bytes compressed:  58.7%
│ │ ├── com.qualcomm.qti.uceservice-V2.0-java.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 34772 bytes, number of entries: 2
│ │ │ │  -rw----     1.0 fat       45 bx stor 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat   108208 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat   108208 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 108253 bytes uncompressed, 34512 bytes compressed:  68.1%
│ │ ├── qcrilhook.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 63138 bytes, number of entries: 2
│ │ │ │  -rw----     2.0 fat       61 bX defN 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat   174624 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat   174624 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 174685 bytes uncompressed, 62862 bytes compressed:  64.0%
│ │ ├── uimlpalibrary.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 7595 bytes, number of entries: 2
│ │ │ │  -rw----     2.0 fat       61 bX defN 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat    20752 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat    20752 bl defN 18-Dec-23 20:57 classes.dex│ │ │ │  2 files, 20813 bytes uncompressed, 7319 bytes compressed:  64.8%
│ │ ├── uimremotesimlocklibrary.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 3940 bytes, number of entries: 2
│ │ │ │  -rw----     2.0 fat       61 bX defN 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat    10544 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat    10544 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 10605 bytes uncompressed, 3664 bytes compressed:  65.5%
│ │ ├── vendor.qti.hardware.alarm-V1.0-java.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 14508 bytes, number of entries: 2
│ │ │ │  -rw----     1.0 fat       45 bx stor 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat    34568 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat    34568 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 34613 bytes uncompressed, 14248 bytes compressed:  58.8%
│ │ ├── vendor.qti.hardware.data.latency-V1.0-java.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 17969 bytes, number of entries: 2
│ │ │ │  -rw----     1.0 fat       45 bx stor 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat    48048 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat    48048 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 48093 bytes uncompressed, 17709 bytes compressed:  63.2%
│ │ ├── vendor.qti.hardware.soter-V1.0-java.jar
│ │ │ ├── zipinfo /dev/stdin
│ │ │ │ @@ -1,4 +1,4 @@
│ │ │ │  Zip file size: 18255 bytes, number of entries: 2
│ │ │ │  -rw----     1.0 fat       45 bx stor 08-Jan-01 00:00 META-INF/MANIFEST.MF
│ │ │ │ --rw----     2.0 fat    45728 bl defN 18-Dec-23 21:04 classes.dex
│ │ │ │ +-rw----     2.0 fat    45728 bl defN 18-Dec-23 20:57 classes.dex
│ │ │ │  2 files, 45773 bytes uncompressed, 17995 bytes compressed:  60.7%

If we look at the generated vendor files we find that the symlink paths for toolbox are not consistent. These are defined in both Android.mk and proprietary/device-vendor.mk. Possible race condition?

--- target_files_1/VENDOR
+++ target_files_2/VENDOR
├── bin
│ ├── dd
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── egrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: grep
│ │ +destination: /vendor/bin/grep
│ ├── fgrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: grep
│ │ +destination: /vendor/bin/grep
│ ├── getevent
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── getprop
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── newfs_msdos
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox

The above seems to be the last thing standing in the way of my goal to reach a fully deterministic pie build that will be easy for others to reproduce and verify.

Even if you can only point me in the right direction for solving this it would be appreciated :)

anestisb commented 5 years ago

@lrvick thanks for reporting. Unfortunately I didn't secure any free time yet to properly look the Pixel 3 support (initial support was contributed by other folks). Hopefully, will catch up with the backlog these days.

So based on your output the diff in the framework Jar files is located in the Dex files. They appear to be of identical size, although they differ. Interesting, will investigate further.

For the toolbox symlink issue I'm not sure I understood the anomaly. Do you mean that when you run the scripts sometimes the links are generated against "/vendor/bin/toolbox" and some against "toolbox"? Or that some links are against the former and some against the latter?

lrvick commented 5 years ago

For the toolbox symlink issue I'm not sure I understood the anomaly. Do you mean that when you run the scripts sometimes the links are generated against "/vendor/bin/toolbox" and some against "toolbox"? Or that some links are against the former and some against the latter?

Yeah this diff is from two back to back builds with the only difference being they output to different folders. One of them gets the symlinks set one way, and the other gets them set the other.

In addition to breaking determinisim, I suspect the symlinks only work half the time too ^_^

anestisb commented 5 years ago

Ok that's a new one. Will investigate since I have no idea why this might happen.

Btw are you running on Linux or macOS?

lrvick commented 5 years ago

Building inside this container on Debian hosts: https://github.com/hashbang/os/blob/master/Dockerfile

anestisb commented 5 years ago

Regarding the Jar files hash mismatch, its due to the timestamp of the generated files.

--- /dev/fd/63  2018-12-24 10:43:47.712139520 +0200
+++ /dev/fd/62  2018-12-24 10:43:47.712139520 +0200
@@ -1,5 +1,5 @@
-Archive:  1.jar
+Archive:  2.jar
 Zip file size: 34772 bytes, number of entries: 2
 -rw----     1.0 fat       45 bx stor 08-Jan-01 00:00 META-INF/MANIFEST.MF
--rw----     2.0 fat   108208 bl defN 18-Dec-24 10:20 classes.dex
+-rw----     2.0 fat   108208 bl defN 18-Dec-24 10:25 classes.dex
 2 files, 108253 bytes uncompressed, 34512 bytes compressed:  68.1%

When the tool runs the entire workspace is cleaned. Therefore, the Dex files are re-extracted from factory image and thus obtain a new timestamp. Then when appended back to the Jar files the zip entries have a different timestamp, despite the Dex files being identical.

I can't think of a straight forward way to deal with this enhancement you've requested., However, since I like the idea of reproducible builds, one workaround that might work is to expose an additional argument with which someone can define a reference timestamp. Then this timestamp is used for all the new files generated by the tool. Unfortunately, this might break the rsync functions (depends on the usage) when someone sets the output directory in AOSP root. Therefore, I cannot enable it by default.

Any further recommendations are more than welcome.

Btw I wasn't able to reproduce the symlink issue yet.

lrvick commented 5 years ago

So the AOSP build system will normalize timestamps globally to either right now, or whatever the value of $BUILD_DATETIME (epoch) is, if it is set.

Anyone who is doing reproducible builds will have BUILD_DATETIME set, so I would suggest respecting that and builds should behave as expected.

anestisb commented 5 years ago

BUILD_DATETIME controls timestamps of AOSP generated files and will not make any difference in the case we're examining here. We're talking about timestamps of Zip entries inside a Zip archive. No matter how many timestamp changes you do in the Zip file, its entries will not get updated from AOSP (even if resigned).

To update the timestamp of the Zip entries it needs some manual work. Since I don't want to integrate any python (or similar) scripts, the only way I know of for this to be accomplished via bash, is to manually set the timestamp of a file and then update the corresponding zip entry. To do that though the script requires a reference timestamp. I cannot make it dynamic based on BUILD_DATETIME since the files will be already generated when AOSP kicks-in. Maybe having some shell automation inside the makefiles will make it possible, although I don't like this road since its fragile.

The best I can think of for your request is that you invoke the tool with a timestamp argument which should match the one you set in AOSP. I cannot handle it automatically since I want to maintain a portable solution. But from your side you can plug-in any changes or scripts you might need for your purposes.

lrvick commented 5 years ago

In my case my build system does have BUILD_DATETIME available when vendor is built, which happens before the rest of AOSP thus extract-all.sh will see that var.

I am happy to call extract-all.sh with a "--timestamp $BUILD_DATETIME" if that is how you prefer to implement it though. No biggie.

anestisb commented 5 years ago

@lrvick sounds good. On it.

anestisb commented 5 years ago

@lrvick can you give it a spin? I think it should cover your request.

$ ./execute-all.sh -d crosshatch -b pq1a.181205.006 -o $(pwd) -i crosshatch/pq1a.181205.006/crosshatch-pq1a.181205.006-factory-96b23504.zip --timestamp 1545897471
lrvick commented 5 years ago 
$ git log HEAD -n1

4cd75bb - U Anestis Bechtsoudis - (HEAD -> master, origin/master, origin/HEAD) Option to timestamp the generated bytecode files (15 hours ago)

$ ./execute-all.sh --debugfs --yes --device crosshatch --buildID pq1a.181205.006 --output out1 --timestamp 1543792453
...

$ ./execute-all.sh --debugfs --yes --device crosshatch --buildID pq1a.181205.006 --output out2 --timestamp 1543792453
...

$ diffoscope --exclude-directory-metadata out1/crosshatch/pq1a.181205.006/vendor/google_devices out2/crosshatch/pq1a.181205.006/vendor/google_devices

No diff!

For reasons I don't at all understand, this seems to have cleared up the symlink issues as well. Closing this :)

lrvick commented 5 years ago

Weird. So -actually- the output in the out directory when execute-all is done running things -are- identical.

When the vendor directory is ingested and placed into target_files dirs for release, the symlink issue manifests:

diffoscope target1/VENDOR target2/VENDOR
--- target1/VENDOR
+++ target2/VENDOR
├── bin
│ ├── dd
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── egrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/grep
│ │ +destination: grep
│ ├── fgrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/grep
│ │ +destination: grep
│ ├── getevent
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── getprop
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── newfs_msdos
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox

This is the same diff I find in final binaries as well.

I don't know Android well but I feel like the multiple references to these paths create some sort of race condition, or some issue that manifests only on the -second- build.

anestisb commented 5 years ago

@lrvick Ok now I understood what you mean regarding the symbolic links. AOSP is already generating the toybox & toolbox items. Therefore, when created by the scripts they are redundant and are overwritten by AOSP anyways. I'm not sure if the double definition is the root cause of the different output across builds. At any case, with commit https://github.com/anestisb/android-prepare-vendor/commit/94bde98d9ed1d974310ced411cf56c894195cdfe toybox & toolbox are not longer processed, so its left to AOSP to handle them. If you see different outputs that is an AOSP issue.

anestisb commented 5 years ago

Be sure to make clean builds before comparing two outputs.

lrvick commented 5 years ago

Okay 3 diff back to back builds, with three different symlink outcomes. This screams race condition to me, but I don't know enough internals to track it down just yet.

$ diffoscope target1/VENDOR target2/VENDOR
--- target1/VENDOR
+++ target2/VENDOR
├── bin
│ ├── dd
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── egrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/grep
│ │ +destination: grep
│ ├── fgrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/grep
│ │ +destination: grep
│ ├── getevent
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── getprop
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
│ ├── newfs_msdos
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/toolbox
│ │ +destination: toolbox
$ diffoscope target2/VENDOR target3/VENDOR
--- target2/VENDOR
+++ target3/VENDOR
├── bin
│ ├── dd
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: toolbox
│ │ +destination: /vendor/bin/toolbox
│ ├── getevent
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: toolbox
│ │ +destination: /vendor/bin/toolbox
│ ├── getprop
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: toolbox
│ │ +destination: /vendor/bin/toolbox
│ ├── newfs_msdos
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: toolbox
│ │ +destination: /vendor/bin/toolbox
$ diffoscope target1/VENDOR target3/VENDOR
--- target1/VENDOR
+++ target3/VENDOR
├── bin
│ ├── egrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/grep
│ │ +destination: grep
│ ├── fgrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: /vendor/bin/grep
│ │ +destination: grep
anestisb commented 5 years ago

Ok let me put a fresh set of builds to check if I can reproduce for crosshatch.

lrvick commented 5 years ago

Those builds did -not- include 94bde98. Didn't see that in time. Trying again with it.

lrvick commented 5 years ago

Did 2 builds with 94bde98 and now only the grep symlink issue is left:

--- target1/VENDOR
+++ target2/VENDOR
├── bin
│ ├── egrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: grep
│ │ +destination: /vendor/bin/grep
│ ├── fgrep
│ │┄ symlink
│ │ @@ -1 +1 @@
│ │ -destination: grep
│ │ +destination: /vendor/bin/grep
anestisb commented 5 years ago

@lrvick I think that you don't do a clean build for the second try. As a result the target module from AOSP is considered built, although the redundant vendor links generated by this tool are always executed thus overriding the previous entry. It's not a race condition. I've verified with 2 clean builds and the output is identical.

At any case, it was a good observation since I removed some components already available in AOSP. https://github.com/anestisb/android-prepare-vendor/commit/e085541ee77fcf35f6ba268bb2b506046b07388a should remove the grep bin (and the egrep/fgrep links) too.

lrvick commented 5 years ago

Clean build or not, a rebuild should always get the same output, not alter symlink paths. Three builds in a row with different grep symlink outcomes too before e085541.

In any event, as of e085541 I can no longer reproduce, so all good here. Thanks for chasing this!