Version 3.11.0

Signed-off-by: Andrew Gunnerson <accounts+github@chiller3.com>

.github/workflows/ci.yml

@@ -155,7 +155,7 @@ jobs:
         run: cp LICENSE README.md target/output/
 
       - name: Archive executable
-        uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
+        uses: actions/upload-artifact@6f51ac03b9356f520e9adb1b1b7802705f340c2b # v4.5.0
         with:
           name: avbroot-${{ steps.get_version.outputs.version }}-${{ matrix.artifact.name }}
           path: |

.github/workflows/release.yml

@@ -27,7 +27,7 @@ jobs:
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
 
       - name: Create release
-        uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974 # v2.1.0
+        uses: softprops/action-gh-release@7b4da11513bf3f43f9999e90eabced41ab8bb048 # v2.2.0
         with:
           tag_name: v${{ steps.get_version.outputs.version }}
           name: Version ${{ steps.get_version.outputs.version }}

CHANGELOG.md

@@ -7,6 +7,17 @@
     to update the actual links at the bottom of the file.
 -->
 
+### Version 3.11.0
+
+* Fix crash when ignoring warning about `--magisk-preinit-device` not being specified ([PR #394])
+* When using `--ignore-magisk-warnings`, assume that unsupported Magisk versions newer than the latest supported version are capable of all features ([Issue #393], [PR #395])
+* Update bzip2-rs and switch to the Rust backend ([PR #397], [PR #402])
+* Minor code cleanup for custom integer range type ([PR #398])
+* Improve errors to make them less ambiguous about what went wrong ([PR #401])
+* Fix bug where a vendor v4 boot image that was truncated in the bootconfig padding section would be accepted as valid ([PR #401])
+* Avoid performing many small I/O operations when reading and writing cpio archives ([PR #403])
+* Update dependencies ([PR #404])
+
 ### Version 3.10.0
 
 * Switch to using zerocopy library for all binary file format parsers ([PR #384])
@@ -284,6 +295,7 @@ Behind-the-scenes changes:
 [Issue #332]: https://github.com/chenxiaolong/avbroot/issues/332
 [Issue #356]: https://github.com/chenxiaolong/avbroot/issues/356
 [Issue #366]: https://github.com/chenxiaolong/avbroot/issues/366
+[Issue #393]: https://github.com/chenxiaolong/avbroot/issues/393
 [PR #130]: https://github.com/chenxiaolong/avbroot/pull/130
 [PR #132]: https://github.com/chenxiaolong/avbroot/pull/132
 [PR #133]: https://github.com/chenxiaolong/avbroot/pull/133
@@ -410,3 +422,11 @@ Behind-the-scenes changes:
 [PR #390]: https://github.com/chenxiaolong/avbroot/pull/390
 [PR #391]: https://github.com/chenxiaolong/avbroot/pull/391
 [PR #392]: https://github.com/chenxiaolong/avbroot/pull/392
+[PR #394]: https://github.com/chenxiaolong/avbroot/pull/394
+[PR #395]: https://github.com/chenxiaolong/avbroot/pull/395
+[PR #397]: https://github.com/chenxiaolong/avbroot/pull/397
+[PR #398]: https://github.com/chenxiaolong/avbroot/pull/398
+[PR #401]: https://github.com/chenxiaolong/avbroot/pull/401
+[PR #402]: https://github.com/chenxiaolong/avbroot/pull/402
+[PR #403]: https://github.com/chenxiaolong/avbroot/pull/403
+[PR #404]: https://github.com/chenxiaolong/avbroot/pull/404

Cargo.lock

@@ -85,9 +85,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.94"
+version = "1.0.95"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c1fd03a028ef38ba2276dce7e33fcd6369c158a1bca17946c4b1b701891c1ff7"
+checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04"
 
 [[package]]
 name = "arbitrary"
@@ -109,7 +109,7 @@ checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26"
 
 [[package]]
 name = "avbroot"
-version = "3.10.0"
+version = "3.11.0"
 dependencies = [
  "anyhow",
  "assert_matches",
@@ -153,14 +153,14 @@ dependencies = [
  "sha1",
  "sha2",
  "tempfile",
- "thiserror 2.0.6",
+ "thiserror 2.0.9",
  "toml_edit",
  "topological-sort",
  "tracing",
  "tracing-subscriber",
  "x509-cert",
- "zerocopy 0.8.13",
- "zerocopy-derive 0.8.13",
+ "zerocopy 0.8.14",
+ "zerocopy-derive 0.8.14",
  "zip",
 ]
 
@@ -211,9 +211,9 @@ dependencies = [
 
 [[package]]
 name = "bstr"
-version = "1.11.0"
+version = "1.11.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1a68f1f47cdf0ec8ee4b941b2eee2a80cb796db73118c0dd09ac63fbe405be22"
+checksum = "786a307d683a5bf92e6fd5fd69a7eb613751668d1d8d67d802846dfe367c62c8"
 dependencies = [
  "memchr",
  "regex-automata",
@@ -234,23 +234,13 @@ checksum = "325918d6fe32f23b19878fe4b34794ae41fc19ddbe53b10571a4874d44ffd39b"
 
 [[package]]
 name = "bzip2"
-version = "0.4.4"
-source = "git+https://github.com/chenxiaolong/bzip2-rs?rev=6e0f9836ec87b19261461b6cc1772e14aff8e851#6e0f9836ec87b19261461b6cc1772e14aff8e851"
+version = "0.5.0"
+source = "git+https://github.com/trifectatechfoundation/bzip2-rs?rev=15258feb0cdc1114d6fc6b936254c4f4e1d5730c#15258feb0cdc1114d6fc6b936254c4f4e1d5730c"
 dependencies = [
- "bzip2-sys",
+ "libbz2-rs-sys",
  "libc",
 ]
 
-[[package]]
-name = "bzip2-sys"
-version = "0.1.11+1.0.8"
-source = "git+https://github.com/chenxiaolong/bzip2-rs?rev=6e0f9836ec87b19261461b6cc1772e14aff8e851#6e0f9836ec87b19261461b6cc1772e14aff8e851"
-dependencies = [
- "cc",
- "libc",
- "pkg-config",
-]
-
 [[package]]
 name = "cap-primitives"
 version = "3.4.2"
@@ -303,9 +293,9 @@ dependencies = [
 
 [[package]]
 name = "cc"
-version = "1.2.3"
+version = "1.2.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "27f657647bcff5394bf56c7317665bbf790a137a50eaaa5c6bfbb9e27a518f2d"
+checksum = "c31a0499c1dc64f458ad13872de75c0eb7e3fdb0e67964610c914b034fc5956e"
 dependencies = [
  "jobserver",
  "libc",
@@ -358,9 +348,9 @@ dependencies = [
 
 [[package]]
 name = "clap_complete"
-version = "4.5.38"
+version = "4.5.40"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d9647a559c112175f17cf724dc72d3645680a883c58481332779192b0d8e7a01"
+checksum = "ac2e663e3e3bed2d32d065a8404024dad306e699a04263ec59919529f803aee9"
 dependencies = [
  "clap",
 ]
@@ -374,7 +364,7 @@ dependencies = [
  "heck",
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -433,9 +423,9 @@ dependencies = [
 
 [[package]]
 name = "crossbeam-deque"
-version = "0.8.5"
+version = "0.8.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "613f8cc01fe9cf1a3eb3d7f488fd2fa8388403e97039e2f73692932e291a770d"
+checksum = "9dd111b7b7f7d55b72c0a6ae361660ee5853c9af73f70c3c2ef6858b950e2e51"
 dependencies = [
  "crossbeam-epoch",
  "crossbeam-utils",
@@ -452,9 +442,9 @@ dependencies = [
 
 [[package]]
 name = "crossbeam-utils"
-version = "0.8.20"
+version = "0.8.21"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "22ec99545bb0ed0ea7bb9b8e1e9122ea386ff8a48c0922e43f36d45ab09e0e80"
+checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28"
 
 [[package]]
 name = "crypto-common"
@@ -532,7 +522,7 @@ checksum = "8034092389675178f570469e6c3b0465d3d30b4505c294a6550db47f3c17ad18"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -555,7 +545,7 @@ checksum = "ecb08c4819242b1ec89b3d0c6affa229005bef46ae4f7eed8b80768187c10087"
 
 [[package]]
 name = "e2e"
-version = "3.10.0"
+version = "3.11.0"
 dependencies = [
  "anyhow",
  "avbroot",
@@ -649,7 +639,7 @@ dependencies = [
 
 [[package]]
 name = "fuzz"
-version = "3.10.0"
+version = "3.11.0"
 dependencies = [
  "avbroot",
  "honggfuzz",
@@ -829,10 +819,16 @@ dependencies = [
 ]
 
 [[package]]
-name = "libc"
-version = "0.2.167"
+name = "libbz2-rs-sys"
+version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "09d6582e104315a817dff97f75133544b2e094ee22447d2acf4a74e189ba06fc"
+checksum = "55bbb91f13e5136d00496b884cdb605fb983d6f964c8735bc5837d1c98550fd5"
+
+[[package]]
+name = "libc"
+version = "0.2.169"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a"
 
 [[package]]
 name = "liblzma"
@@ -845,9 +841,9 @@ dependencies = [
 
 [[package]]
 name = "liblzma-sys"
-version = "0.3.9"
+version = "0.3.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6630cb23edeb2e563cd6c30d4117554c69646871455843c33ddcb1d9aef82ecf"
+checksum = "41e2171ce6827cbab9bc97238a58361bf9a526080475f21dbc470e1842258b2d"
 dependencies = [
  "cc",
  "libc",
@@ -874,18 +870,18 @@ checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24"
 
 [[package]]
 name = "logos"
-version = "0.14.2"
+version = "0.14.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1c6b6e02facda28ca5fb8dbe4b152496ba3b1bd5a4b40bb2b1b2d8ad74e0f39b"
+checksum = "7251356ef8cb7aec833ddf598c6cb24d17b689d20b993f9d11a3d764e34e6458"
 dependencies = [
  "logos-derive",
 ]
 
 [[package]]
 name = "logos-codegen"
-version = "0.14.3"
+version = "0.14.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5f3303189202bb8a052bcd93d66b6c03e6fe70d9c7c47c0ea5e974955e54c876"
+checksum = "59f80069600c0d66734f5ff52cc42f2dabd6b29d205f333d61fd7832e9e9963f"
 dependencies = [
  "beef",
  "fnv",
@@ -893,15 +889,14 @@ dependencies = [
  "proc-macro2",
  "quote",
  "regex-syntax",
- "rustc_version",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
 name = "logos-derive"
-version = "0.14.3"
+version = "0.14.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "774a1c225576486e4fdf40b74646f672c542ca3608160d348749693ae9d456e6"
+checksum = "24fb722b06a9dc12adb0963ed585f19fc61dc5413e6a9be9422ef92c091e731d"
 dependencies = [
  "logos-codegen",
 ]
@@ -956,14 +951,14 @@ checksum = "23c9b935fbe1d6cbd1dac857b54a688145e2d93f48db36010514d0f612d0ad67"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
 name = "miniz_oxide"
-version = "0.8.0"
+version = "0.8.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e2d80299ef12ff69b16a84bb182e3b9df68b5a91574d3d4fa6e41b65deec4df1"
+checksum = "4ffbe83022cedc1d264172192511ae958937694cd57ce297164951b8b3568394"
 dependencies = [
  "adler2",
 ]
@@ -1124,7 +1119,7 @@ dependencies = [
  "phf_shared",
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -1202,7 +1197,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "64d1ec885c64d0457d564db4ec299b2dae3f9c02808b8ad9c3a089c591b18033"
 dependencies = [
  "proc-macro2",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -1240,7 +1235,7 @@ dependencies = [
  "prost",
  "prost-types",
  "regex",
- "syn 2.0.90",
+ "syn 2.0.91",
  "tempfile",
 ]
 
@@ -1254,7 +1249,7 @@ dependencies = [
  "itertools",
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -1442,9 +1437,9 @@ dependencies = [
 
 [[package]]
 name = "rustix"
-version = "0.38.41"
+version = "0.38.42"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d7f649912bc1495e167a6edee79151c84b1bad49748cb4f1f1167f459f6224f6"
+checksum = "f93dc38ecbab2eb790ff964bb77fa94faf256fd3e73285fd7ba0903b76bedb85"
 dependencies = [
  "bitflags",
  "errno",
@@ -1452,7 +1447,7 @@ dependencies = [
  "libc",
  "linux-raw-sys",
  "once_cell",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -1477,28 +1472,28 @@ dependencies = [
 
 [[package]]
 name = "semver"
-version = "1.0.23"
+version = "1.0.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "61697e0a1c7e512e84a621326239844a24d8207b4669b41bc18b32ea5cbf988b"
+checksum = "3cb6eb87a131f756572d7fb904f6e7b68633f09cca868c5df1c4b8d1a694bbba"
 
 [[package]]
 name = "serde"
-version = "1.0.215"
+version = "1.0.216"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6513c1ad0b11a9376da888e3e0baa0077f1aed55c17f50e7b2397136129fb88f"
+checksum = "0b9781016e935a97e8beecf0c933758c97a5520d32930e460142b4cd80c6338e"
 dependencies = [
  "serde_derive",
 ]
 
 [[package]]
 name = "serde_derive"
-version = "1.0.215"
+version = "1.0.216"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ad1e866f866923f252f05c889987993144fb74e722403468a4ebd70c3cd756c0"
+checksum = "46f859dbbf73865c6627ed570e78961cd3ac92407a2d117204c49232485da55e"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -1622,9 +1617,9 @@ dependencies = [
 
 [[package]]
 name = "syn"
-version = "2.0.90"
+version = "2.0.91"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "919d3b74a5dd0ccd15aeb8f93e7006bd9e14c295087c9896a110f490752bcf31"
+checksum = "d53cbcb5a243bd33b7858b1d7f4aca2153490815872d86d955d6ea29f743c035"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1655,11 +1650,11 @@ dependencies = [
 
 [[package]]
 name = "thiserror"
-version = "2.0.6"
+version = "2.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8fec2a1820ebd077e2b90c4df007bebf344cd394098a13c563957d0afc83ea47"
+checksum = "f072643fd0190df67a8bab670c20ef5d8737177d6ac6b2e9a236cb096206b2cc"
 dependencies = [
- "thiserror-impl 2.0.6",
+ "thiserror-impl 2.0.9",
 ]
 
 [[package]]
@@ -1670,18 +1665,18 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
 name = "thiserror-impl"
-version = "2.0.6"
+version = "2.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d65750cab40f4ff1929fb1ba509e9914eb756131cef4210da8d5d700d26f6312"
+checksum = "7b50fa271071aae2e6ee85f842e2e28ba8cd2c5fb67f11fcb1fd70b276f9e7d4"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -1712,7 +1707,7 @@ checksum = "8d9ef545650e79f30233c0003bcc2504d7efac6dad25fca40744de773fe2049c"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -1762,7 +1757,7 @@ checksum = "395ae124c09f9e6918a2310af6038fba074bcf474ac352496d5910dd59a2226d"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -2006,7 +2001,7 @@ dependencies = [
 
 [[package]]
 name = "xtask"
-version = "3.10.0"
+version = "3.11.0"
 dependencies = [
  "anyhow",
  "clap",
@@ -2026,11 +2021,11 @@ dependencies = [
 
 [[package]]
 name = "zerocopy"
-version = "0.8.13"
+version = "0.8.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "67914ab451f3bfd2e69e5e9d2ef3858484e7074d63f204fd166ec391b54de21d"
+checksum = "a367f292d93d4eab890745e75a778da40909cab4d6ff8173693812f79c4a2468"
 dependencies = [
- "zerocopy-derive 0.8.13",
+ "zerocopy-derive 0.8.14",
 ]
 
 [[package]]
@@ -2041,18 +2036,18 @@ checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
 name = "zerocopy-derive"
-version = "0.8.13"
+version = "0.8.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7988d73a4303ca289df03316bc490e934accf371af6bc745393cf3c2c5c4f25d"
+checksum = "d3931cb58c62c13adec22e38686b559c86a30565e16ad6e8510a337cedc611e1"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]
@@ -2072,7 +2067,7 @@ checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn 2.0.90",
+ "syn 2.0.91",
 ]
 
 [[package]]

Cargo.toml

@@ -4,7 +4,7 @@ members = ["avbroot", "e2e", "fuzz", "xtask"]
 resolver = "2"
 
 [workspace.package]
-version = "3.10.0"
+version = "3.11.0"
 license = "GPL-3.0-only"
 edition = "2021"
 repository = "https://github.com/chenxiaolong/avbroot"

avbroot/Cargo.toml

@@ -57,12 +57,12 @@ x509-cert = { version = "0.2.4", features = ["builder"] }
 zerocopy = { version = "0.8.10", features = ["std"] }
 zerocopy-derive = "0.8.5"
 
-# There are multiple upstream bugs that cause infinite loops in the Drop
-# implementation of write::BzDecoder. Unfortunately, the project is no longer
-# maintained, so we have to maintain our own fork with the necessary fixes.
+# Waiting for next stable release.
 [dependencies.bzip2]
-git = "https://github.com/chenxiaolong/bzip2-rs"
-rev = "6e0f9836ec87b19261461b6cc1772e14aff8e851"
+git = "https://github.com/trifectatechfoundation/bzip2-rs"
+rev = "15258feb0cdc1114d6fc6b936254c4f4e1d5730c"
+default-features = false
+features = ["libbz2-rs-sys"]
 
 # https://github.com/zip-rs/zip/pull/383
 [dependencies.zip]
@@ -84,7 +84,7 @@ protox = "0.7.0"
 assert_matches = "1.5.0"
 
 [features]
-static = ["bzip2/static", "liblzma/static"]
+static = ["liblzma/static"]
 
 [lints]
 workspace = true

avbroot/src/cli/avb.rs

@@ -5,9 +5,8 @@ use std::{
     collections::{HashMap, HashSet},
     ffi::{OsStr, OsString},
     fs::{self, File},
-    io::{self, BufReader, BufWriter, Cursor, Read, Seek, SeekFrom, Write},
+    io::{self, BufReader, BufWriter, Cursor, Seek, SeekFrom, Write},
     path::{Path, PathBuf},
-    str,
     sync::atomic::AtomicBool,
 };
 
@@ -28,7 +27,7 @@ use crate::{
         self, AlgorithmType, AppendedDescriptorMut, AppendedDescriptorRef, Descriptor, Footer,
         HashTreeDescriptor, Header, KernelCmdlineDescriptor,
     },
-    stream::{self, check_cancel, PSeekFile, Reopen, ToWriter},
+    stream::{self, check_cancel, PSeekFile, ReadFixedSizeExt, Reopen, ToWriter},
     util,
 };
 
@@ -526,7 +525,7 @@ fn verify_and_repair(
             info!("Verifying hash tree descriptor{suffix}");
 
             match d.verify(&file, cancel_signal) {
-                Err(e @ avb::Error::HashTree(_)) if repair => {
+                Err(e @ avb::Error::HashTreeVerify(_)) if repair => {
                     warn!("Failed to verify hash tree descriptor{suffix}: {e}");
                     warn!("Attempting to repair using FEC data{suffix}");
 
@@ -639,9 +638,8 @@ fn compute_digest_recursive(
         .seek(SeekFrom::Start(header_offset))
         .with_context(|| format!("Failed to seek file: {path:?}"))?;
 
-    let mut raw_header_orig = vec![0u8; raw_header.len()];
-    raw_reader
-        .read_exact(&mut raw_header_orig)
+    let raw_header_orig = raw_reader
+        .read_vec_exact(raw_header.len())
         .with_context(|| format!("Failed to reread AVB header: {path:?}"))?;
 
     if raw_header != raw_header_orig {

avbroot/src/cli/ota.rs

@@ -1013,7 +1013,7 @@ fn patch_ota_zip(
                 // Use the user's certificate
                 info!("Replacing zip entry: {path}");
 
-                crypto::write_pem_cert(&mut writer, cert_ota)
+                crypto::write_pem_cert(Path::new(path), &mut writer, cert_ota)
                     .with_context(|| format!("Failed to write entry: {path}"))?;
             }
             ota::PATH_PAYLOAD => {

avbroot/src/crypto.rs

@@ -5,7 +5,7 @@ use std::{
     env::{self, VarError},
     ffi::{OsStr, OsString},
     fs::{self, File, OpenOptions},
-    io::{self, BufReader, BufWriter, Read, Write},
+    io::{self, Read, Write},
     path::{Path, PathBuf},
     process::{Command, ExitStatus, Stdio},
     time::Duration,
@@ -42,20 +42,22 @@ use x509_cert::{
     Certificate,
 };
 
+use crate::util::DebugString;
+
 #[derive(Debug, Error)]
 pub enum Error {
     #[error("Signature algorithm not supported: {0:?}")]
     UnsupportedAlgorithm(SignatureAlgorithm),
     #[error("RSA key size ({}) not supported", .0 * 8)]
-    UnsupportedKey(usize),
+    UnsupportedKeySize(usize),
     #[error("Invalid digest length ({0} bytes) for {1:?}")]
     InvalidDigestLength(usize, SignatureAlgorithm),
     #[error("Invalid signature length ({0} bytes) for {1:?}")]
     InvalidSignatureLength(usize, SignatureAlgorithm),
-    #[error("Failed to run command: {0}")]
-    CommandSpawnFailed(String, #[source] io::Error),
-    #[error("Command failed with status: {1}: {0}")]
-    CommandExecutionFailed(String, ExitStatus),
+    #[error("Failed to run command: {0:?}")]
+    CommandSpawn(DebugString, #[source] io::Error),
+    #[error("Command failed with status: {1}: {0:?}")]
+    CommandExecution(DebugString, ExitStatus),
     #[error("Signature from signing helper does not match public key: {0:?}")]
     SigningHelperBadSignature(PathBuf),
     #[error("Passphrase prompt requires an interactive terminal")]
@@ -68,28 +70,40 @@ pub enum Error {
     InvalidEnvVar(OsString, #[source] VarError),
     #[error("PEM has start tag, but no end tag")]
     PemNoEndTag,
-    #[error("Failed to load encrypted private key")]
+    #[error("Failed to load encrypted RSA private key")]
     LoadKeyEncrypted(#[source] pkcs8::Error),
-    #[error("Failed to load unencrypted private key")]
+    #[error("Failed to load unencrypted RSA private key")]
     LoadKeyUnencrypted(#[source] pkcs8::Error),
-    #[error("Failed to save encrypted private key")]
+    #[error("Failed to save encrypted RSA private key")]
     SaveKeyEncrypted(#[source] pkcs8::Error),
-    #[error("Failed to save unencrypted private key")]
+    #[error("Failed to save unencrypted RSA private key")]
     SaveKeyUnencrypted(#[source] pkcs8::Error),
+    #[error("Failed to load RSA public key")]
+    LoadPubKey(#[source] pkcs8::spki::Error),
+    #[error("Failed to save RSA public key")]
+    SavePubKey(#[source] pkcs8::spki::Error),
+    #[error("Failed to load X509 certificate")]
+    LoadCert(#[source] x509_cert::der::Error),
+    #[error("Failed to save X509 certificate")]
+    SaveCert(#[source] x509_cert::der::Error),
+    #[error("Failed to generate RSA key")]
+    RsaGenerate(#[source] Box<rsa::Error>),
     #[error("Failed to RSA sign digest")]
-    RsaSign(#[source] rsa::Error),
+    RsaSign(#[source] Box<rsa::Error>),
     #[error("Failed to RSA verify signature")]
-    RsaVerify(#[source] rsa::Error),
-    #[error("X509 error")]
-    X509(#[from] x509_cert::builder::Error),
-    #[error("SPKI error")]
-    Spki(#[from] pkcs8::spki::Error),
-    #[error("DER error")]
-    Der(#[from] x509_cert::der::Error),
-    #[error("RSA error")]
-    Rsa(#[from] rsa::Error),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    RsaVerify(#[source] Box<rsa::Error>),
+    #[error("Failed to generate X509 certificate")]
+    CertGenerate(#[source] x509_cert::builder::Error),
+    #[error("Invalid parameters for X509 certificate generation")]
+    CertParams(#[source] x509_cert::der::Error),
+    #[error("Failed to CMS sign digest")]
+    CmsSign(#[source] x509_cert::der::Error),
+    #[error("Failed to parse CMS signature")]
+    CmsParse(#[source] x509_cert::der::Error),
+    #[error("Failed to read file: {0:?}")]
+    ReadFile(PathBuf, #[source] io::Error),
+    #[error("Failed to write file: {0:?}")]
+    WriteFile(PathBuf, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -175,7 +189,8 @@ impl PassphraseSource {
                 first
             }
             Self::EnvVar(v) => env::var(v).map_err(|e| Error::InvalidEnvVar(v.clone(), e))?,
-            Self::File(p) => fs::read_to_string(p)?
+            Self::File(p) => fs::read_to_string(p)
+                .map_err(|e| Error::ReadFile(p.clone(), e))?
                 .trim_end_matches(['\r', '\n'])
                 .to_owned(),
         };
@@ -187,7 +202,7 @@ impl PassphraseSource {
 fn check_key_size(size: usize) -> Result<()> {
     // RustCrypto does not support 8192-bit keys.
     if size > 4096 / 8 {
-        return Err(Error::UnsupportedKey(size));
+        return Err(Error::UnsupportedKeySize(size));
     }
 
     Ok(())
@@ -256,7 +271,9 @@ impl RsaSigningKey {
         };
 
         match self {
-            Self::Internal(key) => key.sign(scheme, digest).map_err(Error::RsaSign),
+            Self::Internal(key) => key
+                .sign(scheme, digest)
+                .map_err(|e| Error::RsaSign(Box::new(e))),
             Self::External {
                 program,
                 public_key,
@@ -292,7 +309,7 @@ impl RsaSigningKey {
 
                 let mut child = command
                     .spawn()
-                    .map_err(|e| Error::CommandSpawnFailed(format!("{command:?}"), e))?;
+                    .map_err(|e| Error::CommandSpawn(DebugString::new(&command), e))?;
 
                 // We don't bother with spawning a thread. The pipe capacity on
                 // all major OSs is significantly larger than the digest, so we
@@ -304,14 +321,22 @@ impl RsaSigningKey {
                 // * macOS: 4 KiB, 16 KiB (usually), or 64 KiB
                 // * Windows: 4 KiB
 
-                let padded_digest = pkcs1v15_sign_pad(&scheme.prefix, digest, public_key.size())?;
-                child.stdin.as_mut().unwrap().write_all(&padded_digest)?;
+                let padded_digest = pkcs1v15_sign_pad(&scheme.prefix, digest, public_key.size())
+                    .map_err(|e| Error::RsaSign(Box::new(e)))?;
+                child
+                    .stdin
+                    .as_mut()
+                    .unwrap()
+                    .write_all(&padded_digest)
+                    .map_err(|e| Error::WriteFile("<signing helper stdin>".into(), e))?;
 
-                let child = child.wait_with_output()?;
+                let child = child
+                    .wait_with_output()
+                    .map_err(|e| Error::CommandSpawn(DebugString::new(&command), e))?;
 
                 if !child.status.success() {
-                    return Err(Error::CommandExecutionFailed(
-                        format!("{command:?}"),
+                    return Err(Error::CommandExecution(
+                        DebugString::new(&command),
                         child.status,
                     ));
                 } else if child.stdout.len() != self.size() {
@@ -355,7 +380,7 @@ impl RsaPublicKeyExt for RsaPublicKey {
         };
 
         self.verify(scheme, digest, signature)
-            .map_err(Error::RsaVerify)
+            .map_err(|e| Error::RsaVerify(Box::new(e)))
     }
 }
 
@@ -364,7 +389,7 @@ pub fn generate_rsa_key_pair() -> Result<RsaPrivateKey> {
     let mut rng = rand::thread_rng();
 
     // avbroot supports 4096-bit keys only.
-    let key = RsaPrivateKey::new(&mut rng, 4096)?;
+    let key = RsaPrivateKey::new(&mut rng, 4096).map_err(|e| Error::RsaGenerate(Box::new(e)))?;
 
     Ok(key)
 }
@@ -376,20 +401,27 @@ pub fn generate_cert(
     validity: Duration,
     subject: &str,
 ) -> Result<Certificate> {
-    let public_key_der = key.to_public_key().to_public_key_der()?;
+    let public_key_der = key
+        .to_public_key()
+        .to_public_key_der()
+        .map_err(Error::SavePubKey)?;
     let signing_key = SigningKey::<Sha256>::new(key.clone());
 
     let builder = CertificateBuilder::new(
         Profile::Root,
         SerialNumber::from(serial),
-        Validity::from_now(validity)?,
-        subject.parse()?,
-        SubjectPublicKeyInfoOwned::from_der(public_key_der.as_bytes())?,
+        Validity::from_now(validity).map_err(Error::CertParams)?,
+        subject.parse().map_err(Error::CertParams)?,
+        SubjectPublicKeyInfoOwned::from_der(public_key_der.as_bytes())
+            .map_err(Error::CertParams)?,
         &signing_key,
-    )?;
+    )
+    .map_err(Error::CertGenerate)?;
 
     let mut rng = rand::thread_rng();
-    let cert = builder.build_with_rng(&mut rng)?;
+    let cert = builder
+        .build_with_rng(&mut rng)
+        .map_err(Error::CertGenerate)?;
 
     Ok(cert)
 }
@@ -442,80 +474,92 @@ fn reformat_pem(data: &[u8]) -> Result<Vec<u8>> {
 }
 
 /// Read PEM-encoded certificate from a reader.
-pub fn read_pem_cert(mut reader: impl Read) -> Result<Certificate> {
+pub fn read_pem_cert(path: &Path, mut reader: impl Read) -> Result<Certificate> {
     let mut data = vec![];
-    reader.read_to_end(&mut data)?;
+    reader
+        .read_to_end(&mut data)
+        .map_err(|e| Error::ReadFile(path.to_owned(), e))?;
 
     let data = reformat_pem(&data)?;
-    let certificate = Certificate::from_pem(data)?;
+    let certificate = Certificate::from_pem(data).map_err(Error::LoadCert)?;
 
     Ok(certificate)
 }
 
 /// Write PEM-encoded certificate to a writer.
-pub fn write_pem_cert(mut writer: impl Write, cert: &Certificate) -> Result<()> {
-    let data = cert.to_pem(LineEnding::LF)?;
+pub fn write_pem_cert(path: &Path, mut writer: impl Write, cert: &Certificate) -> Result<()> {
+    let data = cert.to_pem(LineEnding::LF).map_err(Error::SaveCert)?;
 
-    writer.write_all(data.as_bytes())?;
+    writer
+        .write_all(data.as_bytes())
+        .map_err(|e| Error::WriteFile(path.to_owned(), e))?;
 
     Ok(())
 }
 
 /// Read PEM-encoded certificate from a file.
 pub fn read_pem_cert_file(path: &Path) -> Result<Certificate> {
-    let file = File::open(path)?;
-    let reader = BufReader::new(file);
+    let reader = File::open(path).map_err(|e| Error::ReadFile(path.to_owned(), e))?;
 
-    read_pem_cert(reader)
+    read_pem_cert(path, reader)
 }
 
 /// Write PEM-encoded certificate to a file.
 pub fn write_pem_cert_file(path: &Path, cert: &Certificate) -> Result<()> {
-    let file = File::create(path)?;
-    let writer = BufWriter::new(file);
+    let writer = File::create(path).map_err(|e| Error::WriteFile(path.to_owned(), e))?;
 
-    write_pem_cert(writer, cert)
+    write_pem_cert(path, writer, cert)
 }
 
 /// Read PEM-encoded PKCS8 public key from a reader.
-pub fn read_pem_public_key(mut reader: impl Read) -> Result<RsaPublicKey> {
+pub fn read_pem_public_key(path: &Path, mut reader: impl Read) -> Result<RsaPublicKey> {
     let mut data = String::new();
-    reader.read_to_string(&mut data)?;
+    reader
+        .read_to_string(&mut data)
+        .map_err(|e| Error::ReadFile(path.to_owned(), e))?;
 
-    let key = RsaPublicKey::from_public_key_pem(&data)?;
+    let key = RsaPublicKey::from_public_key_pem(&data).map_err(Error::LoadPubKey)?;
 
     Ok(key)
 }
 
 /// Write PEM-encoded PKCS8 public key to a writer.
-pub fn write_pem_public_key(mut writer: impl Write, key: &RsaPublicKey) -> Result<()> {
-    let data = key.to_public_key_pem(LineEnding::LF)?;
+pub fn write_pem_public_key(path: &Path, mut writer: impl Write, key: &RsaPublicKey) -> Result<()> {
+    let data = key
+        .to_public_key_pem(LineEnding::LF)
+        .map_err(Error::SavePubKey)?;
 
-    writer.write_all(data.as_bytes())?;
+    writer
+        .write_all(data.as_bytes())
+        .map_err(|e| Error::WriteFile(path.to_owned(), e))?;
 
     Ok(())
 }
 
 /// Read PEM-encoded PKCS8 public key from a file.
 pub fn read_pem_public_key_file(path: &Path) -> Result<RsaPublicKey> {
-    let file = File::open(path)?;
-    let reader = BufReader::new(file);
+    let reader = File::open(path).map_err(|e| Error::ReadFile(path.to_owned(), e))?;
 
-    read_pem_public_key(reader)
+    read_pem_public_key(path, reader)
 }
 
 /// Write PEM-encoded PKCS8 public key to a file.
 pub fn write_pem_public_key_file(path: &Path, key: &RsaPublicKey) -> Result<()> {
-    let file = File::create(path)?;
-    let writer = BufWriter::new(file);
+    let writer = File::create(path).map_err(|e| Error::WriteFile(path.to_owned(), e))?;
 
-    write_pem_public_key(writer, key)
+    write_pem_public_key(path, writer, key)
 }
 
 /// Read PEM-encoded PKCS8 private key from a reader.
-pub fn read_pem_key(mut reader: impl Read, source: &PassphraseSource) -> Result<RsaPrivateKey> {
+pub fn read_pem_key(
+    path: &Path,
+    mut reader: impl Read,
+    source: &PassphraseSource,
+) -> Result<RsaPrivateKey> {
     let mut data = String::new();
-    reader.read_to_string(&mut data)?;
+    reader
+        .read_to_string(&mut data)
+        .map_err(|e| Error::ReadFile(path.to_owned(), e))?;
 
     if data.contains("ENCRYPTED") {
         let passphrase = source.acquire(false)?;
@@ -528,6 +572,7 @@ pub fn read_pem_key(mut reader: impl Read, source: &PassphraseSource) -> Result<
 
 /// Write PEM-encoded PKCS8 private key to a writer.
 pub fn write_pem_key(
+    path: &Path,
     mut writer: impl Write,
     key: &RsaPrivateKey,
     source: &PassphraseSource,
@@ -570,20 +615,24 @@ pub fn write_pem_key(
             .encrypt_with_params(pbes2_params, passphrase)
             .map_err(Error::SaveKeyEncrypted)?;
 
-        secret_doc.to_pem(EncryptedPrivateKeyInfo::PEM_LABEL, LineEnding::LF)?
+        secret_doc
+            .to_pem(EncryptedPrivateKeyInfo::PEM_LABEL, LineEnding::LF)
+            .map_err(pkcs8::Error::Asn1)
+            .map_err(Error::SaveKeyEncrypted)?
     };
 
-    writer.write_all(data.as_bytes())?;
+    writer
+        .write_all(data.as_bytes())
+        .map_err(|e| Error::WriteFile(path.to_owned(), e))?;
 
     Ok(())
 }
 
 /// Read PEM-encoded PKCS8 private key from a file.
 pub fn read_pem_key_file(path: &Path, source: &PassphraseSource) -> Result<RsaPrivateKey> {
-    let file = File::open(path)?;
-    let reader = BufReader::new(file);
+    let reader = File::open(path).map_err(|e| Error::ReadFile(path.to_owned(), e))?;
 
-    read_pem_key(reader, source)
+    read_pem_key(path, reader, source)
 }
 
 /// Save PEM-encoded PKCS8 private key to a file.
@@ -603,16 +652,18 @@ pub fn write_pem_key_file(
         options.mode(0o600);
     }
 
-    let file = options.open(path)?;
-    let writer = BufWriter::new(file);
+    let writer = options
+        .open(path)
+        .map_err(|e| Error::WriteFile(path.to_owned(), e))?;
 
-    write_pem_key(writer, key, source)
+    write_pem_key(path, writer, key, source)
 }
 
 /// Get the RSA public key from a certificate.
 pub fn get_public_key(cert: &Certificate) -> Result<RsaPublicKey> {
     let public_key =
-        RsaPublicKey::try_from(cert.tbs_certificate.subject_public_key_info.owned_to_ref())?;
+        RsaPublicKey::try_from(cert.tbs_certificate.subject_public_key_info.owned_to_ref())
+            .map_err(Error::LoadPubKey)?;
 
     Ok(public_key)
 }
@@ -626,8 +677,11 @@ pub fn cert_matches_key(cert: &Certificate, key: &RsaSigningKey) -> Result<bool>
 
 /// Parse a CMS [`SignedData`] structure from raw DER-encoded data.
 pub fn parse_cms(data: &[u8]) -> Result<SignedData> {
-    let ci = ContentInfo::from_der(data)?;
-    let sd = ci.content.decode_as::<SignedData>()?;
+    let ci = ContentInfo::from_der(data).map_err(Error::CmsParse)?;
+    let sd = ci
+        .content
+        .decode_as::<SignedData>()
+        .map_err(Error::CmsParse)?;
 
     Ok(sd)
 }
@@ -669,14 +723,16 @@ pub fn cms_sign_external(
 
     let signed_data = SignedData {
         version: CmsVersion::V1,
-        digest_algorithms: DigestAlgorithmIdentifiers::try_from(vec![digest_algorithm.clone()])?,
+        digest_algorithms: DigestAlgorithmIdentifiers::try_from(vec![digest_algorithm.clone()])
+            .map_err(Error::CmsSign)?,
         encap_content_info: EncapsulatedContentInfo {
             econtent_type: const_oid::db::rfc5911::ID_DATA,
             econtent: None,
         },
-        certificates: Some(CertificateSet::try_from(vec![
-            CertificateChoices::Certificate(cert.clone()),
-        ])?),
+        certificates: Some(
+            CertificateSet::try_from(vec![CertificateChoices::Certificate(cert.clone())])
+                .map_err(Error::CmsSign)?,
+        ),
         crls: None,
         signer_infos: SignerInfos::try_from(vec![SignerInfo {
             version: CmsVersion::V1,
@@ -690,14 +746,15 @@ pub fn cms_sign_external(
                 oid: const_oid::db::rfc5912::SHA_256_WITH_RSA_ENCRYPTION,
                 parameters: None,
             },
-            signature: SignatureValue::new(signature)?,
+            signature: SignatureValue::new(signature).map_err(Error::CmsSign)?,
             unsigned_attrs: None,
-        }])?,
+        }])
+        .map_err(Error::CmsSign)?,
     };
 
     let signed_data = ContentInfo {
         content_type: const_oid::db::rfc5911::ID_SIGNED_DATA,
-        content: Any::encode_from(&signed_data)?,
+        content: Any::encode_from(&signed_data).map_err(Error::CmsSign)?,
     };
 
     Ok(signed_data)

avbroot/src/format/bootimage.rs

@@ -95,10 +95,16 @@ pub enum Error {
     },
     #[error("VTS signature is missing hash descriptor")]
     MissingHashDescriptor,
-    #[error("AVB error")]
-    Avb(#[from] avb::Error),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to load VTS AVB signature")]
+    VtsAvbLoad(#[source] avb::Error),
+    #[error("Failed to save VTS AVB signature")]
+    VtsAvbSave(#[source] avb::Error),
+    #[error("Failed to generate VTS AVB signature")]
+    VtsAvbSign(#[source] avb::Error),
+    #[error("Failed to read boot image data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write boot image data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -293,7 +299,8 @@ impl<R: Read> FromReader<R> for BootImageV0Through2 {
     fn from_reader(reader: R) -> Result<Self> {
         let mut reader = CountingReader::new(reader);
 
-        let raw_v0 = RawV0::read_from_io(&mut reader)?;
+        let raw_v0 =
+            RawV0::read_from_io(&mut reader).map_err(|e| Error::DataRead("Boot::V0::header", e))?;
 
         if raw_v0.magic != BOOT_MAGIC {
             return Err(Error::UnknownMagic(raw_v0.magic));
@@ -336,7 +343,8 @@ impl<R: Read> FromReader<R> for BootImageV0Through2 {
         }
 
         let mut v1_data = if header_version >= 1 {
-            let raw_v1 = RawV1Extra::read_from_io(&mut reader)?;
+            let raw_v1 = RawV1Extra::read_from_io(&mut reader)
+                .map_err(|e| Error::DataRead("Boot::V1::header", e))?;
 
             let recovery_dtbo_size =
                 util::check_bounds(raw_v1.recovery_dtbo_size.get(), ..=COMPONENT_MAX_SIZE)
@@ -362,7 +370,8 @@ impl<R: Read> FromReader<R> for BootImageV0Through2 {
         }
 
         let mut v2_data = if header_version == 2 {
-            let raw_v2 = RawV2Extra::read_from_io(&mut reader)?;
+            let raw_v2 = RawV2Extra::read_from_io(&mut reader)
+                .map_err(|e| Error::DataRead("Boot::V2::header", e))?;
 
             let dtb_size = util::check_bounds(raw_v2.dtb_size.get(), ..=COMPONENT_MAX_SIZE)
                 .map_err(|e| Error::IntOutOfBounds("Boot::V2::dtb_size", e))?;
@@ -378,34 +387,50 @@ impl<R: Read> FromReader<R> for BootImageV0Through2 {
         };
 
         if let Some(v1) = &v1_data {
-            if reader.stream_position()? != u64::from(v1.header_size) {
+            if reader
+                .stream_position()
+                .map_err(|e| Error::DataRead("Boot::V1::header_size", e))?
+                != u64::from(v1.header_size)
+            {
                 return Err(Error::InvalidHeaderSize(v1.header_size));
             }
         }
 
-        padding::read_discard(&mut reader, page_size.into())?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Boot::V0::header_padding", e))?;
 
-        let kernel = reader.read_vec_exact(kernel_size as usize)?;
-        padding::read_discard(&mut reader, page_size.into())?;
+        let kernel = reader
+            .read_vec_exact(kernel_size as usize)
+            .map_err(|e| Error::DataRead("Boot::V0::kernel", e))?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Boot::V0::kernel_padding", e))?;
 
-        let ramdisk = reader.read_vec_exact(ramdisk_size as usize)?;
-        padding::read_discard(&mut reader, page_size.into())?;
+        let ramdisk = reader
+            .read_vec_exact(ramdisk_size as usize)
+            .map_err(|e| Error::DataRead("Boot::V0::ramdisk", e))?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Boot::V0::ramdisk_padding", e))?;
 
-        let second = reader.read_vec_exact(second_size as usize)?;
-        padding::read_discard(&mut reader, page_size.into())?;
+        let second = reader
+            .read_vec_exact(second_size as usize)
+            .map_err(|e| Error::DataRead("Boot::V0::second", e))?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Boot::V0::second_padding", e))?;
 
         if let Some(v1) = &mut v1_data {
-            v1.v1_extra
-                .recovery_dtbo
-                .resize(v1.recovery_dtbo_size as usize, 0);
-            reader.read_exact(&mut v1.v1_extra.recovery_dtbo)?;
-            padding::read_discard(&mut reader, page_size.into())?;
+            v1.v1_extra.recovery_dtbo = reader
+                .read_vec_exact(v1.recovery_dtbo_size as usize)
+                .map_err(|e| Error::DataRead("Boot::V1::recovery_dtbo", e))?;
+            padding::read_discard(&mut reader, page_size.into())
+                .map_err(|e| Error::DataRead("Boot::V1::recovery_dtbo_padding", e))?;
         }
 
         if let Some(v2) = &mut v2_data {
-            v2.v2_extra.dtb.resize(v2.dtb_size as usize, 0);
-            reader.read_exact(&mut v2.v2_extra.dtb)?;
-            padding::read_discard(&mut reader, page_size.into())?;
+            v2.v2_extra.dtb = reader
+                .read_vec_exact(v2.dtb_size as usize)
+                .map_err(|e| Error::DataRead("Boot::V2::dtb", e))?;
+            padding::read_discard(&mut reader, page_size.into())
+                .map_err(|e| Error::DataRead("Boot::V2::dtb_padding", e))?;
         }
 
         let image = Self {
@@ -500,7 +525,9 @@ impl<W: Write> ToWriter<W> for BootImageV0Through2 {
             extra_cmdline,
         };
 
-        raw_v0.write_to_io(&mut writer)?;
+        raw_v0
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("Boot::V0::header", e))?;
 
         if let Some(v1) = &self.v1_extra {
             let raw_v1 = RawV1Extra {
@@ -509,7 +536,9 @@ impl<W: Write> ToWriter<W> for BootImageV0Through2 {
                 header_size: self.header_size().into(),
             };
 
-            raw_v1.write_to_io(&mut writer)?;
+            raw_v1
+                .write_to_io(&mut writer)
+                .map_err(|e| Error::DataWrite("Boot::V1::header", e))?;
         }
 
         if let Some(v2) = &self.v2_extra {
@@ -518,28 +547,46 @@ impl<W: Write> ToWriter<W> for BootImageV0Through2 {
                 dtb_addr: v2.dtb_addr.into(),
             };
 
-            raw_v2.write_to_io(&mut writer)?;
+            raw_v2
+                .write_to_io(&mut writer)
+                .map_err(|e| Error::DataWrite("Boot::V2::header", e))?;
         }
 
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Boot::V0::header_padding", e))?;
 
-        writer.write_all(&self.kernel)?;
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        writer
+            .write_all(&self.kernel)
+            .map_err(|e| Error::DataWrite("Boot::V0::kernel", e))?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Boot::V0::kernel_padding", e))?;
 
-        writer.write_all(&self.ramdisk)?;
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        writer
+            .write_all(&self.ramdisk)
+            .map_err(|e| Error::DataWrite("Boot::V0::ramdisk", e))?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Boot::V0::ramdisk_padding", e))?;
 
-        writer.write_all(&self.second)?;
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        writer
+            .write_all(&self.second)
+            .map_err(|e| Error::DataWrite("Boot::V0::second", e))?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Boot::V0::second_padding", e))?;
 
         if let Some(v1) = &self.v1_extra {
-            writer.write_all(&v1.recovery_dtbo)?;
-            padding::write_zeros(&mut writer, self.page_size.into())?;
+            writer
+                .write_all(&v1.recovery_dtbo)
+                .map_err(|e| Error::DataWrite("Boot::V1::recovery_dtbo", e))?;
+            padding::write_zeros(&mut writer, self.page_size.into())
+                .map_err(|e| Error::DataWrite("Boot::V1::recovery_dtbo_padding", e))?;
         }
 
         if let Some(v2) = &self.v2_extra {
-            writer.write_all(&v2.dtb)?;
-            padding::write_zeros(&mut writer, self.page_size.into())?;
+            writer
+                .write_all(&v2.dtb)
+                .map_err(|e| Error::DataWrite("Boot::V2::dtb", e))?;
+            padding::write_zeros(&mut writer, self.page_size.into())
+                .map_err(|e| Error::DataWrite("Boot::V2::dtb_padding", e))?;
         }
 
         Ok(())
@@ -646,7 +693,8 @@ impl<R: Read> FromReader<R> for BootImageV3Through4 {
     fn from_reader(reader: R) -> Result<Self> {
         let mut reader = CountingReader::new(reader);
 
-        let raw_v3 = RawV3::read_from_io(&mut reader)?;
+        let raw_v3 =
+            RawV3::read_from_io(&mut reader).map_err(|e| Error::DataRead("Boot::V3::header", e))?;
 
         if raw_v3.magic != BOOT_MAGIC {
             return Err(Error::UnknownMagic(raw_v3.magic));
@@ -668,7 +716,8 @@ impl<R: Read> FromReader<R> for BootImageV3Through4 {
             .map_err(|e| Error::StringNotUtf8("Boot::V3::cmdline", e, cmdline.to_vec()))?;
 
         let signature_size = if header_version == 4 {
-            let raw_v4 = RawV4Extra::read_from_io(&mut reader)?;
+            let raw_v4 = RawV4Extra::read_from_io(&mut reader)
+                .map_err(|e| Error::DataRead("Boot::V4::header", e))?;
 
             let size =
                 util::check_bounds(raw_v4.signature_size.get(), ..=HDR_V4_SIGNATURE_SIZE as u32)
@@ -679,31 +728,48 @@ impl<R: Read> FromReader<R> for BootImageV3Through4 {
             None
         };
 
-        if reader.stream_position()? != u64::from(header_size) {
+        if reader
+            .stream_position()
+            .map_err(|e| Error::DataRead("Boot::V3::header_size", e))?
+            != u64::from(header_size)
+        {
             return Err(Error::InvalidHeaderSize(header_size));
         }
 
-        padding::read_discard(&mut reader, PAGE_SIZE.into())?;
+        padding::read_discard(&mut reader, PAGE_SIZE.into())
+            .map_err(|e| Error::DataRead("Boot::V3::header_padding", e))?;
 
-        let kernel = reader.read_vec_exact(kernel_size as usize)?;
-        padding::read_discard(&mut reader, PAGE_SIZE.into())?;
+        let kernel = reader
+            .read_vec_exact(kernel_size as usize)
+            .map_err(|e| Error::DataRead("Boot::V3::kernel", e))?;
+        padding::read_discard(&mut reader, PAGE_SIZE.into())
+            .map_err(|e| Error::DataRead("Boot::V3::kernel_padding", e))?;
 
-        let ramdisk = reader.read_vec_exact(ramdisk_size as usize)?;
-        padding::read_discard(&mut reader, PAGE_SIZE.into())?;
+        let ramdisk = reader
+            .read_vec_exact(ramdisk_size as usize)
+            .map_err(|e| Error::DataRead("Boot::V3::ramdisk", e))?;
+        padding::read_discard(&mut reader, PAGE_SIZE.into())
+            .map_err(|e| Error::DataRead("Boot::V3::ramdisk_padding", e))?;
 
         // Don't preserve the signature. It is only used for VTS tests and is
         // not relevant for booting.
         let v4_extra = if let Some(s) = signature_size {
             // OnePlus images have an invalid signature consisting of all zeros.
-            let data = reader.read_vec_exact(s as usize)?;
+            let data = reader
+                .read_vec_exact(s as usize)
+                .map_err(|e| Error::DataRead("Boot::V4::signature", e))?;
 
             let signature = if s > 0 && !util::is_zero(&data) {
-                Some(Header::from_reader(Cursor::new(data))?)
+                let avb_header =
+                    Header::from_reader(Cursor::new(data)).map_err(Error::VtsAvbLoad)?;
+
+                Some(avb_header)
             } else {
                 None
             };
 
-            padding::read_discard(&mut reader, PAGE_SIZE.into())?;
+            padding::read_discard(&mut reader, PAGE_SIZE.into())
+                .map_err(|e| Error::DataRead("Boot::V4::signature_padding", e))?;
 
             Some(V4Extra { signature })
         } else {
@@ -755,21 +821,26 @@ impl BootImageV3Through4 {
             cmdline,
         };
 
-        raw_v3.write_to_io(&mut writer)?;
+        raw_v3
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("Boot::V3::header", e))?;
 
         let v4_signature = if let Some(v4) = &self.v4_extra {
             let mut sig_writer = Cursor::new(Vec::new());
 
             if let Some(s) = &v4.signature {
-                s.to_writer(&mut sig_writer)?;
+                s.to_writer(&mut sig_writer).map_err(Error::VtsAvbSave)?;
 
-                let size = sig_writer.stream_position()?;
+                let size = sig_writer
+                    .stream_position()
+                    .map_err(|e| Error::DataWrite("Boot::V4::signature_size", e))?;
 
                 // The VTS signature is always a fixed size.
                 util::check_bounds(size, ..=HDR_V4_SIGNATURE_SIZE)
                     .map_err(|e| Error::IntOutOfBounds("Boot::V4::signature_size", e))?;
 
-                padding::write_zeros(&mut sig_writer, HDR_V4_SIGNATURE_SIZE)?;
+                padding::write_zeros(&mut sig_writer, HDR_V4_SIGNATURE_SIZE)
+                    .map_err(|e| Error::DataWrite("Boot::V4::signature_inner_padding", e))?;
             }
 
             let sig = sig_writer.into_inner();
@@ -778,25 +849,37 @@ impl BootImageV3Through4 {
                 signature_size: (sig.len() as u32).into(),
             };
 
-            raw_v4.write_to_io(&mut writer)?;
+            raw_v4
+                .write_to_io(&mut writer)
+                .map_err(|e| Error::DataWrite("Boot::V4::header", e))?;
 
             Some(sig)
         } else {
             None
         };
 
-        padding::write_zeros(&mut writer, PAGE_SIZE.into())?;
+        padding::write_zeros(&mut writer, PAGE_SIZE.into())
+            .map_err(|e| Error::DataWrite("Boot::V3::header_padding", e))?;
 
-        writer.write_all(&self.kernel)?;
-        padding::write_zeros(&mut writer, PAGE_SIZE.into())?;
+        writer
+            .write_all(&self.kernel)
+            .map_err(|e| Error::DataWrite("Boot::V3::kernel", e))?;
+        padding::write_zeros(&mut writer, PAGE_SIZE.into())
+            .map_err(|e| Error::DataWrite("Boot::V3::kernel_padding", e))?;
 
-        writer.write_all(&self.ramdisk)?;
-        padding::write_zeros(&mut writer, PAGE_SIZE.into())?;
+        writer
+            .write_all(&self.ramdisk)
+            .map_err(|e| Error::DataWrite("Boot::V3::ramdisk", e))?;
+        padding::write_zeros(&mut writer, PAGE_SIZE.into())
+            .map_err(|e| Error::DataWrite("Boot::V3::ramdisk_padding", e))?;
 
         if !skip_v4_sig {
             if let Some(sig) = v4_signature {
-                writer.write_all(&sig)?;
-                padding::write_zeros(&mut writer, PAGE_SIZE.into())?;
+                writer
+                    .write_all(&sig)
+                    .map_err(|e| Error::DataWrite("Boot::V4::signature", e))?;
+                padding::write_zeros(&mut writer, PAGE_SIZE.into())
+                    .map_err(|e| Error::DataWrite("Boot::V4::signature_padding", e))?;
             }
         }
 
@@ -823,10 +906,9 @@ impl BootImageV3Through4 {
                     .ok_or(Error::MissingHashDescriptor)?;
 
                 if descriptor.hash_algorithm != "sha256" {
-                    return Err(avb::Error::UnsupportedHashAlgorithm(
+                    return Err(Error::VtsAvbSign(avb::Error::UnsupportedHashAlgorithm(
                         descriptor.hash_algorithm.clone(),
-                    )
-                    .into());
+                    )));
                 }
 
                 context.update(&descriptor.salt);
@@ -863,7 +945,7 @@ impl BootImageV3Through4 {
 
         descriptor.image_size = image_size;
         descriptor.root_digest = context.finish().as_ref().to_vec();
-        signature.sign(key)?;
+        signature.sign(key).map_err(Error::VtsAvbSign)?;
 
         Ok(true)
     }
@@ -1036,7 +1118,8 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
     fn from_reader(reader: R) -> Result<Self> {
         let mut reader = CountingReader::new(reader);
 
-        let raw_v3 = RawVendorV3::read_from_io(&mut reader)?;
+        let raw_v3 = RawVendorV3::read_from_io(&mut reader)
+            .map_err(|e| Error::DataRead("Vendor::V3::header", e))?;
 
         if raw_v3.magic != VENDOR_BOOT_MAGIC {
             return Err(Error::UnknownMagic(raw_v3.magic));
@@ -1076,7 +1159,8 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
         }
 
         let mut v4_data = if header_version == 4 {
-            let raw_v4 = RawVendorV4Extra::read_from_io(&mut reader)?;
+            let raw_v4 = RawVendorV4Extra::read_from_io(&mut reader)
+                .map_err(|e| Error::DataRead("Vendor::V4::header", e))?;
 
             let table_size = raw_v4.vendor_ramdisk_table_size.get();
             let table_entry_num = raw_v4.vendor_ramdisk_table_entry_num.get();
@@ -1092,7 +1176,7 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
 
             let actual_table_size = table_entry_num
                 .checked_mul(table_entry_size)
-                .ok_or_else(|| Error::IntOverflow("Vendor::V4::actual_table_size"))?;
+                .ok_or(Error::IntOverflow("Vendor::V4::actual_table_size"))?;
             if actual_table_size != table_size {
                 return Err(Error::VendorV4InvalidRamdiskTableSize {
                     actual: actual_table_size,
@@ -1112,16 +1196,24 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
             None
         };
 
-        if reader.stream_position()? != u64::from(header_size) {
+        if reader
+            .stream_position()
+            .map_err(|e| Error::DataRead("Vendor::V3::header_size", e))?
+            != u64::from(header_size)
+        {
             return Err(Error::InvalidHeaderSize(header_size));
         }
 
-        padding::read_discard(&mut reader, page_size.into())?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Vendor::V3::header_padding", e))?;
 
         let mut ramdisks = vec![];
 
-        let mut vendor_ramdisk_data = reader.read_vec_exact(vendor_ramdisk_size as usize)?;
-        padding::read_discard(&mut reader, page_size.into())?;
+        let mut vendor_ramdisk_data = reader
+            .read_vec_exact(vendor_ramdisk_size as usize)
+            .map_err(|e| Error::DataRead("Vendor::V3::ramdisk", e))?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Vendor::V3::ramdisk_padding", e))?;
 
         // For v3, this is just one big ramdisk. For v4, we have to wait until
         // later to parse the data because the table of entries shows up later
@@ -1131,28 +1223,34 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
             vendor_ramdisk_data = vec![];
         }
 
-        let dtb = reader.read_vec_exact(dtb_size as usize)?;
-        padding::read_discard(&mut reader, page_size.into())?;
+        let dtb = reader
+            .read_vec_exact(dtb_size as usize)
+            .map_err(|e| Error::DataRead("Vendor::V3::dtb", e))?;
+        padding::read_discard(&mut reader, page_size.into())
+            .map_err(|e| Error::DataRead("Vendor::V3::dtb_padding", e))?;
 
         if let Some(v4) = &mut v4_data {
             let mut ramdisk_reader = Cursor::new(vendor_ramdisk_data);
             let mut total_ramdisk_size = 0;
 
             for index in 0..v4.vendor_ramdisk_table_entry_num {
-                let raw_entry = RawVendorV4RamdiskTableEntry::read_from_io(&mut reader)?;
+                let raw_entry = RawVendorV4RamdiskTableEntry::read_from_io(&mut reader)
+                    .map_err(|e| Error::DataRead("Vendor::V4::table_entry", e))?;
 
                 let ramdisk_size =
                     util::check_bounds(raw_entry.ramdisk_size.get(), ..=vendor_ramdisk_size)
-                        .map_err(|e| Error::IntOutOfBounds("Vendor::V4::Meta::ramdisk_size", e))?;
+                        .map_err(|e| Error::IntOutOfBounds("Vendor::V4::ramdisk_size", e))?;
 
                 let ramdisk_offset = raw_entry.ramdisk_offset.get();
 
                 let ramdisk_name = raw_entry.ramdisk_name.trim_end_padding();
                 let ramdisk_name = str::from_utf8(ramdisk_name).map_err(|e| {
-                    Error::StringNotUtf8("Vendor::V4::Meta::ramdisk_name", e, ramdisk_name.to_vec())
+                    Error::StringNotUtf8("Vendor::V4::ramdisk_name", e, ramdisk_name.to_vec())
                 })?;
 
-                let table_offset = ramdisk_reader.stream_position()?;
+                let table_offset = ramdisk_reader
+                    .stream_position()
+                    .map_err(|e| Error::DataRead("Vendor::V4::table_offset", e))?;
 
                 if u64::from(ramdisk_offset) != table_offset {
                     return Err(Error::VendorV4InvalidRamdiskEntryOffset {
@@ -1162,7 +1260,9 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
                     });
                 }
 
-                let ramdisk = ramdisk_reader.read_vec_exact(ramdisk_size as usize)?;
+                let ramdisk = ramdisk_reader
+                    .read_vec_exact(ramdisk_size as usize)
+                    .map_err(|e| Error::DataRead("Vendor::V4::ramdisk", e))?;
                 ramdisks.push(ramdisk);
 
                 v4.v4_extra.ramdisk_metas.push(RamdiskMeta {
@@ -1181,9 +1281,15 @@ impl<R: Read> FromReader<R> for VendorBootImageV3Through4 {
                 });
             }
 
-            padding::read_discard(&mut reader, page_size.into())?;
+            padding::read_discard(&mut reader, page_size.into())
+                .map_err(|e| Error::DataRead("Vendor::V4::table_padding", e))?;
+
+            let bootconfig = reader
+                .read_vec_exact(v4.bootconfig_size as usize)
+                .map_err(|e| Error::DataRead("Vendor::V4::bootconfig", e))?;
+            padding::read_discard(&mut reader, page_size.into())
+                .map_err(|e| Error::DataRead("Vendor::V4::bootconfig_padding", e))?;
 
-            let bootconfig = reader.read_vec_exact(v4.bootconfig_size as usize)?;
             v4.v4_extra.bootconfig = String::from_utf8(bootconfig).map_err(|e| {
                 Error::StringNotUtf8("Vendor::V4::bootconfig", e.utf8_error(), e.into_bytes())
             })?;
@@ -1274,18 +1380,19 @@ impl<W: Write> ToWriter<W> for VendorBootImageV3Through4 {
             dtb_addr: self.dtb_addr.into(),
         };
 
-        raw_v3.write_to_io(&mut writer)?;
+        raw_v3
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("Vendor::V3::header", e))?;
 
         if let Some(v4) = &self.v4_extra {
-            let table_entry_num =
-                self.ramdisks.len().to_u32().ok_or_else(|| {
-                    Error::IntOverflow("Vendor::V4::vendor_ramdisk_table_entry_num")
-                })?;
+            let table_entry_num = self.ramdisks.len().to_u32().ok_or(Error::IntOverflow(
+                "Vendor::V4::vendor_ramdisk_table_entry_num",
+            ))?;
             let table_entry_size = mem::size_of::<RawVendorV4RamdiskTableEntry>() as u32;
             let table_size = table_entry_num
                 .checked_mul(table_entry_size)
                 .and_then(|v| v.to_u32())
-                .ok_or_else(|| Error::IntOverflow("Vendor::V4::vendor_ramdisk_table_size"))?;
+                .ok_or(Error::IntOverflow("Vendor::V4::vendor_ramdisk_table_size"))?;
 
             let raw_v4 = RawVendorV4Extra {
                 vendor_ramdisk_table_size: table_size.into(),
@@ -1294,18 +1401,27 @@ impl<W: Write> ToWriter<W> for VendorBootImageV3Through4 {
                 bootconfig_size: (v4.bootconfig.len() as u32).into(),
             };
 
-            raw_v4.write_to_io(&mut writer)?;
+            raw_v4
+                .write_to_io(&mut writer)
+                .map_err(|e| Error::DataWrite("Vendor::V4::header", e))?;
         }
 
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Vendor::V3::header_padding", e))?;
 
         for ramdisk in &self.ramdisks {
-            writer.write_all(ramdisk)?;
+            writer
+                .write_all(ramdisk)
+                .map_err(|e| Error::DataWrite("Vendor::V3::ramdisk", e))?;
         }
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Vendor::V3::ramdisk_padding", e))?;
 
-        writer.write_all(&self.dtb)?;
-        padding::write_zeros(&mut writer, self.page_size.into())?;
+        writer
+            .write_all(&self.dtb)
+            .map_err(|e| Error::DataWrite("Vendor::V3::dtb", e))?;
+        padding::write_zeros(&mut writer, self.page_size.into())
+            .map_err(|e| Error::DataWrite("Vendor::V3::dtb_padding", e))?;
 
         if let Some(v4) = &self.v4_extra {
             let mut ramdisk_offset = 0;
@@ -1319,7 +1435,7 @@ impl<W: Write> ToWriter<W> for VendorBootImageV3Through4 {
                     .to_padded_array::<VENDOR_RAMDISK_NAME_SIZE>()
                     .ok_or_else(|| {
                         Error::StringTooLong(
-                            "Vendor::V4::Meta::ramdisk_name",
+                            "Vendor::V4::ramdisk_name",
                             VENDOR_RAMDISK_NAME_SIZE,
                             meta.ramdisk_name.clone(),
                         )
@@ -1333,14 +1449,20 @@ impl<W: Write> ToWriter<W> for VendorBootImageV3Through4 {
                     board_id: meta.board_id.map(|id| id.into()),
                 };
 
-                raw_entry.write_to_io(&mut writer)?;
+                raw_entry
+                    .write_to_io(&mut writer)
+                    .map_err(|e| Error::DataWrite("Vendor::V4::table_entry", e))?;
 
                 ramdisk_offset += ramdisk_size;
             }
-            padding::write_zeros(&mut writer, self.page_size.into())?;
+            padding::write_zeros(&mut writer, self.page_size.into())
+                .map_err(|e| Error::DataWrite("Vendor::V4::table_padding", e))?;
 
-            writer.write_all(v4.bootconfig.as_bytes())?;
-            padding::write_zeros(&mut writer, self.page_size.into())?;
+            writer
+                .write_all(v4.bootconfig.as_bytes())
+                .map_err(|e| Error::DataWrite("Vendor::V4::bootconfig", e))?;
+            padding::write_zeros(&mut writer, self.page_size.into())
+                .map_err(|e| Error::DataWrite("Vendor::V4::bootconfig_padding", e))?;
         }
 
         Ok(())
@@ -1387,7 +1509,9 @@ impl<R: Read + Seek> FromReader<R> for BootImage {
     type Error = Error;
 
     fn from_reader(mut reader: R) -> Result<Self> {
-        reader.rewind()?;
+        reader
+            .rewind()
+            .map_err(|e| Error::DataRead("Boot::V0::autodetect", e))?;
 
         match BootImageV0Through2::from_reader(&mut reader) {
             Ok(b) => return Ok(Self::V0Through2(b)),
@@ -1395,7 +1519,9 @@ impl<R: Read + Seek> FromReader<R> for BootImage {
             Err(e) => return Err(e),
         }
 
-        reader.rewind()?;
+        reader
+            .rewind()
+            .map_err(|e| Error::DataRead("Boot::V3::autodetect", e))?;
 
         match BootImageV3Through4::from_reader(&mut reader) {
             Ok(b) => return Ok(Self::V3Through4(b)),
@@ -1403,7 +1529,9 @@ impl<R: Read + Seek> FromReader<R> for BootImage {
             Err(e) => return Err(e),
         }
 
-        reader.rewind()?;
+        reader
+            .rewind()
+            .map_err(|e| Error::DataRead("Vendor::V3::autodetect", e))?;
 
         match VendorBootImageV3Through4::from_reader(&mut reader) {
             Ok(b) => return Ok(Self::VendorV3Through4(b)),

avbroot/src/format/compression.rs

@@ -13,6 +13,8 @@ use lz4_flex::frame::FrameDecoder;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
+use crate::stream::ReadFixedSizeExt;
+
 static GZIP_MAGIC: &[u8; 2] = b"\x1f\x8b";
 static LZ4_LEGACY_MAGIC: &[u8; 4] = b"\x02\x21\x4c\x18";
 static XZ_MAGIC: &[u8; 6] = b"\xfd\x37\x7a\x58\x5a\x00";
@@ -21,10 +23,12 @@ static XZ_MAGIC: &[u8; 6] = b"\xfd\x37\x7a\x58\x5a\x00";
 pub enum Error {
     #[error("Unknown compression format")]
     UnknownFormat,
-    #[error("XZ stream error")]
-    XzStream(#[from] liblzma::stream::Error),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("I/O error when autodetecting compression format")]
+    AutoDetect(#[source] io::Error),
+    #[error("Failed to initialize legacy LZ4 encoder")]
+    Lz4Init(#[source] io::Error),
+    #[error("Failed to initialize XZ encoder")]
+    XzInit(#[source] liblzma::stream::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -119,10 +123,9 @@ pub enum CompressedReader<R: Read> {
 
 impl<R: Read + Seek> CompressedReader<R> {
     pub fn new(mut reader: R, raw_if_unknown: bool) -> Result<Self> {
-        let mut magic = [0u8; 6];
-        reader.read_exact(&mut magic)?;
+        let magic = reader.read_array_exact::<6>().map_err(Error::AutoDetect)?;
 
-        reader.rewind()?;
+        reader.rewind().map_err(Error::AutoDetect)?;
 
         if &magic[0..2] == GZIP_MAGIC {
             Ok(Self::Gzip(GzDecoder::new(reader)))
@@ -181,10 +184,13 @@ impl<W: Write> CompressedWriter<W> {
             CompressedFormat::Gzip => {
                 Ok(Self::Gzip(GzEncoder::new(writer, Compression::default())))
             }
-            CompressedFormat::Lz4Legacy => Ok(Self::Lz4Legacy(Lz4LegacyEncoder::new(writer)?)),
+            CompressedFormat::Lz4Legacy => {
+                let encoder = Lz4LegacyEncoder::new(writer).map_err(Error::Lz4Init)?;
+                Ok(Self::Lz4Legacy(encoder))
+            }
             CompressedFormat::Xz => {
                 // Some kernels are compiled without support for the default CRC64.
-                let stream = Stream::new_easy_encoder(6, Check::Crc32)?;
+                let stream = Stream::new_easy_encoder(6, Check::Crc32).map_err(Error::XzInit)?;
                 Ok(Self::Xz(XzEncoder::new_stream(writer, stream)))
             }
         }

avbroot/src/format/cpio.rs

@@ -13,6 +13,8 @@ use bstr::ByteSlice;
 use num_traits::{ToPrimitive, Zero};
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
+use zerocopy::{FromBytes, IntoBytes};
+use zerocopy_derive::{FromBytes, Immutable, IntoBytes, KnownLayout, Unaligned};
 
 use crate::{
     escape,
@@ -48,55 +50,94 @@ const VEC_CAP_THRESHOLD: usize = 16384;
 pub enum Error {
     #[error("Unknown magic: {0:?}")]
     UnknownMagic([u8; 6]),
+    #[error("Path is not NULL-terminated: {:?}", .0.as_bstr())]
+    PathNotNullTerminated(Vec<u8>),
     #[error("Hard links are not supported: {:?}", .0.as_bstr())]
     HardLinksNotSupported(Vec<u8>),
     #[error("Entry of type {0} should not have data: {path:?}", path = .1.as_bstr())]
     EntryHasData(CpioEntryType, Vec<u8>),
-    #[error("No inodes available for device {0:x},{1:x}")]
-    DeviceFull(u32, u32),
+    #[error("No inodes available for device {major:x},{minor:x}")]
+    DeviceFull { major: u32, minor: u32 },
     #[error("{0:?} overflowed integer bounds during calculations")]
     IntOverflow(&'static str),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("{0:?} contains invalid hex integer")]
+    InvalidHexInt(&'static str, #[source] InvalidHexCharError),
+    #[error("Failed to read cpio data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write cpio data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
 
-/// Read u32 formatted as an ASCII 8-char wide hex string.
-fn read_int(mut reader: impl Read) -> io::Result<u32> {
-    let mut buf = [0u8; 8];
-    reader.read_exact(&mut buf)?;
+#[derive(Debug, Error)]
+#[error("{0:?}: Invalid hex char: {1:?}")]
+pub struct InvalidHexCharError(RawHexU32, char);
 
-    let mut value = 0;
+/// ASCII-encoded hex integer value used in cpio header fields.
+#[derive(Clone, Copy, FromBytes, IntoBytes, KnownLayout, Immutable, Unaligned)]
+#[repr(packed)]
+struct RawHexU32([u8; 8]);
 
-    for b in buf {
-        let c = b as char;
-        let digit = c.to_digit(16).ok_or_else(|| {
-            io::Error::new(
-                io::ErrorKind::InvalidData,
-                format!("{:?}: Invalid hex char: {c}", buf.as_bstr()),
-            )
-        })?;
-
-        value <<= 4;
-        value |= digit;
+impl fmt::Debug for RawHexU32 {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:?}", self.0.as_bstr())
     }
-
-    Ok(value)
 }
 
-/// Write u32 formatted as an ASCII 8-char wide hex string.
-fn write_int(mut writer: impl Write, mut value: u32) -> io::Result<()> {
-    let mut buf = [b'0'; 8];
-    let mut index = 7;
+#[allow(clippy::fallible_impl_from)]
+impl From<u32> for RawHexU32 {
+    fn from(mut value: u32) -> Self {
+        let mut buf = [b'0'; 8];
+        let mut index = 7;
 
-    while value != 0 {
-        buf[index] = char::from_digit(value & 0xf, 16).unwrap() as u8;
-        value >>= 4;
-        index -= 1;
+        while value != 0 {
+            buf[index] = char::from_digit(value & 0xf, 16).unwrap() as u8;
+            value >>= 4;
+            index -= 1;
+        }
+
+        Self(buf)
     }
+}
 
-    writer.write_all(&buf)
+impl TryFrom<RawHexU32> for u32 {
+    type Error = InvalidHexCharError;
+
+    fn try_from(raw_value: RawHexU32) -> std::result::Result<Self, Self::Error> {
+        let mut value = 0;
+
+        for b in raw_value.0 {
+            let c = b as char;
+            let digit = c.to_digit(16).ok_or(InvalidHexCharError(raw_value, c))?;
+
+            value <<= 4;
+            value |= digit;
+        }
+
+        Ok(value)
+    }
+}
+
+/// Raw on-disk layout for the cpio header.
+#[derive(Clone, Copy, FromBytes, IntoBytes, KnownLayout, Immutable, Unaligned)]
+#[repr(packed)]
+struct RawHeader {
+    /// Magic value. This should be equal to [`MAGIC_NEW`] or [`MAGIC_NEW_CRC`].
+    magic: [u8; 6],
+    inode: RawHexU32,
+    mode: RawHexU32,
+    uid: RawHexU32,
+    gid: RawHexU32,
+    nlink: RawHexU32,
+    mtime: RawHexU32,
+    file_size: RawHexU32,
+    dev_maj: RawHexU32,
+    dev_min: RawHexU32,
+    rdev_maj: RawHexU32,
+    rdev_min: RawHexU32,
+    path_size: RawHexU32,
+    crc32: RawHexU32,
 }
 
 /// Read a chunk of bytes from the reader. If `size` is less than
@@ -386,41 +427,45 @@ impl<R: Read> FromReader<R> for CpioEntry {
     fn from_reader(reader: R) -> Result<Self> {
         let mut reader = CountingReader::new(reader);
 
-        let mut magic = [0u8; 6];
-        reader.read_exact(&mut magic)?;
+        let header =
+            RawHeader::read_from_io(&mut reader).map_err(|e| Error::DataRead("header", e))?;
 
-        if magic != *MAGIC_NEW && magic != *MAGIC_NEW_CRC {
-            return Err(Error::UnknownMagic(magic));
+        if header.magic != *MAGIC_NEW && header.magic != *MAGIC_NEW_CRC {
+            return Err(Error::UnknownMagic(header.magic));
         }
 
-        let inode = read_int(&mut reader)?;
-        let mode = read_int(&mut reader)?;
-        let uid = read_int(&mut reader)?;
-        let gid = read_int(&mut reader)?;
-        let nlink = read_int(&mut reader)?;
-        let mtime = read_int(&mut reader)?;
-        let file_size = read_int(&mut reader)?;
-        let dev_maj = read_int(&mut reader)?;
-        let dev_min = read_int(&mut reader)?;
-        let rdev_maj = read_int(&mut reader)?;
-        let rdev_min = read_int(&mut reader)?;
-        let path_size = read_int(&mut reader)?;
-        let crc32 = read_int(&mut reader)?;
+        macro_rules! get_field {
+            ($name:ident) => {
+                let $name = u32::try_from(header.$name)
+                    .map_err(|e| Error::InvalidHexInt(stringify!($name), e))?;
+            };
+        }
+
+        get_field!(inode);
+        get_field!(mode);
+        get_field!(uid);
+        get_field!(gid);
+        get_field!(nlink);
+        get_field!(mtime);
+        get_field!(file_size);
+        get_field!(dev_maj);
+        get_field!(dev_min);
+        get_field!(rdev_maj);
+        get_field!(rdev_min);
+        get_field!(path_size);
+        get_field!(crc32);
 
         let mut path = read_data(
             &mut reader,
             path_size.to_usize().unwrap(),
             &AtomicBool::new(false),
-        )?;
+        )
+        .map_err(|e| Error::DataRead("path", e))?;
         if path.last() != Some(&b'\0') {
-            return Err(io::Error::new(
-                io::ErrorKind::InvalidData,
-                "Filename is not NULL-terminated",
-            )
-            .into());
+            return Err(Error::PathNotNullTerminated(path));
         }
         path.pop();
-        padding::read_discard(&mut reader, 4)?;
+        padding::read_discard(&mut reader, 4).map_err(|e| Error::DataRead("path_padding", e))?;
 
         let file_type = CpioEntryType::from_mode(mode);
         let data = match file_type {
@@ -432,8 +477,10 @@ impl<R: Read> FromReader<R> for CpioEntry {
                     &mut reader,
                     file_size.to_usize().unwrap(),
                     &AtomicBool::new(false),
-                )?;
-                padding::read_discard(&mut reader, 4)?;
+                )
+                .map_err(|e| Error::DataRead("content", e))?;
+                padding::read_discard(&mut reader, 4)
+                    .map_err(|e| Error::DataRead("content_padding", e))?;
 
                 CpioEntryData::Data(content)
             }
@@ -482,35 +529,47 @@ impl<W: Write> ToWriter<W> for CpioEntry {
             return Err(Error::EntryHasData(self.file_type, self.path.clone()));
         }
 
-        if self.crc32 == 0 {
-            writer.write_all(MAGIC_NEW)?;
-        } else {
-            writer.write_all(MAGIC_NEW_CRC)?;
-        }
-
         let mode = self.file_type.to_mode() | u32::from(self.file_mode & 0o7777);
 
-        write_int(&mut writer, self.inode)?;
-        write_int(&mut writer, mode)?;
-        write_int(&mut writer, self.uid)?;
-        write_int(&mut writer, self.gid)?;
-        write_int(&mut writer, self.nlink)?;
-        write_int(&mut writer, self.mtime)?;
-        write_int(&mut writer, file_size)?;
-        write_int(&mut writer, self.dev_maj)?;
-        write_int(&mut writer, self.dev_min)?;
-        write_int(&mut writer, self.rdev_maj)?;
-        write_int(&mut writer, self.rdev_min)?;
-        write_int(&mut writer, path_size)?;
-        write_int(&mut writer, self.crc32)?;
+        let raw_header = RawHeader {
+            magic: if self.crc32 == 0 {
+                *MAGIC_NEW
+            } else {
+                *MAGIC_NEW_CRC
+            },
+            inode: self.inode.into(),
+            mode: mode.into(),
+            uid: self.uid.into(),
+            gid: self.gid.into(),
+            nlink: self.nlink.into(),
+            mtime: self.mtime.into(),
+            file_size: file_size.into(),
+            dev_maj: self.dev_maj.into(),
+            dev_min: self.dev_min.into(),
+            rdev_maj: self.rdev_maj.into(),
+            rdev_min: self.rdev_min.into(),
+            path_size: path_size.into(),
+            crc32: self.crc32.into(),
+        };
 
-        writer.write_all(&self.path)?;
-        writer.write_zeros_exact(1)?;
-        padding::write_zeros(&mut writer, 4)?;
+        raw_header
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("header", e))?;
+
+        writer
+            .write_all(&self.path)
+            .map_err(|e| Error::DataWrite("path", e))?;
+        writer
+            .write_zeros_exact(1)
+            .map_err(|e| Error::DataWrite("path", e))?;
+        padding::write_zeros(&mut writer, 4).map_err(|e| Error::DataWrite("path_padding", e))?;
 
         if let CpioEntryData::Data(d) = &self.data {
-            writer.write_all(d)?;
-            padding::write_zeros(&mut writer, 4)?;
+            writer
+                .write_all(d)
+                .map_err(|e| Error::DataWrite("content", e))?;
+            padding::write_zeros(&mut writer, 4)
+                .map_err(|e| Error::DataWrite("content_padding", e))?;
         }
 
         Ok(())
@@ -554,7 +613,8 @@ impl<R: Read> CpioReader<R> {
             return Ok(None);
         }
 
-        self.skip_data()?;
+        self.skip_data()
+            .map_err(|e| Error::DataRead("content", e))?;
 
         let entry = CpioEntry::from_reader(&mut self.reader)?;
 
@@ -619,7 +679,8 @@ impl<W: Write> CpioWriter<W> {
     }
 
     pub fn start_entry(&mut self, entry: &CpioEntry) -> Result<()> {
-        self.finish_entry()?;
+        self.finish_entry()
+            .map_err(|e| Error::DataWrite("content", e))?;
 
         entry.to_writer(&mut self.writer)?;
 
@@ -633,13 +694,15 @@ impl<W: Write> CpioWriter<W> {
     }
 
     pub fn finish(mut self) -> Result<W> {
-        self.finish_entry()?;
+        self.finish_entry()
+            .map_err(|e| Error::DataWrite("content", e))?;
 
         self.start_entry(&CpioEntry::new_trailer())?;
 
         // Pad until the end of the block.
         if self.pad_to_block_size {
-            padding::write_zeros(&mut self.writer, IO_BLOCK_SIZE)?;
+            padding::write_zeros(&mut self.writer, IO_BLOCK_SIZE)
+                .map_err(|e| Error::DataWrite("block_padding", e))?;
         }
 
         Ok(self.writer.finish().0)
@@ -677,14 +740,15 @@ pub fn load(
     let mut entries = vec![];
 
     while let Some(mut entry) = cpio_reader.next_entry()? {
-        stream::check_cancel(cancel_signal)?;
+        stream::check_cancel(cancel_signal).map_err(|e| Error::DataRead("entry", e))?;
 
         if entry.file_type != CpioEntryType::Directory && entry.nlink > 1 {
             return Err(Error::HardLinksNotSupported(entry.path));
         }
 
         if let CpioEntryData::Size(s) = entry.data {
-            let data = read_data(&mut cpio_reader, s.to_usize().unwrap(), cancel_signal)?;
+            let data = read_data(&mut cpio_reader, s.to_usize().unwrap(), cancel_signal)
+                .map_err(|e| Error::DataWrite("data", e))?;
             entry.data = CpioEntryData::Data(data);
         }
 
@@ -740,7 +804,10 @@ pub fn assign_inodes(entries: &mut [CpioEntry], missing_only: bool) -> Result<()
 
             while set.contains(&unused) {
                 if unused == *last {
-                    return Err(Error::DeviceFull(entry.dev_maj, entry.dev_min));
+                    return Err(Error::DeviceFull {
+                        major: entry.dev_maj,
+                        minor: entry.dev_min,
+                    });
                 }
 
                 unused = next_non_zero(unused);
@@ -764,7 +831,7 @@ pub fn save(
     let mut cpio_writer = CpioWriter::new(writer, pad_to_block_size);
 
     for entry in entries {
-        stream::check_cancel(cancel_signal)?;
+        stream::check_cancel(cancel_signal).map_err(|e| Error::DataWrite("entry", e))?;
 
         cpio_writer.start_entry(entry)?;
         // CpioEntryData::Data will have already been written.

avbroot/src/format/fec.rs

@@ -68,8 +68,14 @@ pub enum Error {
     IntOutOfBounds(&'static str, #[source] OutOfBoundsError),
     #[error("{0:?} overflowed integer bounds during calculations")]
     IntOverflow(&'static str),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to reopen input file")]
+    InputReopen(#[source] io::Error),
+    #[error("Failed to reopen output file")]
+    OutputReopen(#[source] io::Error),
+    #[error("Failed to read FEC data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write FEC data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -364,7 +370,9 @@ impl Fec {
             "FEC buffer length does not match block size",
         );
 
-        let grid = self.read_round(reader, round)?;
+        let grid = self
+            .read_round(reader, round)
+            .map_err(|e| Error::DataRead("round", e))?;
         let encode = verityrs::FN_ENCODE[&self.rs_k];
         let parity = usize::from(self.parity());
 
@@ -385,7 +393,9 @@ impl Fec {
             "FEC buffer length does not match block size",
         );
 
-        let grid = self.read_round(reader, round)?;
+        let grid = self
+            .read_round(reader, round)
+            .map_err(|e| Error::DataRead("round", e))?;
         let is_correct = verityrs::FN_IS_CORRECT[&self.rs_k];
         let parity = usize::from(self.parity());
 
@@ -415,7 +425,9 @@ impl Fec {
             "FEC buffer length does not match block size",
         );
 
-        let mut grid = self.read_round(reader, round)?;
+        let mut grid = self
+            .read_round(reader, round)
+            .map_err(|e| Error::DataRead("round", e))?;
         let correct_errors = verityrs::FN_CORRECT_ERRORS[&self.rs_k];
         let parity = usize::from(self.parity());
         let mut num_corrected = 0;
@@ -433,7 +445,8 @@ impl Fec {
         }
 
         if num_corrected > 0 {
-            self.write_round(writer, round, &grid)?;
+            self.write_round(writer, round, &grid)
+                .map_err(|e| Error::DataWrite("round", e))?;
         }
 
         Ok(num_corrected)
@@ -454,9 +467,9 @@ impl Fec {
         fec.par_chunks_exact_mut(fec_size / self.rounds as usize)
             .enumerate()
             .map(|(round, buf)| -> Result<()> {
-                stream::check_cancel(cancel_signal)?;
+                stream::check_cancel(cancel_signal).map_err(Error::InputReopen)?;
 
-                let reader = input.reopen_boxed()?;
+                let reader = input.reopen_boxed().map_err(Error::InputReopen)?;
                 self.generate_one_round(reader, round as u64, buf)
             })
             .collect::<Result<()>>()?;
@@ -489,9 +502,9 @@ impl Fec {
             .enumerate()
             .filter(|(round, _)| rounds_to_update.contains(&(*round as u64)))
             .map(|(round, buf)| -> Result<()> {
-                stream::check_cancel(cancel_signal)?;
+                stream::check_cancel(cancel_signal).map_err(Error::InputReopen)?;
 
-                let reader = input.reopen_boxed()?;
+                let reader = input.reopen_boxed().map_err(Error::InputReopen)?;
                 self.generate_one_round(reader, round as u64, buf)
             })
             .collect::<Result<()>>()?;
@@ -522,9 +535,9 @@ impl Fec {
         fec.par_chunks_exact(fec_size / self.rounds as usize)
             .enumerate()
             .map(|(round, buf)| -> Result<()> {
-                stream::check_cancel(cancel_signal)?;
+                stream::check_cancel(cancel_signal).map_err(Error::InputReopen)?;
 
-                let reader = input.reopen_boxed()?;
+                let reader = input.reopen_boxed().map_err(Error::InputReopen)?;
                 self.verify_one_round(reader, round as u64, buf)
             })
             .collect::<Result<()>>()?;
@@ -563,10 +576,10 @@ impl Fec {
             .par_chunks_exact(fec_size / self.rounds as usize)
             .enumerate()
             .map(|(round, buf)| -> Result<u64> {
-                stream::check_cancel(cancel_signal)?;
+                stream::check_cancel(cancel_signal).map_err(Error::InputReopen)?;
 
-                let reader = input.reopen_boxed()?;
-                let writer = output.reopen_boxed()?;
+                let reader = input.reopen_boxed().map_err(Error::InputReopen)?;
+                let writer = output.reopen_boxed().map_err(Error::OutputReopen)?;
                 self.repair_one_round(reader, writer, round as u64, buf)
             })
             .collect::<Result<Vec<u64>>>()?
@@ -627,10 +640,10 @@ impl FecImage {
         parity: u8,
         cancel_signal: &AtomicBool,
     ) -> Result<Self> {
-        let data_size = {
-            let mut file = input.reopen_boxed()?;
-            file.seek(SeekFrom::End(0))?
-        };
+        let data_size = input
+            .reopen_boxed()
+            .and_then(|mut f| f.seek(SeekFrom::End(0)))
+            .map_err(Error::InputReopen)?;
         let fec = Fec::new(data_size, FEC_BLOCK_SIZE as u32, parity)?;
         let fec_data = fec.generate(input, cancel_signal)?;
 
@@ -717,7 +730,9 @@ impl<R: Read> FromReader<R> for FecImage {
         // Avoid requiring seekable readers since we need to read everything
         // into memory anyway.
         let mut fec = Vec::new();
-        reader.read_to_end(&mut fec)?;
+        reader
+            .read_to_end(&mut fec)
+            .map_err(|e| Error::DataRead("fec", e))?;
 
         if fec.len() < FEC_BLOCK_SIZE {
             return Err(Error::DataTooSmall);
@@ -791,10 +806,18 @@ impl<W: Write> ToWriter<W> for FecImage {
     fn to_writer(&self, mut writer: W) -> Result<()> {
         let header = self.build_header()?;
 
-        writer.write_all(&self.fec)?;
-        header.write_to_io(&mut writer)?;
-        writer.write_zeros_exact((FEC_BLOCK_SIZE - 2 * header.as_bytes().len()) as u64)?;
-        header.write_to_io(&mut writer)?;
+        writer
+            .write_all(&self.fec)
+            .map_err(|e| Error::DataWrite("fec_data", e))?;
+        header
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("fec_header_1", e))?;
+        writer
+            .write_zeros_exact((FEC_BLOCK_SIZE - 2 * header.as_bytes().len()) as u64)
+            .map_err(|e| Error::DataWrite("fec_header_padding", e))?;
+        header
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("fec_header_2", e))?;
 
         Ok(())
     }

avbroot/src/format/hashtree.rs

@@ -24,7 +24,7 @@ use crate::{
         avb,
         padding::{self, ZeroPadding},
     },
-    stream::{self, FromReader, ReadSeekReopen, ToWriter},
+    stream::{self, FromReader, ReadFixedSizeExt, ReadSeekReopen, ToWriter},
     util::{self, NumBytes, OutOfBoundsError},
 };
 
@@ -50,8 +50,14 @@ pub enum Error {
     IntOutOfBounds(&'static str, #[source] OutOfBoundsError),
     #[error("{0:?} overflowed integer bounds during calculations")]
     IntOverflow(&'static str),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to reopen input file")]
+    InputReopen(#[source] io::Error),
+    #[error("Failed to compute hash tree of input file")]
+    InputDigest(#[source] io::Error),
+    #[error("Failed to read hash tree data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write hash tree data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -264,9 +270,10 @@ impl HashTree {
     ) -> Result<Vec<u8>> {
         // Small files are hashed directly.
         if image_size <= u64::from(self.block_size) {
-            let mut reader = input.reopen_boxed()?;
-            let mut buf = vec![0u8; image_size as usize];
-            reader.read_exact(&mut buf)?;
+            let mut reader = input.reopen_boxed().map_err(Error::InputReopen)?;
+            let buf = reader
+                .read_vec_exact(image_size as usize)
+                .map_err(Error::InputDigest)?;
 
             let mut context = self.salted_context.clone();
             context.update(&buf);
@@ -291,7 +298,8 @@ impl HashTree {
                     prev_size as u64,
                     level_data,
                     cancel_signal,
-                )?;
+                )
+                .map_err(Error::InputDigest)?;
             } else if let Some(r) = ranges {
                 // Read partial blocks from file.
                 let block_ranges = self.blocks_for_ranges(image_size, r)?;
@@ -302,10 +310,12 @@ impl HashTree {
                     &block_ranges,
                     level_data,
                     cancel_signal,
-                )?;
+                )
+                .map_err(Error::InputDigest)?;
             } else {
                 // Read entire file.
-                self.hash_one_level_parallel(input, image_size, level_data, cancel_signal)?;
+                self.hash_one_level_parallel(input, image_size, level_data, cancel_signal)
+                    .map_err(Error::InputDigest)?;
             }
 
             // No need to explicitly ensure the level is padded to the block
@@ -482,7 +492,7 @@ impl HashTreeImage {
     const MAGIC: &'static [u8; 16] = b"avbroot!hashtree";
     const VERSION: u16 = 1;
 
-    pub fn ring_algorithm(name: &str) -> Result<&'static Algorithm> {
+    fn ring_algorithm(name: &str) -> Result<&'static Algorithm> {
         avb::ring_algorithm(name, false)
             .map_err(|_| Error::UnsupportedHashAlgorithm(name.to_owned().into_bytes()))
     }
@@ -495,10 +505,10 @@ impl HashTreeImage {
         salt: &[u8],
         cancel_signal: &AtomicBool,
     ) -> Result<Self> {
-        let image_size = {
-            let mut file = input.reopen_boxed()?;
-            file.seek(SeekFrom::End(0))?
-        };
+        let image_size = input
+            .reopen_boxed()
+            .and_then(|mut f| f.seek(SeekFrom::End(0)))
+            .map_err(Error::InputReopen)?;
         let ring_algorithm = Self::ring_algorithm(algorithm)?;
         let hash_tree = HashTree::new(block_size, ring_algorithm, salt);
         let (root_digest, hash_tree_data) = hash_tree.generate(input, image_size, cancel_signal)?;
@@ -557,7 +567,8 @@ impl<R: Read> FromReader<R> for HashTreeImage {
     type Error = Error;
 
     fn from_reader(mut reader: R) -> Result<Self> {
-        let header = RawHeader::read_from_io(&mut reader)?;
+        let header =
+            RawHeader::read_from_io(&mut reader).map_err(|e| Error::DataRead("header", e))?;
 
         if header.magic != *Self::MAGIC {
             return Err(Error::InvalidHeaderMagic(header.magic));
@@ -571,14 +582,17 @@ impl<R: Read> FromReader<R> for HashTreeImage {
         let algorithm = str::from_utf8(algorithm)
             .map_err(|_| Error::UnsupportedHashAlgorithm(algorithm.to_vec()))?;
 
-        let mut salt = vec![0u8; usize::from(header.salt_size)];
-        reader.read_exact(&mut salt)?;
+        let salt = reader
+            .read_vec_exact(usize::from(header.salt_size))
+            .map_err(|e| Error::DataRead("header", e))?;
 
-        let mut root_digest = vec![0u8; usize::from(header.root_digest_size)];
-        reader.read_exact(&mut root_digest)?;
+        let root_digest = reader
+            .read_vec_exact(usize::from(header.root_digest_size))
+            .map_err(|e| Error::DataRead("root_digest", e))?;
 
-        let mut hash_tree = vec![0u8; header.hash_tree_size.get() as usize];
-        reader.read_exact(&mut hash_tree)?;
+        let hash_tree = reader
+            .read_vec_exact(header.hash_tree_size.get() as usize)
+            .map_err(|e| Error::DataRead("hash_tree", e))?;
 
         Ok(Self {
             image_size: header.image_size.get(),
@@ -619,10 +633,18 @@ impl<W: Write> ToWriter<W> for HashTreeImage {
             hash_tree_size: hash_tree_size.into(),
         };
 
-        header.write_to_io(&mut writer)?;
-        writer.write_all(&self.salt)?;
-        writer.write_all(&self.root_digest)?;
-        writer.write_all(&self.hash_tree)?;
+        header
+            .write_to_io(&mut writer)
+            .map_err(|e| Error::DataWrite("header", e))?;
+        writer
+            .write_all(&self.salt)
+            .map_err(|e| Error::DataWrite("salt", e))?;
+        writer
+            .write_all(&self.root_digest)
+            .map_err(|e| Error::DataWrite("root_digest", e))?;
+        writer
+            .write_all(&self.hash_tree)
+            .map_err(|e| Error::DataWrite("hash_tree", e))?;
 
         Ok(())
     }

avbroot/src/format/lp.rs

@@ -19,7 +19,9 @@ use zerocopy_derive::{FromBytes, Immutable, IntoBytes, KnownLayout, Unaligned};
 
 use crate::{
     format::padding,
-    stream::{CountingReader, FromReader, ReadDiscardExt, ToWriter, WriteZerosExt},
+    stream::{
+        CountingReader, FromReader, ReadDiscardExt, ReadFixedSizeExt, ToWriter, WriteZerosExt,
+    },
     util::{self, is_zero, DebugString},
 };
 
@@ -58,8 +60,8 @@ const METADATA_MAX_SIZE: u32 = 128 * 1024;
 #[derive(Debug, Error)]
 pub enum Error {
     // Naming errors.
-    #[error("Invalid partition name: {0}")]
-    PartitionNameInvalid(String),
+    #[error("Invalid partition name: {0:?}")]
+    PartitionNameInvalid(DebugString),
     // Geometry errors.
     #[error("Invalid geometry magic: {0:#010x}")]
     GeometryInvalidMagic(u32),
@@ -185,8 +187,10 @@ pub enum Error {
     #[error("Insufficient space on block devices to allocate sectors")]
     AllocatorDeviceFull,
     // Wrapped errors.
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to read LP data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write LP data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -600,14 +604,14 @@ impl PartitionName {
             match b {
                 b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' => has_alnum = true,
                 b'_' => {}
-                _ => return Err(Error::PartitionNameInvalid(format!("{self:?}"))),
+                _ => return Err(Error::PartitionNameInvalid(DebugString::new(self))),
             }
         }
 
         if has_alnum && is_zero(suffix) {
             Ok(())
         } else {
-            Err(Error::PartitionNameInvalid(format!("{self:?}")))
+            Err(Error::PartitionNameInvalid(DebugString::new(self)))
         }
     }
 
@@ -626,7 +630,7 @@ impl FromStr for PartitionName {
         let mut name = Self([0u8; 36]);
 
         if s.len() > name.0.len() {
-            return Err(Error::PartitionNameInvalid(format!("{s:?}")));
+            return Err(Error::PartitionNameInvalid(DebugString::new(s)));
         }
 
         let to_copy = s.len().min(name.0.len());
@@ -1052,7 +1056,9 @@ impl RawMetadata {
     /// Read the [`RawGeometry`] at the current offset.
     fn read_geometry(mut reader: impl Read) -> Result<(ImageType, RawGeometry)> {
         let mut buf = [0u8; GEOMETRY_SIZE as usize];
-        reader.read_exact(&mut buf)?;
+        reader
+            .read_exact(&mut buf)
+            .map_err(|e| Error::DataRead("geometry", e))?;
 
         let image_type = if util::is_zero(&buf) {
             ImageType::Normal
@@ -1067,16 +1073,22 @@ impl RawMetadata {
                 // as a boot sector.
 
                 // Read the primary copy of the geometry.
-                reader.read_exact(&mut buf)?;
+                reader
+                    .read_exact(&mut buf)
+                    .map_err(|e| Error::DataRead("geometry_primary", e))?;
 
                 let mut geometry = RawGeometry::ref_from_prefix(&buf).unwrap().0;
 
                 if geometry.validate().is_ok() {
                     // Skip the backup copy.
-                    reader.read_discard_exact(GEOMETRY_SIZE.into())?;
+                    reader
+                        .read_discard_exact(GEOMETRY_SIZE.into())
+                        .map_err(|e| Error::DataRead("geometry_secondary", e))?;
                 } else {
                     // Try to parse the backup copy.
-                    reader.read_exact(&mut buf)?;
+                    reader
+                        .read_exact(&mut buf)
+                        .map_err(|e| Error::DataRead("geometry_secondary", e))?;
 
                     geometry = RawGeometry::ref_from_prefix(&buf).unwrap().0;
                     geometry.validate()?;
@@ -1108,9 +1120,13 @@ impl RawMetadata {
     ) -> Result<RawMetadataSlot> {
         let mut header = RawHeader::new_zeroed();
 
-        reader.read_exact(&mut header.as_mut_bytes()[..RawHeader::SIZE_V1_0])?;
+        reader
+            .read_exact(&mut header.as_mut_bytes()[..RawHeader::SIZE_V1_0])
+            .map_err(|e| Error::DataRead("header_v1.0", e))?;
         if header.size() > RawHeader::SIZE_V1_0 {
-            reader.read_exact(&mut header.as_mut_bytes()[RawHeader::SIZE_V1_0..])?;
+            reader
+                .read_exact(&mut header.as_mut_bytes()[RawHeader::SIZE_V1_0..])
+                .map_err(|e| Error::DataRead("header_v1.2", e))?;
         }
 
         // We'll end up validating this again at the end, but this initial
@@ -1118,8 +1134,9 @@ impl RawMetadata {
         // parsing the tables.
         header.validate(geometry)?;
 
-        let mut tables_buf = vec![0u8; header.tables_size.get() as usize];
-        reader.read_exact(&mut tables_buf)?;
+        let tables_buf = reader
+            .read_vec_exact(header.tables_size.get() as usize)
+            .map_err(|e| Error::DataRead("tables", e))?;
 
         let partitions = header
             .partitions
@@ -1210,20 +1227,26 @@ impl<R: Read> FromReader<R> for RawMetadata {
                 let mut to_skip = u64::from(geometry.metadata_max_size.get());
 
                 if slot.is_none() {
-                    let orig_offset = reader.stream_position()?;
+                    let orig_offset = reader
+                        .stream_position()
+                        .map_err(|e| Error::DataRead("orig_offset", e))?;
 
                     match Self::read_metadata(&mut reader, image_type, &geometry) {
                         Ok(m) => *slot = Some(m),
-                        Err(e @ Error::Io(_)) => return Err(e),
+                        Err(e @ Error::DataRead(_, _)) => return Err(e),
                         Err(e) => last_err = Some(e),
                     }
 
                     // Skip the remaining padding.
-                    let cur_offset = reader.stream_position()?;
+                    let cur_offset = reader
+                        .stream_position()
+                        .map_err(|e| Error::DataRead("cur_offset", e))?;
                     to_skip -= cur_offset - orig_offset;
                 }
 
-                reader.read_discard(to_skip)?;
+                reader
+                    .read_discard(to_skip)
+                    .map_err(|e| Error::DataRead("slot_padding", e))?;
             }
         }
 
@@ -1251,11 +1274,17 @@ impl<W: Write> ToWriter<W> for RawMetadata {
 
         match self.image_type {
             ImageType::Normal => {
-                writer.write_zeros_exact(PARTITION_RESERVED_BYTES.into())?;
+                writer
+                    .write_zeros_exact(PARTITION_RESERVED_BYTES.into())
+                    .map_err(|e| Error::DataWrite("reserved", e))?;
 
                 for _ in 0..2 {
-                    writer.write_all(geometry)?;
-                    writer.write_zeros_exact(geometry_padding as u64)?;
+                    writer
+                        .write_all(geometry)
+                        .map_err(|e| Error::DataWrite("geometry", e))?;
+                    writer
+                        .write_zeros_exact(geometry_padding as u64)
+                        .map_err(|e| Error::DataWrite("geometry_padding", e))?;
                 }
 
                 let metadata_max_size = self.geometry.metadata_max_size.get() as usize;
@@ -1270,24 +1299,50 @@ impl<W: Write> ToWriter<W> for RawMetadata {
                         let tables_size = slot.header.tables_size.get() as usize;
                         let metadata_padding = metadata_max_size - header.len() - tables_size;
 
-                        writer.write_all(header)?;
-                        writer.write_all(slot.partitions.as_bytes())?;
-                        writer.write_all(slot.extents.as_bytes())?;
-                        writer.write_all(slot.groups.as_bytes())?;
-                        writer.write_all(slot.block_devices.as_bytes())?;
-                        writer.write_zeros_exact(metadata_padding as u64)?;
+                        writer
+                            .write_all(header)
+                            .map_err(|e| Error::DataWrite("header", e))?;
+                        writer
+                            .write_all(slot.partitions.as_bytes())
+                            .map_err(|e| Error::DataWrite("partition_tables", e))?;
+                        writer
+                            .write_all(slot.extents.as_bytes())
+                            .map_err(|e| Error::DataWrite("extent_tables", e))?;
+                        writer
+                            .write_all(slot.groups.as_bytes())
+                            .map_err(|e| Error::DataWrite("group_tables", e))?;
+                        writer
+                            .write_all(slot.block_devices.as_bytes())
+                            .map_err(|e| Error::DataWrite("block_device_tables", e))?;
+                        writer
+                            .write_zeros_exact(metadata_padding as u64)
+                            .map_err(|e| Error::DataWrite("metadata_padding", e))?;
                     }
                 }
             }
             ImageType::Empty => {
-                writer.write_all(geometry)?;
-                writer.write_zeros_exact(geometry_padding as u64)?;
+                writer
+                    .write_all(geometry)
+                    .map_err(|e| Error::DataWrite("geometry", e))?;
+                writer
+                    .write_zeros_exact(geometry_padding as u64)
+                    .map_err(|e| Error::DataWrite("geometry_padding", e))?;
 
-                writer.write_all(self.slots[0].header.as_bytes())?;
-                writer.write_all(self.slots[0].partitions.as_bytes())?;
-                writer.write_all(self.slots[0].extents.as_bytes())?;
-                writer.write_all(self.slots[0].groups.as_bytes())?;
-                writer.write_all(self.slots[0].block_devices.as_bytes())?;
+                writer
+                    .write_all(self.slots[0].header.as_bytes())
+                    .map_err(|e| Error::DataWrite("header", e))?;
+                writer
+                    .write_all(self.slots[0].partitions.as_bytes())
+                    .map_err(|e| Error::DataWrite("partition_tables", e))?;
+                writer
+                    .write_all(self.slots[0].extents.as_bytes())
+                    .map_err(|e| Error::DataWrite("extent_tables", e))?;
+                writer
+                    .write_all(self.slots[0].groups.as_bytes())
+                    .map_err(|e| Error::DataWrite("group_tables", e))?;
+                writer
+                    .write_all(self.slots[0].block_devices.as_bytes())
+                    .map_err(|e| Error::DataWrite("block_device_tables", e))?;
             }
         }
 

avbroot/src/format/ota.rs

@@ -6,6 +6,7 @@ use std::{
     fmt,
     io::{self, Cursor, Read, Seek, SeekFrom, Write},
     iter,
+    path::Path,
     sync::atomic::AtomicBool,
 };
 
@@ -23,7 +24,7 @@ use crate::{
     crypto::{self, RsaPublicKeyExt, RsaSigningKey, SignatureAlgorithm},
     format::payload::{self, PayloadHeader},
     protobuf::build::tools::releasetools::{ota_metadata::OtaType, OtaMetadata},
-    stream::{self, FromReader, HashingReader, HashingWriter},
+    stream::{self, FromReader, HashingReader, HashingWriter, ReadFixedSizeExt},
 };
 
 pub const PATH_METADATA: &str = "META-INF/com/android/metadata";
@@ -49,10 +50,12 @@ pub enum Error {
     OtaMagicNotFound,
     #[error("Cannot find EOCD magic")]
     EocdMagicNotFound,
-    #[error("EOCD magic found in archive comment")]
-    EocdMagicInComment,
+    #[error("EOCD magic found in archive comment at offset {0}")]
+    EocdMagicInComment(usize),
     #[error("Zip is too small to contain EOCD")]
     ZipTooSmall,
+    #[error("Zip archive comment is not empty: {0}")]
+    ZipNonEmptyComment(u16),
     #[error("Signature offset exceeds archive comment size")]
     SignatureOffsetTooLarge,
     #[error("Expected exactly one CMS embedded certificate, but found {0}")]
@@ -69,32 +72,50 @@ pub enum Error {
     UnsupportedLegacyMetadataField { key: String, value: String },
     #[error("Expected entry offsets {expected:?}, but have {actual:?}")]
     MismatchedPropertyFiles { expected: String, actual: String },
-    #[error("Property files {0:?} exceed {1} byte reserved space")]
-    InsufficientReservedSpace(String, usize),
+    #[error("Property files {value:?} exceed {reserved} byte reserved space")]
+    InsufficientReservedSpace { value: String, reserved: usize },
     #[error("Invalid property file entry: {0:?}")]
     InvalidPropertyFileEntry(String),
-    #[error("Missing entry in OTA zip: {0}")]
+    #[error("Missing entry in OTA zip: {0:?}")]
     MissingZipEntry(&'static str),
-    #[error("CMS signing error")]
-    CmsSign(#[from] crypto::Error),
-    #[error("Payload error")]
-    Payload(#[from] payload::Error),
-    #[error("Failed to decode protobuf message")]
-    ProtobufDecode(#[from] prost::DecodeError),
-    #[error("SPKI error")]
-    Spki(#[from] pkcs8::spki::Error),
-    #[error("x509 DER error")]
-    Der(#[from] x509_cert::der::Error),
-    #[error("Zip error")]
-    Zip(#[from] ZipError),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to decode OTA metadata protobuf message")]
+    MetadataDecode(#[source] prost::DecodeError),
+    #[error("Failed to open zip file")]
+    ZipOpen(#[source] ZipError),
+    #[error("Failed to open zip entry: {0:?}")]
+    ZipEntryOpen(&'static str, #[source] ZipError),
+    #[error("Failed to start new zip entry: {0:?}")]
+    ZipEntryStart(&'static str, #[source] ZipError),
+    #[error("Failed to read zip entry: {0:?}")]
+    ZipEntryRead(&'static str, #[source] io::Error),
+    #[error("Failed to write zip entry: {0:?}")]
+    ZipEntryWrite(&'static str, #[source] io::Error),
+    #[error("Failed to open zip entry #{0}")]
+    ZipIndexOpen(usize, #[source] ZipError),
+    #[error("Failed to load OTA certificate")]
+    OtaCertLoad(#[source] crypto::Error),
+    #[error("Failed to extract public key from OTA certificate")]
+    OtaCertExtractPubKey(#[source] crypto::Error),
+    #[error("Failed to load payload binary")]
+    PayloadLoad(#[source] payload::Error),
+    #[error("Failed to load CMS signature")]
+    CmsLoad(#[source] crypto::Error),
+    #[error("Failed to save CMS signature")]
+    CmsSave(#[source] x509_cert::der::Error),
+    #[error("Failed to generate CMS signature")]
+    CmsSign(#[source] crypto::Error),
+    #[error("Failed to verify CMS signature")]
+    CmsVerify(#[source] crypto::Error),
+    #[error("Failed to read OTA data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write OTA data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
 
 pub fn parse_protobuf_metadata(data: &[u8]) -> Result<OtaMetadata> {
-    Ok(OtaMetadata::decode(data)?)
+    OtaMetadata::decode(data).map_err(Error::MetadataDecode)
 }
 
 /// Synthesize protobuf structure from legacy plain-text metadata.
@@ -306,7 +327,7 @@ fn compute_property_files(
         let entry = entries
             .iter()
             .find(|e| e.name == path)
-            .ok_or_else(|| Error::MissingZipEntry(path))?;
+            .ok_or(Error::MissingZipEntry(path))?;
         let name = path.rsplit_once('/').map_or(path, |p| p.1);
 
         Ok(format!("{name}:{}:{}", entry.offset, entry.size))
@@ -349,7 +370,10 @@ fn compute_property_files(
 
     if let Some(l) = max_length {
         if joined.len() > l {
-            return Err(Error::InsufficientReservedSpace(joined, l));
+            return Err(Error::InsufficientReservedSpace {
+                value: joined,
+                reserved: l,
+            });
         }
 
         let remain = l - joined.len();
@@ -368,7 +392,7 @@ fn add_payload_metadata_entry(
     let payload_offset = entries
         .iter()
         .find(|e| e.name == PATH_PAYLOAD)
-        .ok_or_else(|| Error::MissingZipEntry(PATH_PAYLOAD))?
+        .ok_or(Error::MissingZipEntry(PATH_PAYLOAD))?
         .offset;
     entries.push(ZipEntry {
         name: NAME_PAYLOAD_METADATA.to_owned(),
@@ -433,13 +457,25 @@ pub fn add_metadata(
             ZipMode::Seekable => ZipWriter::new(raw_writer),
         };
 
-        writer.start_file_with_extra_data(PATH_METADATA, options)?;
-        let legacy_offset = writer.end_extra_data()?;
-        writer.write_all(legacy_raw.as_bytes())?;
+        writer
+            .start_file_with_extra_data(PATH_METADATA, options)
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA, e))?;
+        let legacy_offset = writer
+            .end_extra_data()
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA, e))?;
+        writer
+            .write_all(legacy_raw.as_bytes())
+            .map_err(|e| Error::ZipEntryWrite(PATH_METADATA, e))?;
 
-        writer.start_file_with_extra_data(PATH_METADATA_PB, options)?;
-        let modern_offset = writer.end_extra_data()?;
-        writer.write_all(&modern_raw)?;
+        writer
+            .start_file_with_extra_data(PATH_METADATA_PB, options)
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA_PB, e))?;
+        let modern_offset = writer
+            .end_extra_data()
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA_PB, e))?;
+        writer
+            .write_all(&modern_raw)
+            .map_err(|e| Error::ZipEntryWrite(PATH_METADATA_PB, e))?;
 
         zip_entries.push(ZipEntry {
             name: PATH_METADATA.to_owned(),
@@ -464,13 +500,25 @@ pub fn add_metadata(
     {
         let (legacy_raw, modern_raw) = serialize_metadata(&metadata);
 
-        zip_writer.start_file_with_extra_data(PATH_METADATA, options)?;
-        let legacy_offset = zip_writer.end_extra_data()?;
-        zip_writer.write_all(legacy_raw.as_bytes())?;
+        zip_writer
+            .start_file_with_extra_data(PATH_METADATA, options)
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA, e))?;
+        let legacy_offset = zip_writer
+            .end_extra_data()
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA, e))?;
+        zip_writer
+            .write_all(legacy_raw.as_bytes())
+            .map_err(|e| Error::ZipEntryWrite(PATH_METADATA, e))?;
 
-        zip_writer.start_file_with_extra_data(PATH_METADATA_PB, options)?;
-        let modern_offset = zip_writer.end_extra_data()?;
-        zip_writer.write_all(&modern_raw)?;
+        zip_writer
+            .start_file_with_extra_data(PATH_METADATA_PB, options)
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA_PB, e))?;
+        let modern_offset = zip_writer
+            .end_extra_data()
+            .map_err(|e| Error::ZipEntryStart(PATH_METADATA_PB, e))?;
+        zip_writer
+            .write_all(&modern_raw)
+            .map_err(|e| Error::ZipEntryWrite(PATH_METADATA_PB, e))?;
 
         assert_eq!(legacy_offset, temp_legacy_offset);
         assert_eq!(modern_offset, temp_modern_offset);
@@ -485,11 +533,13 @@ pub fn verify_metadata(
     metadata: &OtaMetadata,
     payload_metadata_size: u64,
 ) -> Result<()> {
-    let mut zip_reader = ZipArchive::new(reader)?;
+    let mut zip_reader = ZipArchive::new(reader).map_err(Error::ZipOpen)?;
     let mut zip_entries = vec![];
 
     for i in 0..zip_reader.len() {
-        let entry = zip_reader.by_index(i)?;
+        let entry = zip_reader
+            .by_index(i)
+            .map_err(|e| Error::ZipIndexOpen(i, e))?;
         zip_entries.push(ZipEntry {
             name: entry.name().to_owned(),
             offset: entry.data_start(),
@@ -520,11 +570,16 @@ pub fn verify_metadata(
 /// that's covered by the signature. This does not perform any parsing of zip
 /// data structures.
 fn parse_ota_sig(mut reader: impl Read + Seek) -> Result<(SignedData, u64)> {
-    let file_size = reader.seek(SeekFrom::End(0))?;
+    let file_size = reader
+        .seek(SeekFrom::End(0))
+        .map_err(|e| Error::DataRead("file_size", e))?;
 
-    reader.seek(SeekFrom::Current(-6))?;
-    let mut footer = [0u8; 6];
-    reader.read_exact(&mut footer)?;
+    reader
+        .seek(SeekFrom::Current(-6))
+        .map_err(|e| Error::DataRead("footer", e))?;
+    let footer = reader
+        .read_array_exact::<6>()
+        .map_err(|e| Error::DataRead("footer", e))?;
 
     let abs_eoc_offset = u16::from_le_bytes(footer[0..2].try_into().unwrap());
     let sig_magic = u16::from_le_bytes(footer[2..4].try_into().unwrap());
@@ -543,20 +598,24 @@ fn parse_ota_sig(mut reader: impl Read + Seek) -> Result<(SignedData, u64)> {
         return Err(Error::SignatureOffsetTooLarge);
     }
 
-    reader.seek(SeekFrom::Start(file_size - eocd_size))?;
-    let mut eocd = vec![0u8; eocd_size as usize];
-    reader.read_exact(&mut eocd)?;
+    reader
+        .seek(SeekFrom::Start(file_size - eocd_size))
+        .map_err(|e| Error::DataRead("eocd", e))?;
+    let eocd = reader
+        .read_vec_exact(eocd_size as usize)
+        .map_err(|e| Error::DataRead("eocd", e))?;
 
     let mut eocd_magic_iter = memmem::find_iter(&eocd, ZIP_EOCD_MAGIC);
     if eocd_magic_iter.next() != Some(0) {
         return Err(Error::EocdMagicNotFound);
     }
-    if eocd_magic_iter.next().is_some() {
-        return Err(Error::EocdMagicInComment);
+    if let Some(offset) = eocd_magic_iter.next() {
+        return Err(Error::EocdMagicInComment(offset));
     }
 
     let sig_offset = eocd_size as usize - usize::from(abs_eoc_offset);
-    let sd = crypto::parse_cms(&eocd[sig_offset..eocd_size as usize - 6])?;
+    let sd =
+        crypto::parse_cms(&eocd[sig_offset..eocd_size as usize - 6]).map_err(Error::CmsLoad)?;
     // The signature covers everything aside from the archive comment and its
     // length field.
     let hashed_size = file_size - 2 - u64::from(comment_size);
@@ -582,7 +641,7 @@ pub fn verify_ota(mut reader: impl Read + Seek, cancel_signal: &AtomicBool) -> R
     }
 
     let cert = &certs[0];
-    let public_key = crypto::get_public_key(cert)?;
+    let public_key = crypto::get_public_key(cert).map_err(Error::OtaCertExtractPubKey)?;
 
     // Make sure this is a signature scheme we can handle. There's currently no
     // Rust library to verify arbitrary CMS signatures for large files without
@@ -603,7 +662,9 @@ pub fn verify_ota(mut reader: impl Read + Seek, cancel_signal: &AtomicBool) -> R
     }
 
     // Manually hash the parts of the file covered by the signature.
-    reader.seek(SeekFrom::Start(0))?;
+    reader
+        .seek(SeekFrom::Start(0))
+        .map_err(|e| Error::DataRead("raw_data", e))?;
 
     // We support SHA1 for verification only.
     let (algorithm, algo) = if signer.digest_alg.oid == rfc5912::ID_SHA_256 {
@@ -617,13 +678,16 @@ pub fn verify_ota(mut reader: impl Read + Seek, cancel_signal: &AtomicBool) -> R
 
     let mut hashing_reader = HashingReader::new(reader, Context::new(algorithm));
 
-    stream::copy_n(&mut hashing_reader, io::sink(), hashed_size, cancel_signal)?;
+    stream::copy_n(&mut hashing_reader, io::sink(), hashed_size, cancel_signal)
+        .map_err(|e| Error::DataRead("raw_data", e))?;
 
     let (_, context) = hashing_reader.finish();
     let digest = context.finish();
 
     // Verify the signature against the public key.
-    public_key.verify_sig(algo, digest.as_ref(), signer.signature.as_bytes())?;
+    public_key
+        .verify_sig(algo, digest.as_ref(), signer.signature.as_bytes())
+        .map_err(Error::CmsVerify)?;
 
     Ok(cert.clone())
 }
@@ -633,38 +697,52 @@ pub fn verify_ota(mut reader: impl Read + Seek, cancel_signal: &AtomicBool) -> R
 pub fn parse_zip_ota_info(
     reader: impl Read + Seek,
 ) -> Result<(OtaMetadata, Certificate, PayloadHeader, String)> {
-    let mut zip = ZipArchive::new(reader)?;
+    let mut zip = ZipArchive::new(reader).map_err(Error::ZipOpen)?;
 
     let metadata = match zip.by_name(PATH_METADATA_PB) {
         Ok(mut entry) => {
             let mut buf = Vec::new();
-            entry.read_to_end(&mut buf)?;
+            entry
+                .read_to_end(&mut buf)
+                .map_err(|e| Error::ZipEntryRead(PATH_METADATA_PB, e))?;
             parse_protobuf_metadata(&buf)?
         }
         e @ Err(ZipError::FileNotFound) => {
             drop(e);
-            let mut entry = zip.by_name(PATH_METADATA)?;
+            let mut entry = zip
+                .by_name(PATH_METADATA)
+                .map_err(|e| Error::ZipEntryOpen(PATH_METADATA, e))?;
             let mut buf = String::new();
-            entry.read_to_string(&mut buf)?;
+            entry
+                .read_to_string(&mut buf)
+                .map_err(|e| Error::ZipEntryRead(PATH_METADATA, e))?;
             parse_legacy_metadata(&buf)?
         }
-        Err(e) => return Err(e.into()),
+        Err(e) => return Err(Error::ZipEntryOpen(PATH_METADATA_PB, e)),
     };
 
     let certificate = {
-        let entry = zip.by_name(PATH_OTACERT)?;
-        crypto::read_pem_cert(entry)?
+        let entry = zip
+            .by_name(PATH_OTACERT)
+            .map_err(|e| Error::ZipEntryOpen(PATH_OTACERT, e))?;
+        crypto::read_pem_cert(Path::new(PATH_OTACERT), entry).map_err(Error::OtaCertLoad)?
     };
 
     let header = {
-        let entry = zip.by_name(PATH_PAYLOAD)?;
-        PayloadHeader::from_reader(entry)?
+        let entry = zip
+            .by_name(PATH_PAYLOAD)
+            .map_err(|e| Error::ZipEntryOpen(PATH_PAYLOAD, e))?;
+        PayloadHeader::from_reader(entry).map_err(Error::PayloadLoad)?
     };
 
     let properties = {
-        let mut entry = zip.by_name(PATH_PROPERTIES)?;
+        let mut entry = zip
+            .by_name(PATH_PROPERTIES)
+            .map_err(|e| Error::ZipEntryOpen(PATH_PROPERTIES, e))?;
         let mut buf = String::new();
-        entry.read_to_string(&mut buf)?;
+        entry
+            .read_to_string(&mut buf)
+            .map_err(|e| Error::ZipEntryRead(PATH_PROPERTIES, e))?;
         buf
     };
 
@@ -672,17 +750,12 @@ pub fn parse_zip_ota_info(
 }
 
 /// Ensure that we're using a non-zip64 EOCD and there's no archive comment.
-fn validate_eocd(eocd: &[u8]) -> io::Result<()> {
+fn validate_eocd(eocd: &[u8]) -> Result<()> {
     if &eocd[..4] != b"PK\x05\x06" {
-        return Err(io::Error::new(
-            io::ErrorKind::InvalidData,
-            "EOCD magic not found",
-        ));
+        return Err(Error::EocdMagicNotFound);
     } else if &eocd[20..22] != b"\0\0" {
-        return Err(io::Error::new(
-            io::ErrorKind::InvalidData,
-            "Archive comment is not 0 bytes",
-        ));
+        let size = u16::from_le_bytes(eocd[20..22].try_into().unwrap());
+        return Err(Error::ZipNonEmptyComment(size));
     }
 
     Ok(())
@@ -697,8 +770,9 @@ fn compute_signature_comment(
     cert: &Certificate,
     digest: ring::digest::Digest,
 ) -> Result<Vec<u8>> {
-    let cms_signature = crypto::cms_sign_external(key, cert, digest.as_ref())?;
-    let cms_signature_der = cms_signature.to_der()?;
+    let cms_signature =
+        crypto::cms_sign_external(key, cert, digest.as_ref()).map_err(Error::CmsSign)?;
+    let cms_signature_der = cms_signature.to_der().map_err(Error::CmsSave)?;
 
     // Includes placeholder for the EOCD comment size field.
     let mut buf = vec![0; 2];
@@ -720,12 +794,8 @@ fn compute_signature_comment(
     // Archive comment size (for use by the OTA signature verifier).
     buf.extend(((comment_size) as u16).to_le_bytes());
 
-    if let Some(o) = memmem::find(&buf[2..], ZIP_EOCD_MAGIC) {
-        return Err(io::Error::new(
-            io::ErrorKind::InvalidData,
-            format!("Archive comment contains EOCD magic at offset {o}"),
-        )
-        .into());
+    if let Some(offset) = memmem::find(&buf[2..], ZIP_EOCD_MAGIC) {
+        return Err(Error::EocdMagicInComment(offset));
     }
 
     // Archive comment size (for the EOCD comment size field).
@@ -758,21 +828,23 @@ impl<W: Write> StreamingSigningWriter<W> {
 
     pub fn finish(mut self, key: &RsaSigningKey, cert: &Certificate) -> Result<W> {
         if self.used < self.queue.len() {
-            return Err(
-                io::Error::new(io::ErrorKind::InvalidData, "Too small to contain EOCD").into(),
-            );
+            return Err(Error::ZipTooSmall);
         }
 
         validate_eocd(&self.queue)?;
 
         // Chop off the archive comment size field and write the remaining data.
-        self.inner.write_all(&self.queue[..20])?;
+        self.inner
+            .write_all(&self.queue[..20])
+            .map_err(|e| Error::DataWrite("eocd_minus_comment", e))?;
 
         let (mut raw_writer, context) = self.inner.finish();
         let digest = context.finish();
 
         let size_and_comment = compute_signature_comment(key, cert, digest)?;
-        raw_writer.write_all(&size_and_comment)?;
+        raw_writer
+            .write_all(&size_and_comment)
+            .map_err(|e| Error::DataWrite("size_and_comment", e))?;
 
         Ok(raw_writer)
     }
@@ -832,18 +904,20 @@ impl<W: Read + Write + Seek> SeekableSigningWriter<W> {
         cert: &Certificate,
         cancel_signal: &AtomicBool,
     ) -> Result<W> {
-        let file_size = self.seek(SeekFrom::End(0))?;
+        let file_size = self
+            .seek(SeekFrom::End(0))
+            .map_err(|e| Error::DataRead("file_size", e))?;
 
         // Android only supports non-zip64 EOCD.
         if file_size < 22 {
-            return Err(
-                io::Error::new(io::ErrorKind::InvalidData, "Too small to contain EOCD").into(),
-            );
+            return Err(Error::ZipTooSmall);
         }
 
-        self.seek_relative(-22)?;
-        let mut eocd = [0u8; 22];
-        self.read_exact(&mut eocd)?;
+        self.seek_relative(-22)
+            .map_err(|e| Error::DataRead("eocd", e))?;
+        let eocd = self
+            .read_array_exact::<22>()
+            .map_err(|e| Error::DataRead("eocd", e))?;
 
         validate_eocd(&eocd)?;
 
@@ -853,13 +927,16 @@ impl<W: Read + Write + Seek> SeekableSigningWriter<W> {
             ring::digest::Context::new(&ring::digest::SHA256),
         );
 
-        self.rewind()?;
-        stream::copy_n(&mut self, &mut hashing_writer, file_size - 2, cancel_signal)?;
+        self.rewind().map_err(|e| Error::DataRead("raw_data", e))?;
+        stream::copy_n(&mut self, &mut hashing_writer, file_size - 2, cancel_signal)
+            .map_err(|e| Error::DataRead("raw_data", e))?;
 
         let digest = hashing_writer.finish().1.finish();
 
         let size_and_comment = compute_signature_comment(key, cert, digest)?;
-        self.inner.write_all(&size_and_comment)?;
+        self.inner
+            .write_all(&size_and_comment)
+            .map_err(|e| Error::DataWrite("size_and_comment", e))?;
 
         Ok(self.inner)
     }

avbroot/src/format/payload.rs

@@ -36,8 +36,8 @@ use crate::{
         InstallOperation, PartitionInfo, PartitionUpdate, Signatures,
     },
     stream::{
-        self, CountingReader, FromReader, HashingWriter, ReadDiscardExt, ReadSeekReopen, WriteSeek,
-        WriteSeekReopen,
+        self, CountingReader, FromReader, HashingWriter, ReadDiscardExt, ReadFixedSizeExt,
+        ReadSeekReopen, WriteSeek, WriteSeekReopen,
     },
     util::{self, OutOfBoundsError},
 };
@@ -94,14 +94,41 @@ pub enum Error {
     IntOutOfBounds(&'static str, #[source] OutOfBoundsError),
     #[error("{0:?} overflowed integer bounds during calculations")]
     IntOverflow(&'static str),
-    #[error("Crypto error")]
-    Crypto(#[from] crypto::Error),
-    #[error("Failed to decode protobuf message")]
-    ProtobufDecode(#[from] prost::DecodeError),
-    #[error("XZ stream error")]
-    XzStream(#[from] liblzma::stream::Error),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to decode payload manifest protobuf message")]
+    ManifestDecode(#[source] prost::DecodeError),
+    #[error("Failed to decode payload signatures protobuf message")]
+    SignaturesDecode(#[source] prost::DecodeError),
+    #[error("Failed to generate payload signature")]
+    SignatureGenerate(#[source] crypto::Error),
+    #[error("Failed to verify payload signature")]
+    SignatureVerify(#[source] crypto::Error),
+    #[error("Failed to read payload data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write payload data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
+    #[error("Expected {expected} bytes, but only wrote {actual} bytes")]
+    UnwrittenData { actual: u64, expected: u64 },
+    #[error("I/O error when applying {op_type:?} operation for {num_blocks} blocks starting at {start_block}")]
+    OperationApply {
+        op_type: Type,
+        start_block: u64,
+        num_blocks: u64,
+        source: io::Error,
+    },
+    #[error("Failed to reopen payload")]
+    PayloadReopen(#[source] io::Error),
+    #[error("Failed to open input file for partition: {0}")]
+    InputOpen(String, #[source] io::Error),
+    #[error("Failed to open output file for partition: {0}")]
+    OutputOpen(String, #[source] io::Error),
+    #[error("Failed to initialize XZ encoder")]
+    XzInit(#[source] liblzma::stream::Error),
+    #[error("Failed to XZ compress partition image chunk")]
+    XzCompress(#[source] io::Error),
+    #[error("Failed to read uncompressed input partition image chunk")]
+    ChunkRead(#[source] io::Error),
+    #[error("Failed to write XZ-compressed output partition image chunk")]
+    ChunkWrite(#[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -147,7 +174,8 @@ impl<R: Read> FromReader<R> for PayloadHeader {
     fn from_reader(reader: R) -> Result<Self> {
         let mut reader = CountingReader::new(reader);
 
-        let header = RawHeader::read_from_io(&mut reader)?;
+        let header =
+            RawHeader::read_from_io(&mut reader).map_err(|e| Error::DataRead("header", e))?;
 
         if header.magic != *PAYLOAD_MAGIC {
             return Err(Error::UnknownMagic(header.magic));
@@ -161,18 +189,24 @@ impl<R: Read> FromReader<R> for PayloadHeader {
             .and_then(|s| util::check_bounds(s, ..=MANIFEST_MAX_SIZE))
             .map_err(|e| Error::IntOutOfBounds("manifest_size", e))?;
 
-        let mut manifest_raw = vec![0u8; manifest_size];
-        reader.read_exact(&mut manifest_raw)?;
-        let manifest = DeltaArchiveManifest::decode(manifest_raw.as_slice())?;
+        let manifest_raw = reader
+            .read_vec_exact(manifest_size)
+            .map_err(|e| Error::DataRead("manifest", e))?;
+        let manifest =
+            DeltaArchiveManifest::decode(manifest_raw.as_slice()).map_err(Error::ManifestDecode)?;
 
         // Skip manifest signatures.
-        reader.read_discard_exact(header.metadata_signature_size.into())?;
+        reader
+            .read_discard_exact(header.metadata_signature_size.into())
+            .map_err(|e| Error::DataRead("metadata_signatures", e))?;
 
         Ok(Self {
             version: header.file_format_version.get(),
             manifest,
             metadata_signature_size: header.metadata_signature_size.get(),
-            blob_offset: reader.stream_position()?,
+            blob_offset: reader
+                .stream_position()
+                .map_err(|e| Error::DataRead("blob_offset", e))?,
         })
     }
 }
@@ -180,7 +214,9 @@ impl<R: Read> FromReader<R> for PayloadHeader {
 /// Sign `digest` with `key` and return a [`Signatures`] protobuf struct with
 /// the signature padded to the maximum size.
 fn sign_digest(digest: &[u8], key: &RsaSigningKey) -> Result<Signatures> {
-    let mut digest_signed = key.sign(SignatureAlgorithm::Sha256WithRsa, digest)?;
+    let mut digest_signed = key
+        .sign(SignatureAlgorithm::Sha256WithRsa, digest)
+        .map_err(Error::SignatureGenerate)?;
     assert!(
         digest_signed.len() <= key.size(),
         "Signature exceeds maximum size",
@@ -205,7 +241,7 @@ fn sign_digest(digest: &[u8], key: &RsaSigningKey) -> Result<Signatures> {
 
 /// Verify `digest` inside `signatures` using `cert`.
 fn verify_digest(digest: &[u8], signatures: &Signatures, cert: &Certificate) -> Result<()> {
-    let public_key = crypto::get_public_key(cert)?;
+    let public_key = crypto::get_public_key(cert).map_err(Error::SignatureVerify)?;
     let mut last_error = None;
 
     for signature in &signatures.signatures {
@@ -219,11 +255,11 @@ fn verify_digest(digest: &[u8], signatures: &Signatures, cert: &Certificate) ->
 
         match public_key.verify_sig(SignatureAlgorithm::Sha256WithRsa, digest, without_padding) {
             Ok(()) => return Ok(()),
-            Err(e) => last_error = Some(e),
+            Err(e) => last_error = Some(Error::SignatureVerify(e)),
         }
     }
 
-    Err(last_error.map_or(Error::NoSignatures, |e| e.into()))
+    Err(last_error.unwrap_or(Error::NoSignatures))
 }
 
 fn parse_properties(data: &str) -> Result<HashMap<String, String>> {
@@ -359,17 +395,20 @@ impl<W: Write> PayloadWriter<W> {
             manifest_size: (manifest_raw_new.len() as u64).into(),
             metadata_signature_size: (dummy_sig_size as u32).into(),
         };
-        write_hash!(inner, [h_partial, h_full], raw_header.as_bytes())?;
+        write_hash!(inner, [h_partial, h_full], raw_header.as_bytes())
+            .map_err(|e| Error::DataWrite("header", e))?;
 
         // Write new manifest.
-        write_hash!(inner, [h_partial, h_full], &manifest_raw_new)?;
+        write_hash!(inner, [h_partial, h_full], &manifest_raw_new)
+            .map_err(|e| Error::DataWrite("manifest", e))?;
 
         // Sign metadata (header + manifest) hash. The signature is not included
         // in the payload hash.
         let metadata_hash = h_partial.clone().finish();
         let metadata_sig = sign_digest(metadata_hash.as_ref(), &key)?;
         let metadata_sig_raw = metadata_sig.encode_to_vec();
-        write_hash!(inner, [h_full], &metadata_sig_raw)?;
+        write_hash!(inner, [h_full], &metadata_sig_raw)
+            .map_err(|e| Error::DataWrite("metadata_signatures", e))?;
 
         Ok(Self {
             inner,
@@ -397,7 +436,8 @@ impl<W: Write> PayloadWriter<W> {
         let payload_partial_hash = self.h_partial.clone().finish();
         let payload_sig = sign_digest(payload_partial_hash.as_ref(), &self.key)?;
         let payload_sig_raw = payload_sig.encode_to_vec();
-        write_hash!(self.inner, [self.h_full], &payload_sig_raw)?;
+        write_hash!(self.inner, [self.h_full], &payload_sig_raw)
+            .map_err(|e| Error::DataWrite("payload_signatures", e))?;
 
         // Everything before the blob.
         let metadata_with_sig_size =
@@ -422,23 +462,20 @@ impl<W: Write> PayloadWriter<W> {
     /// Prepare for writing the next source data blob corresponding to an
     /// [`InstallOperation`]. To write all of the payload data, call this method
     /// followed by [`Self::write()`] repeatedly until `Ok(false)` is returned
-    /// or an error occurs. This function will fail if the amount of data
-    /// written for the previous operation does not match
+    /// or an error occurs. [`Error::UnwrittenData`] will be returned if the
+    /// amount of data written for the previous operation does not match
     /// [`InstallOperation::data_length`].
     pub fn begin_next_operation(&mut self) -> Result<bool> {
         if let Some(operation) = self.operation() {
             // Only operations that reference data in the blob will have a
             // length set.
-            if self.written < operation.data_length.unwrap_or(0) {
-                return Err(io::Error::new(
-                    io::ErrorKind::InvalidInput,
-                    format!(
-                        "Expected {} bytes, but only wrote {} bytes",
-                        operation.data_length.unwrap(),
-                        self.written,
-                    ),
-                )
-                .into());
+            let expected = operation.data_length.unwrap_or(0);
+
+            if self.written < expected {
+                return Err(Error::UnwrittenData {
+                    actual: self.written,
+                    expected,
+                });
             }
         }
 
@@ -548,16 +585,16 @@ pub fn verify_payload(
     cancel_signal: &AtomicBool,
 ) -> Result<()> {
     let header = PayloadHeader::from_reader(&mut reader)?;
-    reader.rewind()?;
+    reader.rewind().map_err(|e| Error::DataRead("header", e))?;
 
     let payload_signatures_offset = header
         .manifest
         .signatures_offset
-        .ok_or_else(|| Error::MissingField("signatures_offset"))?;
+        .ok_or(Error::MissingField("signatures_offset"))?;
     let payload_signatures_size = header
         .manifest
         .signatures_size
-        .ok_or_else(|| Error::MissingField("signatures_size"))?;
+        .ok_or(Error::MissingField("signatures_size"))?;
 
     // Excludes signatures (hashes are for signing).
     let mut h_partial = Context::new(&ring::digest::SHA256);
@@ -575,7 +612,8 @@ pub fn verify_payload(
             h_full.update(data);
         },
         cancel_signal,
-    )?;
+    )
+    .map_err(|e| Error::DataRead("metadata", e))?;
     let metadata_hash = h_partial.clone().finish();
 
     // Read the metadata signatures.
@@ -588,10 +626,11 @@ pub fn verify_payload(
             header.metadata_signature_size.into(),
             |data| h_full.update(data),
             cancel_signal,
-        )?;
+        )
+        .map_err(|e| Error::DataRead("metadata_signatures", e))?;
 
         let buf = writer.into_inner();
-        Signatures::decode(buf.as_slice())?
+        Signatures::decode(buf.as_slice()).map_err(Error::SignaturesDecode)?
     };
 
     // Check the metadata signatures.
@@ -599,7 +638,9 @@ pub fn verify_payload(
 
     // Check the blob offset.
     {
-        let actual = reader.stream_position()?;
+        let actual = reader
+            .stream_position()
+            .map_err(|e| Error::DataRead("blob_offset", e))?;
         if header.blob_offset != actual {
             return Err(Error::InvalidBlobOffset {
                 expected: header.blob_offset,
@@ -618,13 +659,16 @@ pub fn verify_payload(
             h_full.update(data);
         },
         cancel_signal,
-    )?;
+    )
+    .map_err(|e| Error::DataRead("blob", e))?;
     let payload_hash = h_partial.clone().finish();
 
     // Check the payload signatures offset.
     {
         let expected = header.blob_offset + payload_signatures_offset;
-        let actual = reader.stream_position()?;
+        let actual = reader
+            .stream_position()
+            .map_err(|e| Error::DataRead("payload_signatures_offset", e))?;
         if expected != actual {
             return Err(Error::InvalidPayloadSignaturesOffset { expected, actual });
         }
@@ -640,10 +684,11 @@ pub fn verify_payload(
             payload_signatures_size,
             |data| h_full.update(data),
             cancel_signal,
-        )?;
+        )
+        .map_err(|e| Error::DataRead("payload_signatures", e))?;
 
         let buf = writer.into_inner();
-        Signatures::decode(buf.as_slice())?
+        Signatures::decode(buf.as_slice()).map_err(Error::SignaturesDecode)?
     };
 
     // Check the payload signatures.
@@ -652,7 +697,9 @@ pub fn verify_payload(
     // Check properties file.
     let expected_properties_raw = generate_properties(
         h_full.finish().as_ref(),
-        reader.stream_position()?,
+        reader
+            .stream_position()
+            .map_err(|e| Error::DataRead("payload_size", e))?,
         metadata_hash.as_ref(),
         metadata_size,
     );
@@ -687,19 +734,24 @@ pub fn apply_operation(
     for extent in &op.dst_extents {
         let start_block = extent
             .start_block
-            .ok_or_else(|| Error::MissingField("start_block"))?;
-        let num_blocks = extent
-            .num_blocks
-            .ok_or_else(|| Error::MissingField("num_blocks"))?;
+            .ok_or(Error::MissingField("start_block"))?;
+        let num_blocks = extent.num_blocks.ok_or(Error::MissingField("num_blocks"))?;
 
         let out_offset = start_block
             .checked_mul(block_size.into())
-            .ok_or_else(|| Error::IntOverflow("out_offset"))?;
+            .ok_or(Error::IntOverflow("out_offset"))?;
         let out_data_length = num_blocks
             .checked_mul(block_size.into())
-            .ok_or_else(|| Error::IntOverflow("out_data_length"))?;
+            .ok_or(Error::IntOverflow("out_data_length"))?;
 
-        writer.seek(SeekFrom::Start(out_offset))?;
+        let error_fn = |e: io::Error| Error::OperationApply {
+            op_type: op.r#type(),
+            start_block,
+            num_blocks,
+            source: e,
+        };
+
+        writer.seek(SeekFrom::Start(out_offset)).map_err(error_fn)?;
 
         let mut hasher = Context::new(&ring::digest::SHA256);
 
@@ -713,20 +765,17 @@ pub fn apply_operation(
                     out_data_length,
                     |data| hasher.update(data),
                     cancel_signal,
-                )?;
+                )
+                .map_err(error_fn)?;
             }
             other => {
-                let data_offset = op
-                    .data_offset
-                    .ok_or_else(|| Error::MissingField("data_offset"))?;
-                let data_length = op
-                    .data_length
-                    .ok_or_else(|| Error::MissingField("data_length"))?;
+                let data_offset = op.data_offset.ok_or(Error::MissingField("data_offset"))?;
+                let data_length = op.data_length.ok_or(Error::MissingField("data_length"))?;
                 let in_offset = blob_offset
                     .checked_add(data_offset)
-                    .ok_or_else(|| Error::IntOverflow("in_offset"))?;
+                    .ok_or(Error::IntOverflow("in_offset"))?;
 
-                reader.seek(SeekFrom::Start(in_offset))?;
+                reader.seek(SeekFrom::Start(in_offset)).map_err(error_fn)?;
 
                 match other {
                     Type::Replace => {
@@ -736,7 +785,8 @@ pub fn apply_operation(
                             data_length,
                             |data| hasher.update(data),
                             cancel_signal,
-                        )?;
+                        )
+                        .map_err(error_fn)?;
                     }
                     Type::ReplaceBz => {
                         let mut decoder = BzDecoder::new(&mut writer);
@@ -746,8 +796,9 @@ pub fn apply_operation(
                             data_length,
                             |data| hasher.update(data),
                             cancel_signal,
-                        )?;
-                        decoder.finish()?;
+                        )
+                        .and_then(|()| decoder.finish())
+                        .map_err(error_fn)?;
                     }
                     Type::ReplaceXz => {
                         let mut decoder = XzDecoder::new(&mut writer);
@@ -757,8 +808,9 @@ pub fn apply_operation(
                             data_length,
                             |data| hasher.update(data),
                             cancel_signal,
-                        )?;
-                        decoder.finish()?;
+                        )
+                        .and_then(|()| decoder.finish())
+                        .map_err(error_fn)?;
                     }
                     _ => return Err(Error::UnsupportedOperation(op.r#type())),
                 }
@@ -800,8 +852,10 @@ pub fn extract_image(
         .operations
         .par_iter()
         .map(|op| -> Result<()> {
-            let reader = payload.reopen_boxed()?;
-            let writer = output.reopen_boxed()?;
+            let reader = payload.reopen_boxed().map_err(Error::PayloadReopen)?;
+            let writer = output
+                .reopen_boxed()
+                .map_err(|e| Error::OutputOpen(partition_name.to_owned(), e))?;
 
             apply_operation(
                 reader,
@@ -848,8 +902,8 @@ pub fn extract_images<'a>(
     operations
         .into_par_iter()
         .map(|(name, op)| -> Result<()> {
-            let reader = payload.reopen_boxed()?;
-            let writer = open_output(name)?;
+            let reader = payload.reopen_boxed().map_err(Error::PayloadReopen)?;
+            let writer = open_output(name).map_err(|e| Error::OutputOpen(name.to_owned(), e))?;
 
             apply_operation(
                 reader,
@@ -874,12 +928,13 @@ fn compress_chunk(raw_data: &[u8], cancel_signal: &AtomicBool) -> Result<(Vec<u8
     // decompression. Also, we intentionally pick the lowest compression level
     // since we primarily care about squishing zeros. The non-zero portions of
     // boot images are usually already-compressed kernels and ramdisks.
-    let stream = Stream::new_easy_encoder(0, Check::None)?;
+    let stream = Stream::new_easy_encoder(0, Check::None).map_err(Error::XzInit)?;
     let mut xz_writer = XzEncoder::new_stream(hashing_writer, stream);
 
-    stream::copy_n(reader, &mut xz_writer, raw_data.len() as u64, cancel_signal)?;
+    stream::copy_n(reader, &mut xz_writer, raw_data.len() as u64, cancel_signal)
+        .map_err(Error::XzCompress)?;
 
-    let hashing_writer = xz_writer.finish()?;
+    let hashing_writer = xz_writer.finish().map_err(Error::XzCompress)?;
     let (writer, context_compressed) = hashing_writer.finish();
     let digest_compressed = context_compressed.finish();
     let data = writer.into_inner();
@@ -958,7 +1013,10 @@ pub fn compress_image(
     const CHUNK_SIZE: u64 = 2 * 1024 * 1024;
     const CHUNK_GROUP: u64 = 32;
 
-    let file_size = input.reopen_boxed()?.seek(SeekFrom::End(0))?;
+    let file_size = input
+        .reopen_boxed()
+        .and_then(|mut r| r.seek(SeekFrom::End(0)))
+        .map_err(|e| Error::InputOpen(partition_name.to_owned(), e))?;
     let final_chunk_different = file_size % CHUNK_SIZE != 0;
 
     if file_size % u64::from(block_size) != 0 || CHUNK_SIZE % u64::from(block_size) != 0 {
@@ -984,7 +1042,7 @@ pub fn compress_image(
 
         let uncompressed_data_group = (chunks_done..chunks_done + chunks_group)
             .into_par_iter()
-            .map(|chunk| -> Result<(u64, Vec<u8>)> {
+            .map(|chunk| -> io::Result<(u64, Vec<u8>)> {
                 let mut reader = input.reopen_boxed()?;
                 let offset = reader.seek(SeekFrom::Start(chunk * CHUNK_SIZE))?;
 
@@ -993,14 +1051,14 @@ pub fn compress_image(
                 } else {
                     CHUNK_SIZE
                 };
-                let mut data = vec![0u8; chunk_size as usize];
 
                 stream::check_cancel(cancel_signal)?;
-                reader.read_exact(&mut data)?;
+                let data = reader.read_vec_exact(chunk_size as usize)?;
 
                 Ok((offset, data))
             })
-            .collect::<Result<Vec<_>>>()?;
+            .collect::<io::Result<Vec<_>>>()
+            .map_err(Error::ChunkRead)?;
 
         for (_, data) in &uncompressed_data_group {
             context_uncompressed.update(data);
@@ -1038,14 +1096,15 @@ pub fn compress_image(
 
         let group_operations = compressed_data_group
             .into_par_iter()
-            .map(|(data, operation, _)| -> Result<InstallOperation> {
+            .map(|(data, operation, _)| -> io::Result<InstallOperation> {
                 let mut writer = output.reopen_boxed()?;
                 writer.seek(SeekFrom::Start(operation.data_offset.unwrap()))?;
                 writer.write_all(&data)?;
 
                 Ok(operation)
             })
-            .collect::<Result<Vec<_>>>()?;
+            .collect::<io::Result<Vec<_>>>()
+            .map_err(Error::ChunkWrite)?;
 
         operations.extend(group_operations.into_iter());
     }
@@ -1156,27 +1215,29 @@ pub fn compress_modified_image(
                 let extents_start = operation.dst_extents[0]
                     .start_block()
                     .checked_mul(u64::from(block_size))
-                    .ok_or_else(|| Error::IntOverflow("extents_start"))?;
+                    .ok_or(Error::IntOverflow("extents_start"))?;
                 let extents_size = operation
                     .dst_extents
                     .iter()
                     .map(|e| e.num_blocks())
                     .try_fold(0u64, |acc, n| acc.checked_add(n))
                     .and_then(|n| n.checked_mul(u64::from(block_size)))
-                    .ok_or_else(|| Error::IntOverflow("extents_size"))?;
+                    .ok_or(Error::IntOverflow("extents_size"))?;
                 let extents_end = extents_start
                     .checked_add(extents_size)
-                    .ok_or_else(|| Error::IntOverflow("extents_end"))?;
+                    .ok_or(Error::IntOverflow("extents_end"))?;
                 let extents_size: usize = util::try_cast(extents_size)
                     .map_err(|e| Error::IntOutOfBounds("extents_size", e))?;
 
-                let mut reader = input.reopen_boxed()?;
-                reader.seek(SeekFrom::Start(extents_start))?;
+                let mut reader = input.reopen_boxed().map_err(Error::ChunkRead)?;
+                reader
+                    .seek(SeekFrom::Start(extents_start))
+                    .map_err(Error::ChunkRead)?;
 
-                let mut data = vec![0u8; extents_size];
-
-                stream::check_cancel(cancel_signal)?;
-                reader.read_exact(&mut data)?;
+                stream::check_cancel(cancel_signal).map_err(Error::ChunkRead)?;
+                let data = reader
+                    .read_vec_exact(extents_size)
+                    .map_err(Error::ChunkRead)?;
 
                 let was_modified = util::ranges_overlaps(ranges, &(extents_start..extents_end));
 
@@ -1221,7 +1282,8 @@ pub fn compress_modified_image(
 
                 Ok(i..i + 1)
             })
-            .collect::<Result<Vec<_>>>()?;
+            .collect::<io::Result<Vec<_>>>()
+            .map_err(Error::ChunkWrite)?;
 
         modified_operations.extend(modified_group_operations);
     }

avbroot/src/format/sparse.rs

@@ -96,8 +96,10 @@ pub enum Error {
     #[error("Gap between end of last chunk {prev_end} and start of new chunk {cur_start}")]
     GapBetweenChunks { prev_end: u32, cur_start: u32 },
     // Wrapped errors.
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to read sparse data: {0}")]
+    DataRead(&'static str, #[source] io::Error),
+    #[error("Failed to write sparse data: {0}")]
+    DataWrite(&'static str, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -694,11 +696,14 @@ impl<R: Read> SparseReader<R> {
     /// data chunks if they are not needed. If the underlying file is seekable
     /// and skipping chunks is needed, use [`Self::new_seekable`] instead.
     pub fn new(mut inner: R, crc_mode: CrcMode) -> Result<Self> {
-        let header = RawHeader::read_from_io(&mut inner)?;
+        let header =
+            RawHeader::read_from_io(&mut inner).map_err(|e| Error::DataRead("header", e))?;
 
         header.validate()?;
 
-        inner.read_discard(header.excess_raw_header_bytes().into())?;
+        inner
+            .read_discard(header.excess_raw_header_bytes().into())
+            .map_err(|e| Error::DataRead("header_excess", e))?;
 
         Ok(Self {
             inner,
@@ -741,7 +746,8 @@ impl<R: Read> SparseReader<R> {
                     return Err(Error::Crc32RandomRead);
                 }
 
-                seek(&mut self.inner, SeekFrom::Current(self.data_remain.into()))?;
+                seek(&mut self.inner, SeekFrom::Current(self.data_remain.into()))
+                    .map_err(|e| Error::DataRead("data_remain", e))?;
                 self.data_remain = 0;
             } else {
                 return Err(Error::UnreadChunkData(self.data_remain));
@@ -752,12 +758,14 @@ impl<R: Read> SparseReader<R> {
             return Ok(None);
         }
 
-        let raw_chunk = RawChunk::read_from_io(&mut self.inner)?;
+        let raw_chunk =
+            RawChunk::read_from_io(&mut self.inner).map_err(|e| Error::DataRead("chunk", e))?;
 
         raw_chunk.validate(self.chunk, &self.header, self.block)?;
 
         self.inner
-            .read_discard(self.header.excess_raw_chunk_bytes().into())?;
+            .read_discard(self.header.excess_raw_chunk_bytes().into())
+            .map_err(|e| Error::DataRead("chunk_excess", e))?;
 
         let data: ChunkData;
 
@@ -769,7 +777,8 @@ impl<R: Read> SparseReader<R> {
                 data = ChunkData::Data;
             }
             CHUNK_TYPE_FILL => {
-                let fill_value = little_endian::U32::read_from_io(&mut self.inner)?;
+                let fill_value = little_endian::U32::read_from_io(&mut self.inner)
+                    .map_err(|e| Error::DataRead("chunk_fill_value", e))?;
 
                 if let Some(hasher) = &mut self.hasher {
                     hash_fill_chunk(&raw_chunk, fill_value, &self.header, hasher);
@@ -785,7 +794,8 @@ impl<R: Read> SparseReader<R> {
                 data = ChunkData::Hole;
             }
             CHUNK_TYPE_CRC32 => {
-                let expected = little_endian::U32::read_from_io(&mut self.inner)?;
+                let expected = little_endian::U32::read_from_io(&mut self.inner)
+                    .map_err(|e| Error::DataRead("chunk_crc32", e))?;
 
                 if let Some(hasher) = &mut self.hasher {
                     let actual = hasher.clone().finalize();
@@ -886,7 +896,9 @@ impl<W: Write> SparseWriter<W> {
 
         header.validate()?;
 
-        header.write_to_io(&mut inner)?;
+        header
+            .write_to_io(&mut inner)
+            .map_err(|e| Error::DataWrite("header", e))?;
 
         Ok(Self {
             inner,
@@ -937,7 +949,9 @@ impl<W: Write> SparseWriter<W> {
         self.chunk += 1;
         self.block = chunk.bounds.end;
 
-        raw_chunk.write_to_io(&mut self.inner)?;
+        raw_chunk
+            .write_to_io(&mut self.inner)
+            .map_err(|e| Error::DataWrite("chunk", e))?;
 
         match chunk.data {
             ChunkData::Data => {
@@ -945,7 +959,9 @@ impl<W: Write> SparseWriter<W> {
                     raw_chunk.total_sz.get() - u32::from(self.header.chunk_hdr_sz.get());
             }
             ChunkData::Fill(fill_value) => {
-                self.inner.write_all(&fill_value.to_le_bytes())?;
+                self.inner
+                    .write_all(&fill_value.to_le_bytes())
+                    .map_err(|e| Error::DataWrite("chunk_fill_value", e))?;
 
                 hash_fill_chunk(
                     &raw_chunk,
@@ -958,7 +974,9 @@ impl<W: Write> SparseWriter<W> {
                 hash_fill_chunk(&raw_chunk, 0.into(), &self.header, &mut self.hasher);
             }
             ChunkData::Crc32(expected) => {
-                self.inner.write_all(&expected.to_le_bytes())?;
+                self.inner
+                    .write_all(&expected.to_le_bytes())
+                    .map_err(|e| Error::DataWrite("chunk_crc32", e))?;
 
                 let actual = self.hasher.clone().finalize();
                 if actual != expected {

avbroot/src/patch/boot.rs

@@ -4,12 +4,13 @@
 use std::{
     cmp::Ordering,
     collections::{HashMap, HashSet},
+    fmt::Write,
     fs::File,
-    io::{self, BufRead, BufReader, Cursor, Read, Seek},
+    io::{self, BufRead, BufReader, Cursor, Read},
     num::ParseIntError,
-    ops::Range,
+    ops::{Range, RangeFrom},
     path::{Path, PathBuf},
-    slice,
+    slice, str,
     sync::atomic::AtomicBool,
 };
 
@@ -41,8 +42,6 @@ use crate::{
 
 #[derive(Debug, Error)]
 pub enum Error {
-    #[error("No compatible boot image found for {0}")]
-    NoTargets(&'static str),
     #[error("Boot image has no vbmeta footer")]
     NoFooter,
     #[error("No hash descriptor found in vbmeta header")]
@@ -53,41 +52,80 @@ pub enum Error {
     ParseMagiskVersion(String, #[source] ParseIntError),
     #[error("Failed to determine Magisk version from: {0:?}")]
     FindMagiskVersion(PathBuf),
-    #[error("AVB error")]
-    Avb(#[from] avb::Error),
-    #[error("Boot image error")]
-    BootImage(#[from] bootimage::Error),
-    #[error("Compression error")]
-    Compression(#[from] compression::Error),
-    #[error("Crypto error")]
-    Crypto(#[from] crypto::Error),
-    #[error("CPIO error")]
-    Cpio(#[from] cpio::Error),
-    #[error("OTA certificate error")]
-    OtaCert(#[from] otacert::Error),
-    #[error("XZ stream error")]
-    XzStream(#[from] liblzma::stream::Error),
-    #[error("Zip error")]
-    Zip(#[source] ZipError),
-    #[error("Zip error for entry name: {0:?}")]
-    ZipEntryName(String, #[source] ZipError),
-    #[error("Zip error for entry index #{0}")]
-    ZipEntryIndex(usize, #[source] ZipError),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
-    #[error("File I/O error")]
-    File(PathBuf, #[source] io::Error),
+    #[error("Failed to load potentially compressed ramdisk")]
+    RamdiskLoadCompression(#[source] compression::Error),
+    #[error("Failed to save potentially compressed ramdisk")]
+    RamdiskSaveCompression(#[source] compression::Error),
+    #[error("Failed to finalize compressed ramdisk")]
+    RamdiskSaveCompressionFinalize(#[source] io::Error),
+    #[error("Failed to load ramdisk cpio entries")]
+    RamdiskLoadCpio(#[source] cpio::Error),
+    #[error("Failed to save ramdisk cpio entries")]
+    RamdiskSaveCpio(#[source] cpio::Error),
+    #[error("Failed to load potentially compressed kernel")]
+    KernelLoadCompression(#[source] compression::Error),
+    #[error("Failed to read kernel image")]
+    KernelRead(#[source] io::Error),
+    #[error("Failed to load boot image")]
+    BootImageLoad(#[source] bootimage::Error),
+    #[error("Failed to save boot image")]
+    BootImageSave(#[source] bootimage::Error),
+    #[error("Failed to seek boot image")]
+    BootImageSeek(#[source] io::Error),
+    #[error("Failed to encode public key in AVB binary format")]
+    AvbEncodeKey(#[source] avb::Error),
+    #[error("Failed to load AVB header from boot image")]
+    AvbLoad(#[source] avb::Error),
+    #[error("Failed to update AVB header for boot image")]
+    AvbUpdate(#[source] avb::Error),
+    #[error("Failed to load OTA certificate")]
+    OtaCertLoad(#[source] crypto::Error),
+    #[error("Failed to generate replacement otacerts zip")]
+    OtaCertZip(#[source] otacert::Error),
+    #[error("Failed to initialize XZ encoder")]
+    XzInit(#[source] liblzma::stream::Error),
+    #[error("Failed to XZ compress entry: {:?}", .0.as_bstr())]
+    XzCompress(Vec<u8>, #[source] io::Error),
+    #[error("Failed to open zip file: {0:?}")]
+    ZipOpen(PathBuf, #[source] ZipError),
+    #[error("Failed to open zip entry: {0:?}")]
+    ZipEntryOpen(&'static str, #[source] ZipError),
+    #[error("Failed to read zip entry: {0:?}")]
+    ZipEntryRead(&'static str, #[source] io::Error),
+    #[error("Failed to open zip entry #{0}")]
+    ZipIndexOpen(usize, #[source] ZipError),
+    #[error("Failed to open file: {0:?}")]
+    FileOpen(PathBuf, #[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
 
+#[derive(Debug, Error)]
+pub enum TargetsError {
+    #[error("No compatible boot image found for {0}")]
+    NoTargets(&'static str),
+    #[error("Targets validation error: {0}")]
+    TargetValidation(String),
+    #[error("Failed to open boot image: {0}")]
+    Open(String, #[source] io::Error),
+    #[error("Failed to load boot image: {0}")]
+    Load(String, #[source] Error),
+    #[error("Failed to save boot image: {0}")]
+    Save(String, #[source] Error),
+    #[error("Failed to patch boot image: {0}")]
+    Patch(String, #[source] Error),
+}
+
+type TargetsResult<T> = std::result::Result<T, TargetsError>;
+
 fn load_ramdisk(
     data: &[u8],
     cancel_signal: &AtomicBool,
 ) -> Result<(Vec<CpioEntry>, CompressedFormat)> {
     let raw_reader = Cursor::new(data);
-    let mut reader = CompressedReader::new(raw_reader, false)?;
-    let entries = cpio::load(&mut reader, false, cancel_signal)?;
+    let mut reader =
+        CompressedReader::new(raw_reader, false).map_err(Error::RamdiskLoadCompression)?;
+    let entries = cpio::load(&mut reader, false, cancel_signal).map_err(Error::RamdiskLoadCpio)?;
 
     trace!(
         "Loaded {:?} ramdisk with {} entries",
@@ -104,12 +142,15 @@ fn save_ramdisk(
     cancel_signal: &AtomicBool,
 ) -> Result<Vec<u8>> {
     let raw_writer = Cursor::new(vec![]);
-    let mut writer = CompressedWriter::new(raw_writer, format)?;
-    cpio::save(&mut writer, entries, false, cancel_signal)?;
+    let mut writer =
+        CompressedWriter::new(raw_writer, format).map_err(Error::RamdiskSaveCompression)?;
+    cpio::save(&mut writer, entries, false, cancel_signal).map_err(Error::RamdiskSaveCpio)?;
 
     trace!("Wrote {format:?} ramdisk with {} entries", entries.len());
 
-    let raw_writer = writer.finish()?;
+    let raw_writer = writer
+        .finish()
+        .map_err(Error::RamdiskSaveCompressionFinalize)?;
     Ok(raw_writer.into_inner())
 }
 
@@ -130,7 +171,7 @@ pub trait BootImagePatch {
         &self,
         boot_images: &HashMap<&'a str, BootImageInfo>,
         cancel_signal: &AtomicBool,
-    ) -> Result<Vec<&'a str>>;
+    ) -> TargetsResult<Vec<&'a str>>;
 
     fn patch(&self, boot_image: &mut BootImage, cancel_signal: &AtomicBool) -> Result<()>;
 }
@@ -151,13 +192,13 @@ impl MagiskRootPatcher {
     //   RULESDEVICE config option, which stored the writable block device as an
     //   rdev major/minor pair, which was not consistent across reboots and was
     //   replaced by PREINITDEVICE
+    // - Versions newer than the latest supported version are assumed to support
+    //   the same features as the latest version
     const VERS_SUPPORTED: &'static [Range<u32>] = &[25102..25207, 25211..28200];
-    const VER_PREINIT_DEVICE: Range<u32> =
-        25211..Self::VERS_SUPPORTED[Self::VERS_SUPPORTED.len() - 1].end;
+    const VER_PREINIT_DEVICE: RangeFrom<u32> = 25211..;
     const VER_RANDOM_SEED: Range<u32> = 25211..26103;
     const VER_PATCH_VBMETA: Range<u32> = Self::VERS_SUPPORTED[0].start..26202;
-    const VER_XZ_BACKUP: Range<u32> =
-        26403..Self::VERS_SUPPORTED[Self::VERS_SUPPORTED.len() - 1].end;
+    const VER_XZ_BACKUP: RangeFrom<u32> = 26403..;
 
     const ZIP_INIT_LD: &'static str = "lib/arm64-v8a/libinit-ld.so";
     const ZIP_LIBMAGISK: &'static str = "lib/arm64-v8a/libmagisk.so";
@@ -216,18 +257,21 @@ impl MagiskRootPatcher {
     }
 
     fn get_version(path: &Path) -> Result<u32> {
-        let reader = File::open(path).map_err(|e| Error::File(path.to_owned(), e))?;
-        let reader = BufReader::new(reader);
-        let mut zip = ZipArchive::new(reader).map_err(Error::Zip)?;
+        let reader = File::open(path)
+            .map(BufReader::new)
+            .map_err(|e| Error::FileOpen(path.to_owned(), e))?;
+        let mut zip = ZipArchive::new(reader).map_err(|e| Error::ZipOpen(path.to_owned(), e))?;
         let entry = zip
             .by_name(Self::ZIP_UTIL_FUNCTIONS)
-            .map_err(|e| Error::ZipEntryName(Self::ZIP_UTIL_FUNCTIONS.to_owned(), e))?;
+            .map_err(|e| Error::ZipEntryOpen(Self::ZIP_UTIL_FUNCTIONS, e))?;
         let mut entry = BufReader::new(entry);
         let mut line = String::new();
 
         loop {
             line.clear();
-            let n = entry.read_line(&mut line)?;
+            let n = entry
+                .read_line(&mut line)
+                .map_err(|e| Error::ZipEntryRead(Self::ZIP_UTIL_FUNCTIONS, e))?;
             if n == 0 {
                 return Err(Error::FindMagiskVersion(path.to_owned()));
             }
@@ -243,14 +287,14 @@ impl MagiskRootPatcher {
         }
     }
 
-    fn xz_compress(reader: impl Read, cancel_signal: &AtomicBool) -> Result<Vec<u8>> {
-        let stream = Stream::new_easy_encoder(9, Check::Crc32)?;
+    fn xz_compress(name: &[u8], reader: impl Read, cancel_signal: &AtomicBool) -> Result<Vec<u8>> {
+        let stream = Stream::new_easy_encoder(9, Check::Crc32).map_err(Error::XzInit)?;
         let raw_writer = Cursor::new(Vec::new());
         let mut writer = XzEncoder::new_stream(raw_writer, stream);
 
-        stream::copy(reader, &mut writer, cancel_signal)?;
-
-        let raw_writer = writer.finish()?;
+        let raw_writer = stream::copy(reader, &mut writer, cancel_signal)
+            .and_then(|_| writer.finish())
+            .map_err(|e| Error::XzCompress(name.to_owned(), e))?;
 
         Ok(raw_writer.into_inner())
     }
@@ -339,7 +383,7 @@ impl MagiskRootPatcher {
                     new_path.extend(b".xz");
 
                     let reader = Cursor::new(data);
-                    let buf = Self::xz_compress(reader, cancel_signal)?;
+                    let buf = Self::xz_compress(&new_path, reader, cancel_signal)?;
                     new_data = Some(CpioEntryData::Data(buf));
                 }
             }
@@ -381,7 +425,7 @@ impl BootImagePatch for MagiskRootPatcher {
         &self,
         boot_images: &HashMap<&'a str, BootImageInfo>,
         _cancel_signal: &AtomicBool,
-    ) -> Result<Vec<&'a str>> {
+    ) -> TargetsResult<Vec<&'a str>> {
         let mut targets = vec![];
 
         if boot_images.contains_key("init_boot") {
@@ -394,9 +438,11 @@ impl BootImagePatch for MagiskRootPatcher {
     }
 
     fn patch(&self, boot_image: &mut BootImage, cancel_signal: &AtomicBool) -> Result<()> {
-        let zip_reader =
-            File::open(&self.apk_path).map_err(|e| Error::File(self.apk_path.clone(), e))?;
-        let mut zip = ZipArchive::new(BufReader::new(zip_reader)).map_err(Error::Zip)?;
+        let zip_reader = File::open(&self.apk_path)
+            .map(BufReader::new)
+            .map_err(|e| Error::FileOpen(self.apk_path.clone(), e))?;
+        let mut zip =
+            ZipArchive::new(zip_reader).map_err(|e| Error::ZipOpen(self.apk_path.clone(), e))?;
 
         // Load the first ramdisk. If it doesn't exist, we have to generate one
         // from scratch.
@@ -427,9 +473,11 @@ impl BootImagePatch for MagiskRootPatcher {
         {
             let mut zip_entry = zip
                 .by_name(Self::ZIP_MAGISKINIT)
-                .map_err(|e| Error::ZipEntryName(Self::ZIP_MAGISKINIT.to_owned(), e))?;
+                .map_err(|e| Error::ZipEntryOpen(Self::ZIP_MAGISKINIT, e))?;
             let mut data = vec![];
-            zip_entry.read_to_end(&mut data)?;
+            zip_entry
+                .read_to_end(&mut data)
+                .map_err(|e| Error::ZipEntryRead(Self::ZIP_MAGISKINIT, e))?;
 
             entries.push(CpioEntry::new_file(
                 b"init",
@@ -471,8 +519,8 @@ impl BootImagePatch for MagiskRootPatcher {
         for (source, target) in xz_files {
             let reader = zip
                 .by_name(source)
-                .map_err(|e| Error::ZipEntryName(source.to_owned(), e))?;
-            let buf = Self::xz_compress(reader, cancel_signal)?;
+                .map_err(|e| Error::ZipEntryOpen(source, e))?;
+            let buf = Self::xz_compress(source.as_bytes(), reader, cancel_signal)?;
 
             entries.push(CpioEntry::new_file(target, 0o644, CpioEntryData::Data(buf)));
         }
@@ -497,10 +545,9 @@ impl BootImagePatch for MagiskRootPatcher {
         magisk_config.push_str("RECOVERYMODE=false\n");
 
         if Self::VER_PREINIT_DEVICE.contains(&self.version) {
-            magisk_config.push_str(&format!(
-                "PREINITDEVICE={}\n",
-                self.preinit_device.as_ref().unwrap(),
-            ));
+            if let Some(device) = &self.preinit_device {
+                writeln!(&mut magisk_config, "PREINITDEVICE={device}").unwrap();
+            }
         }
 
         // Magisk normally saves the original SHA1 digest in its config file. It
@@ -510,7 +557,7 @@ impl BootImagePatch for MagiskRootPatcher {
         magisk_config.push_str("SHA1=0000000000000000000000000000000000000000\n");
 
         if Self::VER_RANDOM_SEED.contains(&self.version) {
-            magisk_config.push_str(&format!("RANDOMSEED={:#x}\n", self.random_seed));
+            writeln!(&mut magisk_config, "RANDOMSEED={:#x}", self.random_seed).unwrap();
         }
 
         trace!("Magisk config: {magisk_config:?}");
@@ -523,7 +570,7 @@ impl BootImagePatch for MagiskRootPatcher {
 
         // Repack ramdisk.
         cpio::sort(&mut entries);
-        cpio::assign_inodes(&mut entries, false)?;
+        cpio::assign_inodes(&mut entries, false).map_err(Error::RamdiskSaveCpio)?;
         let new_ramdisk = save_ramdisk(&entries, ramdisk_format, cancel_signal)?;
 
         match boot_image {
@@ -590,18 +637,22 @@ impl OtaCertPatcher {
                 continue;
             };
 
-            let mut zip = ZipArchive::new(Cursor::new(&data)).map_err(Error::Zip)?;
+            let mut zip = ZipArchive::new(Cursor::new(&data)).map_err(|e| {
+                Error::ZipOpen(str::from_utf8(Self::OTACERTS_PATH).unwrap().into(), e)
+            })?;
 
             for index in 0..zip.len() {
                 let zip_entry = zip
                     .by_index(index)
-                    .map_err(|e| Error::ZipEntryIndex(index, e))?;
+                    .map_err(|e| Error::ZipIndexOpen(index, e))?;
                 if !zip_entry.name().ends_with(".x509.pem") {
                     debug!("Skipping invalid entry path: {}", zip_entry.name());
                     continue;
                 }
 
-                let certificate = crypto::read_pem_cert(zip_entry)?;
+                let path = PathBuf::from(zip_entry.name());
+                let certificate =
+                    crypto::read_pem_cert(&path, zip_entry).map_err(Error::OtaCertLoad)?;
                 certificates.push(certificate);
             }
         }
@@ -639,10 +690,10 @@ impl BootImagePatch for OtaCertPatcher {
         &self,
         boot_images: &HashMap<&'a str, BootImageInfo>,
         cancel_signal: &AtomicBool,
-    ) -> Result<Vec<&'a str>> {
+    ) -> TargetsResult<Vec<&'a str>> {
         let mut targets = vec![];
 
-        'outer: for (name, info) in boot_images {
+        'outer: for (&name, info) in boot_images {
             let ramdisks = match &info.boot_image {
                 BootImage::V0Through2(b) => slice::from_ref(&b.ramdisk),
                 BootImage::V3Through4(b) => slice::from_ref(&b.ramdisk),
@@ -654,9 +705,10 @@ impl BootImagePatch for OtaCertPatcher {
                     continue;
                 }
 
-                let (entries, _) = load_ramdisk(ramdisk, cancel_signal)?;
+                let (entries, _) = load_ramdisk(ramdisk, cancel_signal)
+                    .map_err(|e| TargetsError::Load(name.to_owned(), e))?;
                 if entries.iter().any(|e| e.path == Self::OTACERTS_PATH) {
-                    targets.push(*name);
+                    targets.push(name);
                     continue 'outer;
                 }
             }
@@ -672,7 +724,8 @@ impl BootImagePatch for OtaCertPatcher {
             BootImage::VendorV3Through4(b) => &mut b.ramdisks,
         };
 
-        let new_zip = otacert::create_zip(&self.cert, OtaCertBuildFlags::empty())?;
+        let new_zip = otacert::create_zip(&self.cert, OtaCertBuildFlags::empty())
+            .map_err(Error::OtaCertZip)?;
         trace!("Generated new {} byte otacerts.zip", new_zip.len());
 
         for ramdisk in ramdisks {
@@ -718,7 +771,8 @@ impl DsuPubKeyPatcher {
             entries.push(CpioEntry::new_directory(Self::DSU_KEYS_PATH, 0o755));
         }
 
-        let data = CpioEntryData::Data(avb::encode_public_key(&self.key)?);
+        let binary_key = avb::encode_public_key(&self.key).map_err(Error::AvbEncodeKey)?;
+        let data = CpioEntryData::Data(binary_key);
 
         if let Some(e) = entries
             .iter_mut()
@@ -744,11 +798,11 @@ impl BootImagePatch for DsuPubKeyPatcher {
         &self,
         boot_images: &HashMap<&'a str, BootImageInfo>,
         cancel_signal: &AtomicBool,
-    ) -> Result<Vec<&'a str>> {
+    ) -> TargetsResult<Vec<&'a str>> {
         let mut dsu_keys_targets = vec![];
         let mut first_stage_targets = vec![];
 
-        'outer: for (name, info) in boot_images {
+        'outer: for (&name, info) in boot_images {
             let ramdisks = match &info.boot_image {
                 BootImage::V0Through2(b) => slice::from_ref(&b.ramdisk),
                 BootImage::V3Through4(b) => slice::from_ref(&b.ramdisk),
@@ -760,15 +814,16 @@ impl BootImagePatch for DsuPubKeyPatcher {
                     continue;
                 }
 
-                let (entries, _) = load_ramdisk(ramdisk, cancel_signal)?;
+                let (entries, _) = load_ramdisk(ramdisk, cancel_signal)
+                    .map_err(|e| TargetsError::Load(name.to_owned(), e))?;
                 let mut found = false;
 
                 for entry in entries {
                     if entry.path == Self::DSU_KEYS_PATH {
-                        dsu_keys_targets.push(*name);
+                        dsu_keys_targets.push(name);
                         found = true;
                     } else if entry.path == Self::FIRST_STAGE_PATH {
-                        first_stage_targets.push(*name);
+                        first_stage_targets.push(name);
                         found = true;
                     }
                 }
@@ -783,7 +838,7 @@ impl BootImagePatch for DsuPubKeyPatcher {
             // Most builds trust as least one DSU key. For these builds, add the
             // user's key to the same directory.
             if dsu_keys_targets.len() > 1 {
-                return Err(Error::Validation(format!(
+                return Err(TargetsError::TargetValidation(format!(
                     "DSU keys found in more than one boot image: {dsu_keys_targets:?}",
                 )));
             }
@@ -852,19 +907,22 @@ impl PrepatchedImagePatcher {
     }
 
     fn load_prepatched_image(&self) -> Result<BootImage> {
-        let raw_reader =
-            File::open(&self.prepatched).map_err(|e| Error::File(self.prepatched.clone(), e))?;
-        let boot_image = BootImage::from_reader(BufReader::new(raw_reader))?;
+        let reader = File::open(&self.prepatched)
+            .map(BufReader::new)
+            .map_err(|e| Error::FileOpen(self.prepatched.clone(), e))?;
 
-        Ok(boot_image)
+        BootImage::from_reader(reader).map_err(Error::BootImageLoad)
     }
 
     fn get_kmi_version(kernel: &[u8]) -> Result<Option<String>> {
         let mut decompressed = vec![];
         {
             let raw_reader = Cursor::new(kernel);
-            let mut reader = CompressedReader::new(raw_reader, true)?;
-            reader.read_to_end(&mut decompressed)?;
+            let mut reader =
+                CompressedReader::new(raw_reader, true).map_err(Error::KernelLoadCompression)?;
+            reader
+                .read_to_end(&mut decompressed)
+                .map_err(Error::KernelRead)?;
         }
 
         let regex = Regex::new(Self::VERSION_REGEX).unwrap();
@@ -896,8 +954,10 @@ impl BootImagePatch for PrepatchedImagePatcher {
         &self,
         boot_images: &HashMap<&'a str, BootImageInfo>,
         _cancel_signal: &AtomicBool,
-    ) -> Result<Vec<&'a str>> {
-        let prepatched_image = self.load_prepatched_image()?;
+    ) -> TargetsResult<Vec<&'a str>> {
+        let prepatched_image = self
+            .load_prepatched_image()
+            .map_err(|e| TargetsError::Load("prepatched".to_owned(), e))?;
 
         let has_kernel = match prepatched_image {
             BootImage::V0Through2(b) => !b.kernel.is_empty(),
@@ -1084,36 +1144,90 @@ impl BootImagePatch for PrepatchedImagePatcher {
     }
 }
 
+fn load_boot_image(reader: &mut dyn ReadSeek) -> Result<BootImageInfo> {
+    let (header, footer, image_size) = avb::load_image(&mut *reader).map_err(Error::AvbLoad)?;
+    let Some(footer) = footer else {
+        return Err(Error::NoFooter);
+    };
+
+    let section_reader =
+        SectionReader::new(reader, 0, footer.original_image_size).map_err(Error::BootImageSeek)?;
+    let boot_image = BootImage::from_reader(section_reader).map_err(Error::BootImageLoad)?;
+
+    let info = BootImageInfo {
+        header,
+        footer,
+        image_size,
+        boot_image,
+    };
+
+    trace!("Loaded {image_size} byte boot image");
+
+    Ok(info)
+}
+
+fn save_boot_image(
+    writer: &mut dyn WriteSeek,
+    info: &mut BootImageInfo,
+    key: &RsaSigningKey,
+) -> Result<()> {
+    let AppendedDescriptorMut::Hash(descriptor) = info
+        .header
+        .appended_descriptor_mut()
+        .map_err(Error::AvbUpdate)?
+    else {
+        return Err(Error::NoHashDescriptor);
+    };
+
+    // Write new boot image. We reuse the existing salt for the digest.
+    let mut context = Context::new(&ring::digest::SHA256);
+    context.update(&descriptor.salt);
+    let mut hashing_writer = HashingWriter::new(writer, context);
+    info.boot_image
+        .to_writer(&mut hashing_writer)
+        .map_err(Error::BootImageSave)?;
+    let (writer, context) = hashing_writer.finish();
+
+    descriptor.image_size = writer.stream_position().map_err(Error::BootImageSeek)?;
+    "sha256".clone_into(&mut descriptor.hash_algorithm);
+    descriptor.root_digest = context.finish().as_ref().to_vec();
+
+    if !info.header.public_key.is_empty() {
+        debug!("Signing boot image");
+        info.header
+            .set_algo_for_key(key)
+            .map_err(Error::AvbUpdate)?;
+        info.header.sign(key).map_err(Error::AvbUpdate)?;
+    }
+
+    avb::write_appended_image(
+        writer,
+        &info.header,
+        &mut info.footer,
+        Some(info.image_size),
+    )
+    .map_err(Error::AvbUpdate)?;
+
+    Ok(())
+}
+
 pub fn load_boot_images<'a>(
     names: &[&'a str],
     open_input: impl Fn(&str) -> io::Result<Box<dyn ReadSeek>> + Sync,
-) -> Result<HashMap<&'a str, BootImageInfo>> {
+) -> TargetsResult<HashMap<&'a str, BootImageInfo>> {
     let parent_span = Span::current();
 
     names
         .par_iter()
-        .map(|name| {
+        .map(|&name| {
             let _span = debug_span!(parent: &parent_span, "image", name).entered();
-            let mut reader = open_input(name)?;
+            let mut reader =
+                open_input(name).map_err(|e| TargetsError::Open(name.to_owned(), e))?;
 
-            let (header, footer, image_size) = avb::load_image(&mut reader)?;
-            let Some(footer) = footer else {
-                return Err(Error::NoFooter);
-            };
+            let info =
+                load_boot_image(&mut reader).map_err(|e| TargetsError::Load(name.to_owned(), e))?;
 
-            let section_reader = SectionReader::new(reader, 0, footer.original_image_size)?;
-            let boot_image = BootImage::from_reader(section_reader)?;
-
-            let info = BootImageInfo {
-                header,
-                footer,
-                image_size,
-                boot_image,
-            };
-
-            trace!("Loaded {image_size} byte boot image: {name}");
-
-            Ok((*name, info))
+            Ok((name, info))
         })
         .collect()
 }
@@ -1130,7 +1244,7 @@ pub fn patch_boot_images<'a>(
     key: &RsaSigningKey,
     patchers: &[Box<dyn BootImagePatch + Sync>],
     cancel_signal: &AtomicBool,
-) -> Result<HashSet<&'a str>> {
+) -> TargetsResult<HashSet<&'a str>> {
     let parent_span = Span::current();
 
     if patchers.is_empty() {
@@ -1149,14 +1263,14 @@ pub fn patch_boot_images<'a>(
                 debug_span!(parent: &parent_span, "patcher", name = p.patcher_name()).entered();
             p.find_targets(&images, cancel_signal).and_then(|targets| {
                 if targets.is_empty() {
-                    Err(Error::NoTargets(p.patcher_name()))
+                    Err(TargetsError::NoTargets(p.patcher_name()))
                 } else {
                     debug!("Found patcher targets: {targets:?}");
                     Ok(targets)
                 }
             })
         })
-        .collect::<Result<Vec<_>>>()?;
+        .collect::<TargetsResult<Vec<_>>>()?;
 
     debug!("All patcher targets: {all_targets:?}");
 
@@ -1178,54 +1292,22 @@ pub fn patch_boot_images<'a>(
     // Apply all patches.
     groups
         .par_iter_mut()
-        .try_for_each(|(_, (info, patchers))| -> Result<()> {
+        .try_for_each(|(&name, (info, patchers))| -> TargetsResult<()> {
             patchers.iter().try_for_each(|p| {
                 let _span =
                     debug_span!(parent: &parent_span, "patcher", name = p.patcher_name()).entered();
                 p.patch(&mut info.boot_image, cancel_signal)
+                    .map_err(|e| TargetsError::Patch(name.to_owned(), e))
             })
         })?;
 
     // Resign and write new images.
-    groups
-        .par_iter_mut()
-        .map(|(name, (info, _))| {
-            let _span = debug_span!(parent: &parent_span, "image", name).entered();
+    groups.par_iter_mut().try_for_each(|(&name, (info, _))| {
+        let _span = debug_span!(parent: &parent_span, "image", name).entered();
+        let mut writer = open_output(name).map_err(|e| TargetsError::Open(name.to_owned(), e))?;
 
-            let AppendedDescriptorMut::Hash(descriptor) = info.header.appended_descriptor_mut()?
-            else {
-                return Err(Error::NoHashDescriptor);
-            };
-
-            let writer = open_output(name)?;
-
-            // Write new boot image. We reuse the existing salt for the digest.
-            let mut context = Context::new(&ring::digest::SHA256);
-            context.update(&descriptor.salt);
-            let mut hashing_writer = HashingWriter::new(writer, context);
-            info.boot_image.to_writer(&mut hashing_writer)?;
-            let (mut writer, context) = hashing_writer.finish();
-
-            descriptor.image_size = writer.stream_position()?;
-            "sha256".clone_into(&mut descriptor.hash_algorithm);
-            descriptor.root_digest = context.finish().as_ref().to_vec();
-
-            if !info.header.public_key.is_empty() {
-                debug!("Signing boot image");
-                info.header.set_algo_for_key(key)?;
-                info.header.sign(key)?;
-            }
-
-            avb::write_appended_image(
-                writer,
-                &info.header,
-                &mut info.footer,
-                Some(info.image_size),
-            )?;
-
-            Ok(())
-        })
-        .collect::<Result<()>>()?;
+        save_boot_image(&mut writer, info, key).map_err(|e| TargetsError::Save(name.to_owned(), e))
+    })?;
 
     Ok(groups.keys().copied().collect())
 }

avbroot/src/patch/otacert.rs

@@ -1,7 +1,7 @@
-// SPDX-FileCopyrightText: 2023 Andrew Gunnerson
+// SPDX-FileCopyrightText: 2023-2024 Andrew Gunnerson
 // SPDX-License-Identifier: GPL-3.0-only
 
-use std::{borrow::Cow, cmp::Ordering, io::Cursor};
+use std::{borrow::Cow, cmp::Ordering, io::Cursor, path::Path};
 
 use bitflags::bitflags;
 use thiserror::Error;
@@ -17,12 +17,10 @@ pub enum Error {
     ZipTooSmall(usize),
     #[error("New otacerts.zip is too large to fit in {0} bytes")]
     ZipTooLarge(usize),
-    #[error("Crypto error")]
-    Crypto(#[from] crypto::Error),
-    #[error("x509 DER error")]
-    Der(#[from] x509_cert::der::Error),
-    #[error("Zip error")]
-    Zip(#[from] ZipError),
+    #[error("Failed to write otacerts zip")]
+    ZipWrite(#[source] ZipError),
+    #[error("Failed to write certificate to otacerts zip")]
+    CertWrite(#[source] crypto::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -82,7 +80,8 @@ pub fn create_zip(cert: &Certificate, flags: OtaCertBuildFlags) -> Result<Vec<u8
     };
 
     let options = FileOptions::default().compression_method(compression_method);
-    writer.start_file("ota.x509.pem", options)?;
+    let name = "ota.x509.pem";
+    writer.start_file(name, options).map_err(Error::ZipWrite)?;
 
     let cert = if flags.is_empty() {
         Cow::Borrowed(cert)
@@ -90,7 +89,9 @@ pub fn create_zip(cert: &Certificate, flags: OtaCertBuildFlags) -> Result<Vec<u8
         let mut modified = cert.clone();
 
         if flags.contains(OtaCertBuildFlags::REMOVE_SIGNATURE) {
-            modified.signature = BitString::from_bytes(&[])?;
+            // An empty ASN.1 bit string is always valid.
+            modified.signature =
+                BitString::from_bytes(&[]).expect("Empty ASN.1 bit string was invalid");
         }
         if flags.contains(OtaCertBuildFlags::REMOVE_EXTENSIONS) {
             if let Some(extensions) = &mut modified.tbs_certificate.extensions {
@@ -109,9 +110,9 @@ pub fn create_zip(cert: &Certificate, flags: OtaCertBuildFlags) -> Result<Vec<u8
         Cow::Owned(modified)
     };
 
-    crypto::write_pem_cert(&mut writer, &cert)?;
+    crypto::write_pem_cert(Path::new(name), &mut writer, &cert).map_err(Error::CertWrite)?;
 
-    let raw_writer = writer.finish()?;
+    let raw_writer = writer.finish().map_err(Error::ZipWrite)?;
 
     Ok(raw_writer.into_inner())
 }

avbroot/src/patch/system.rs

@@ -21,7 +21,7 @@ use crate::{
         ota,
     },
     patch::otacert,
-    stream::{self, ReadSeekReopen, SectionReader, WriteSeekReopen},
+    stream::{self, ReadFixedSizeExt, ReadSeekReopen, SectionReader, WriteSeekReopen},
     util,
 };
 
@@ -35,12 +35,14 @@ pub enum Error {
     NoHashTreeDescriptor,
     #[error("{0:?} overflowed integer bounds during calculations")]
     IntOverflow(&'static str),
-    #[error("AVB error")]
-    Avb(#[from] avb::Error),
-    #[error("OTA certificate error")]
-    OtaCert(#[from] otacert::Error),
-    #[error("I/O error")]
-    Io(#[from] io::Error),
+    #[error("Failed to update AVB header")]
+    AvbUpdate(#[source] avb::Error),
+    #[error("Failed to generate replacement otacerts zip")]
+    OtaCertZip(#[source] otacert::Error),
+    #[error("Failed to read image data")]
+    ReadData(#[source] io::Error),
+    #[error("Failed to write image data")]
+    WriteData(#[source] io::Error),
 }
 
 type Result<T> = std::result::Result<T, Error>;
@@ -119,11 +121,15 @@ pub fn patch_system_image(
 
     let parent_span = Span::current();
 
-    let (mut header, footer, image_size) = avb::load_image(input.reopen_boxed()?)?;
+    let (mut header, footer, image_size) =
+        avb::load_image(input.reopen_boxed().map_err(Error::ReadData)?)
+            .map_err(Error::AvbUpdate)?;
     let Some(mut footer) = footer else {
         return Err(Error::NoFooter);
     };
-    let AppendedDescriptorMut::HashTree(descriptor) = header.appended_descriptor_mut()? else {
+    let AppendedDescriptorMut::HashTree(descriptor) =
+        header.appended_descriptor_mut().map_err(Error::AvbUpdate)?
+    else {
         return Err(Error::NoHashTreeDescriptor);
     };
 
@@ -133,17 +139,20 @@ pub fn patch_system_image(
     let modified_ranges = (0..num_chunks)
         .into_par_iter()
         .map(|chunk| -> Result<Vec<Range<u64>>> {
-            stream::check_cancel(cancel_signal)?;
+            stream::check_cancel(cancel_signal).map_err(Error::ReadData)?;
 
             let offset = chunk * CHUNK_SIZE;
             let size = CHUNK_SIZE.min(footer.original_image_size - offset);
-            let mut buf = vec![0u8; size as usize];
 
-            let mut reader = input.reopen_boxed()?;
-            reader.seek(SeekFrom::Start(offset))?;
-            reader.read_exact(&mut buf)?;
+            let mut reader = input.reopen_boxed().map_err(Error::ReadData)?;
+            reader
+                .seek(SeekFrom::Start(offset))
+                .map_err(Error::ReadData)?;
+            let buf = reader
+                .read_vec_exact(size as usize)
+                .map_err(Error::ReadData)?;
 
-            let mut writer = output.reopen_boxed()?;
+            let mut writer = output.reopen_boxed().map_err(Error::WriteData)?;
             let mut ranges = Vec::<Range<u64>>::new();
 
             for eocd_offset_rel in memmem::find_iter(&buf, ota::ZIP_EOCD_MAGIC) {
@@ -155,14 +164,17 @@ pub fn patch_system_image(
                 };
 
                 let zip_size = bounds_rel.end - bounds_rel.start;
-                let new_zip = otacert::create_zip_with_size(certificate, zip_size)?;
+                let new_zip = otacert::create_zip_with_size(certificate, zip_size)
+                    .map_err(Error::OtaCertZip)?;
 
                 let bounds = offset + bounds_rel.start as u64..offset + bounds_rel.end as u64;
 
-                stream::check_cancel(cancel_signal)?;
+                stream::check_cancel(cancel_signal).map_err(Error::WriteData)?;
 
-                writer.seek(SeekFrom::Start(bounds.start))?;
-                writer.write_all(&new_zip)?;
+                writer
+                    .seek(SeekFrom::Start(bounds.start))
+                    .map_err(Error::WriteData)?;
+                writer.write_all(&new_zip).map_err(Error::WriteData)?;
 
                 ranges.push(bounds);
             }
@@ -196,18 +208,23 @@ pub fn patch_system_image(
         Some(modified_ranges.as_slice())
     };
 
-    descriptor.update(input, output, update_ranges, cancel_signal)?;
+    descriptor
+        .update(input, output, update_ranges, cancel_signal)
+        .map_err(Error::AvbUpdate)?;
 
     if !header.public_key.is_empty() {
         debug!("Signing system image");
-        header.set_algo_for_key(key)?;
-        header.sign(key)?;
+        header.set_algo_for_key(key).map_err(Error::AvbUpdate)?;
+        header.sign(key).map_err(Error::AvbUpdate)?;
     }
 
-    let writer = output.reopen_boxed()?;
-    avb::write_appended_image(writer, &header, &mut footer, Some(image_size))?;
+    let writer = output.reopen_boxed().map_err(Error::WriteData)?;
+    avb::write_appended_image(writer, &header, &mut footer, Some(image_size))
+        .map_err(Error::AvbUpdate)?;
 
-    let AppendedDescriptorMut::HashTree(descriptor) = header.appended_descriptor_mut()? else {
+    let AppendedDescriptorMut::HashTree(descriptor) =
+        header.appended_descriptor_mut().map_err(Error::AvbUpdate)?
+    else {
         return Err(Error::NoHashTreeDescriptor);
     };
 
@@ -215,15 +232,15 @@ pub fn patch_system_image(
     let hash_tree_end = descriptor
         .tree_offset
         .checked_add(descriptor.tree_size)
-        .ok_or_else(|| Error::IntOverflow("hash_tree_end"))?;
+        .ok_or(Error::IntOverflow("hash_tree_end"))?;
     let fec_data_end = descriptor
         .fec_offset
         .checked_add(descriptor.fec_size)
-        .ok_or_else(|| Error::IntOverflow("fec_data_end"))?;
+        .ok_or(Error::IntOverflow("fec_data_end"))?;
     let header_end = footer
         .vbmeta_offset
         .checked_add(footer.vbmeta_size)
-        .ok_or_else(|| Error::IntOverflow("avb_end"))?;
+        .ok_or(Error::IntOverflow("avb_end"))?;
     let footer_start = image_size - Footer::SIZE as u64;
 
     let other_ranges = util::merge_overlapping(&[

avbroot/src/util.rs

@@ -4,7 +4,9 @@
 use std::{
     cmp::Ordering,
     fmt, mem,
-    ops::{Bound, Range, RangeBounds},
+    ops::{
+        Bound, Range, RangeBounds, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive,
+    },
     path::Path,
 };
 
@@ -43,59 +45,118 @@ impl fmt::Debug for DebugString {
     }
 }
 
-/// A single bound in a range.
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum IntBound<T: PrimInt> {
-    Included(T),
-    Excluded(T),
+#[derive(Clone, Hash, PartialEq, Eq)]
+pub enum AnyRange<T> {
+    Range(Range<T>),
+    RangeFrom(RangeFrom<T>),
+    RangeFull(RangeFull),
+    RangeInclusive(RangeInclusive<T>),
+    RangeTo(RangeTo<T>),
+    RangeToInclusive(RangeToInclusive<T>),
 }
 
-/// A bounded primitive integer range. Unlike std's range types, this is a
-/// single type that can represent open, closed, and half-open intervals.
-#[derive(Clone, Copy, Debug)]
-pub struct IntRange<T: PrimInt> {
-    pub start: IntBound<T>,
-    pub end: IntBound<T>,
-}
-
-impl<T: PrimInt> IntRange<T> {
-    /// Returns [`None`] if the range bounds cannot be represented by `T`. If
-    /// the start and end of `range` are unbounded, then it gets converted to
-    /// [`IntBound::Included`] with `N`'s minimum or maximum value.
-    pub fn new<N: PrimInt, R: RangeBounds<N>>(range: R) -> Option<Self> {
-        let start = match range.start_bound() {
-            Bound::Included(n) => IntBound::Included(T::from(*n)?),
-            Bound::Excluded(n) => IntBound::Excluded(T::from(*n)?),
-            Bound::Unbounded => IntBound::Included(T::from(N::min_value())?),
+impl<T> AnyRange<T> {
+    pub fn with_bounds(start: Bound<T>, end: Bound<T>) -> Option<Self> {
+        let result = match (start, end) {
+            (Bound::Included(s), Bound::Excluded(e)) => Self::Range(s..e),
+            (Bound::Included(s), Bound::Unbounded) => Self::RangeFrom(s..),
+            (Bound::Unbounded, Bound::Unbounded) => Self::RangeFull(..),
+            (Bound::Included(s), Bound::Included(e)) => Self::RangeInclusive(s..=e),
+            (Bound::Unbounded, Bound::Excluded(e)) => Self::RangeTo(..e),
+            (Bound::Unbounded, Bound::Included(e)) => Self::RangeToInclusive(..=e),
+            (Bound::Excluded(_), _) => return None,
         };
 
-        let end = match range.end_bound() {
-            Bound::Included(n) => IntBound::Included(T::from(*n)?),
-            Bound::Excluded(n) => IntBound::Excluded(T::from(*n)?),
-            Bound::Unbounded => IntBound::Included(T::from(N::max_value())?),
-        };
-
-        Some(Self { start, end })
+        Some(result)
     }
 }
 
-impl<T: PrimInt + fmt::Display> fmt::Display for IntRange<T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self.start {
-            IntBound::Included(n) => write!(f, "[{n}, ")?,
-            IntBound::Excluded(n) => write!(f, "({n}, ")?,
-        }
+impl<T: PartialOrd<T>> AnyRange<T> {
+    pub fn contains<U>(&self, item: &U) -> bool
+    where
+        T: PartialOrd<U>,
+        U: ?Sized + PartialOrd<T>,
+    {
+        <Self as RangeBounds<T>>::contains(self, item)
+    }
+}
 
-        match self.end {
-            IntBound::Included(n) => write!(f, "{n}]"),
-            IntBound::Excluded(n) => write!(f, "{n})"),
+impl<T: fmt::Debug> fmt::Debug for AnyRange<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Range(r) => r.fmt(f),
+            Self::RangeFrom(r) => r.fmt(f),
+            Self::RangeFull(r) => r.fmt(f),
+            Self::RangeInclusive(r) => r.fmt(f),
+            Self::RangeTo(r) => r.fmt(f),
+            Self::RangeToInclusive(r) => r.fmt(f),
         }
     }
 }
 
+impl<T> RangeBounds<T> for AnyRange<T> {
+    fn start_bound(&self) -> Bound<&T> {
+        match self {
+            Self::Range(r) => r.start_bound(),
+            Self::RangeFrom(r) => r.start_bound(),
+            Self::RangeFull(r) => r.start_bound(),
+            Self::RangeInclusive(r) => r.start_bound(),
+            Self::RangeTo(r) => r.start_bound(),
+            Self::RangeToInclusive(r) => r.start_bound(),
+        }
+    }
+
+    fn end_bound(&self) -> Bound<&T> {
+        match self {
+            Self::Range(r) => r.end_bound(),
+            Self::RangeFrom(r) => r.end_bound(),
+            Self::RangeFull(r) => r.end_bound(),
+            Self::RangeInclusive(r) => r.end_bound(),
+            Self::RangeTo(r) => r.end_bound(),
+            Self::RangeToInclusive(r) => r.end_bound(),
+        }
+    }
+}
+
+impl<T> From<Range<T>> for AnyRange<T> {
+    fn from(value: Range<T>) -> Self {
+        Self::Range(value)
+    }
+}
+
+impl<T> From<RangeFrom<T>> for AnyRange<T> {
+    fn from(value: RangeFrom<T>) -> Self {
+        Self::RangeFrom(value)
+    }
+}
+
+impl<T> From<RangeFull> for AnyRange<T> {
+    fn from(value: RangeFull) -> Self {
+        Self::RangeFull(value)
+    }
+}
+
+impl<T> From<RangeInclusive<T>> for AnyRange<T> {
+    fn from(value: RangeInclusive<T>) -> Self {
+        Self::RangeInclusive(value)
+    }
+}
+
+impl<T> From<RangeTo<T>> for AnyRange<T> {
+    fn from(value: RangeTo<T>) -> Self {
+        Self::RangeTo(value)
+    }
+}
+
+impl<T> From<RangeToInclusive<T>> for AnyRange<T> {
+    fn from(value: RangeToInclusive<T>) -> Self {
+        Self::RangeToInclusive(value)
+    }
+}
+
 /// A non-generic type that can represent any 64-bit or smaller primitive
 /// integer.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum LargeInt {
     Signed(i64),
     Unsigned(u64),
@@ -112,13 +173,13 @@ impl fmt::Display for LargeInt {
 
 /// A non-generic type that can represent any 64-bit or smaller primitive
 /// integer range.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, PartialEq, Eq)]
 pub enum LargeIntRange {
-    Signed(IntRange<i64>),
-    Unsigned(IntRange<u64>),
+    Signed(AnyRange<i64>),
+    Unsigned(AnyRange<u64>),
 }
 
-impl fmt::Display for LargeIntRange {
+impl fmt::Debug for LargeIntRange {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::Signed(r) => r.fmt(f),
@@ -128,8 +189,8 @@ impl fmt::Display for LargeIntRange {
 }
 
 /// An error returned when a value is not within a specific range.
-#[derive(Clone, Copy, Debug, Error)]
-#[error("Integer value {value} not in bounds: {range}")]
+#[derive(Clone, Debug, Error)]
+#[error("Integer value {value} not in bounds: {range:?}")]
 pub struct OutOfBoundsError {
     value: LargeInt,
     range: LargeIntRange,
@@ -138,7 +199,7 @@ pub struct OutOfBoundsError {
 /// Verify that `value` is within `bounds` and then return `value` if it is.
 pub fn check_bounds<T: PrimInt>(
     value: T,
-    bounds: impl RangeBounds<T>,
+    range: impl Into<AnyRange<T>>,
 ) -> Result<T, OutOfBoundsError> {
     const {
         assert!(
@@ -147,7 +208,9 @@ pub fn check_bounds<T: PrimInt>(
         );
     }
 
-    if !bounds.contains(&value) {
+    let range = range.into();
+
+    if !range.contains(&value) {
         let value = if T::min_value() != T::zero() {
             LargeInt::Signed(NumCast::from(value).unwrap())
         } else {
@@ -155,9 +218,33 @@ pub fn check_bounds<T: PrimInt>(
         };
 
         let range = if T::min_value() != T::zero() {
-            LargeIntRange::Signed(IntRange::new(bounds).unwrap())
+            let start = match range.start_bound() {
+                Bound::Excluded(n) => Bound::Excluded(NumCast::from(*n).unwrap()),
+                Bound::Included(n) => Bound::Included(NumCast::from(*n).unwrap()),
+                Bound::Unbounded => Bound::Unbounded,
+            };
+
+            let end = match range.end_bound() {
+                Bound::Excluded(n) => Bound::Excluded(NumCast::from(*n).unwrap()),
+                Bound::Included(n) => Bound::Included(NumCast::from(*n).unwrap()),
+                Bound::Unbounded => Bound::Unbounded,
+            };
+
+            LargeIntRange::Signed(AnyRange::with_bounds(start, end).unwrap())
         } else {
-            LargeIntRange::Unsigned(IntRange::new(bounds).unwrap())
+            let start = match range.start_bound() {
+                Bound::Excluded(n) => Bound::Excluded(NumCast::from(*n).unwrap()),
+                Bound::Included(n) => Bound::Included(NumCast::from(*n).unwrap()),
+                Bound::Unbounded => Bound::Unbounded,
+            };
+
+            let end = match range.end_bound() {
+                Bound::Excluded(n) => Bound::Excluded(NumCast::from(*n).unwrap()),
+                Bound::Included(n) => Bound::Included(NumCast::from(*n).unwrap()),
+                Bound::Unbounded => Bound::Unbounded,
+            };
+
+            LargeIntRange::Unsigned(AnyRange::with_bounds(start, end).unwrap())
         };
 
         return Err(OutOfBoundsError { value, range });
@@ -184,9 +271,15 @@ pub fn try_cast<T: PrimInt, V: PrimInt>(value: V) -> Result<T, OutOfBoundsError>
         };
 
         let range = if T::min_value() != T::zero() {
-            LargeIntRange::Signed(IntRange::new::<T, _>(..).unwrap())
+            let min = NumCast::from(T::min_value()).unwrap();
+            let max = NumCast::from(T::max_value()).unwrap();
+
+            LargeIntRange::Signed((min..=max).into())
         } else {
-            LargeIntRange::Unsigned(IntRange::new::<T, _>(..).unwrap())
+            let min = NumCast::from(T::min_value()).unwrap();
+            let max = NumCast::from(T::max_value()).unwrap();
+
+            LargeIntRange::Unsigned((min..=max).into())
         };
 
         OutOfBoundsError { value, range }
@@ -287,21 +380,27 @@ where
 
 #[cfg(test)]
 mod tests {
-    use assert_matches::assert_matches;
-
     use super::*;
 
     #[test]
-    fn test_int_range() {
-        let range = IntRange::new::<u8, _>(..).unwrap();
-        assert_eq!(range.start, IntBound::Included(u8::MIN));
-        assert_eq!(range.end, IntBound::Included(u8::MAX));
+    fn test_any_range() {
+        let range = AnyRange::with_bounds(Bound::Included(0), Bound::Excluded(1)).unwrap();
+        assert_eq!(range, AnyRange::from(0..1));
 
-        let range = IntRange::<i8>::new(-2i16..2i16).unwrap();
-        assert_eq!(range.start, IntBound::Included(-2));
-        assert_eq!(range.end, IntBound::Excluded(2));
+        let range = AnyRange::with_bounds(Bound::Included(0), Bound::Unbounded).unwrap();
+        assert_eq!(range, AnyRange::from(0..));
 
-        assert!(IntRange::<u8>::new::<u16, _>(..).is_none());
+        let range = AnyRange::<i32>::with_bounds(Bound::Unbounded, Bound::Unbounded).unwrap();
+        assert_eq!(range, AnyRange::from(..));
+
+        let range = AnyRange::with_bounds(Bound::Included(0), Bound::Included(1)).unwrap();
+        assert_eq!(range, AnyRange::from(0..=1));
+
+        let range = AnyRange::with_bounds(Bound::Unbounded, Bound::Excluded(1)).unwrap();
+        assert_eq!(range, AnyRange::from(..1));
+
+        let range = AnyRange::with_bounds(Bound::Unbounded, Bound::Included(1)).unwrap();
+        assert_eq!(range, AnyRange::from(..=1));
     }
 
     #[test]
@@ -313,24 +412,12 @@ mod tests {
         check_bounds(0, -1..=1).unwrap();
 
         let err = check_bounds(i8::MAX, 0..=0).unwrap_err();
-        assert_matches!(err.value, LargeInt::Signed(127));
-        assert_matches!(
-            err.range,
-            LargeIntRange::Signed(IntRange {
-                start: IntBound::Included(0),
-                end: IntBound::Included(0),
-            })
-        );
+        assert_eq!(err.value, LargeInt::Signed(127));
+        assert_eq!(err.range, LargeIntRange::Signed(AnyRange::from(0..=0)));
 
         let err = check_bounds(u8::MAX, 0..=0).unwrap_err();
-        assert_matches!(err.value, LargeInt::Unsigned(255));
-        assert_matches!(
-            err.range,
-            LargeIntRange::Unsigned(IntRange {
-                start: IntBound::Included(0),
-                end: IntBound::Included(0),
-            })
-        );
+        assert_eq!(err.value, LargeInt::Unsigned(255));
+        assert_eq!(err.range, LargeIntRange::Unsigned(AnyRange::from(0..=0)));
     }
 
     #[test]
@@ -339,14 +426,8 @@ mod tests {
         assert_eq!(value, 255);
 
         let err = try_cast::<i8, _>(256u16).unwrap_err();
-        assert_matches!(err.value, LargeInt::Unsigned(256));
-        assert_matches!(
-            err.range,
-            LargeIntRange::Signed(IntRange {
-                start: IntBound::Included(-128),
-                end: IntBound::Included(127),
-            })
-        );
+        assert_eq!(err.value, LargeInt::Unsigned(256));
+        assert_eq!(err.range, LargeIntRange::Signed(AnyRange::from(-128..=127)));
     }
 
     #[test]

deny.toml

@@ -33,6 +33,7 @@ allow = [
     "Apache-2.0",
     "Apache-2.0 WITH LLVM-exception",
     "BSD-3-Clause",
+    "bzip2-1.0.6",
     "GPL-3.0",
     "ISC",
     "MIT",
@@ -68,6 +69,6 @@ bypass = [
 unknown-registry = "deny"
 unknown-git = "deny"
 allow-git = [
-    "https://github.com/chenxiaolong/bzip2-rs",
     "https://github.com/chenxiaolong/zip",
+    "https://github.com/trifectatechfoundation/bzip2-rs",
 ]

e2e/src/main.rs

@@ -755,7 +755,7 @@ fn create_ota(
 
         match path {
             ota::PATH_OTACERT => {
-                crypto::write_pem_cert(&mut writer, cert_ota)
+                crypto::write_pem_cert(Path::new(path), &mut writer, cert_ota)
                     .with_context(|| format!("Failed to write entry: {path}"))?;
             }
             ota::PATH_PAYLOAD => {