Commit graph

4246 commits

Author SHA1 Message Date
Jeff Vander Stoep
2c347c75c1 OWNERS: add alan and jeff, alphabetize
Test: n/a
Change-Id: I3b9f18906b72d0a3aed712f6c6990180294d0d2b
2020-12-10 13:08:28 +01:00
Treehugger Robot
12eb9de463 Merge "Follow vdc naming convention: earlyBootEnded" am: e8838a862d
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1512480

Change-Id: I7df00e8936fd2185541645594a9633317632fe99
2020-12-02 08:20:27 +00:00
Treehugger Robot
e8838a862d Merge "Follow vdc naming convention: earlyBootEnded" 2020-12-02 07:49:38 +00:00
Paul Crowley
ed06b3eabe Follow vdc naming convention: earlyBootEnded
vdc commands use camelCase, not kebab-case.

Test: EarlyBootKeyTest.CannotCreateEarlyBootKeys
Change-Id: I7be4d3008a731829e5d5e025216cb2ade238a530
2020-12-01 14:36:06 -08:00
Xin Li
2d717e91f1 Skip rvc-qpr-dev-plus-aosp-without-vendor@6881855
Bug: 172690556
Merged-In: I51672944372d24483679d6f81df4e80869d3fd99
Change-Id: Ib64d7c9c68de9058bae79d0de9f9c59341d207c3
2020-11-23 16:26:04 -08:00
Martijn Coenen
00382980e5 Merge "Convert to lower fs path for createObb()." am: c237cbc575
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1505371

Change-Id: Ib2cacb3602bc21c5e6d03e15337c188ae2f7bdd5
2020-11-19 17:48:44 +00:00
Martijn Coenen
5b5083b8a9 Merge "Unmount pass_through path last." am: 2fb2757c2d
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1505131

Change-Id: I16944515f12a656d9b6a2da23a04b7615c9e2f1a
2020-11-19 17:48:36 +00:00
Martijn Coenen
c237cbc575 Merge "Convert to lower fs path for createObb()." 2020-11-19 17:31:54 +00:00
Martijn Coenen
2fb2757c2d Merge "Unmount pass_through path last." 2020-11-19 17:31:45 +00:00
Martijn Coenen
d6a612ac20 Convert to lower fs path for createObb().
Since /storage/emulated/userId isn't accessible for users != userId,
and vold should anyway try to avoid accessing the FUSE filesystem itself.

Bug: 172078780
Test: atest StorageManagerTest --user-type secondary_user
Change-Id: I98222bf844a6b7d8ec0d9873eddc71f61aa68c90
2020-11-19 15:27:55 +00:00
Martijn Coenen
64b3bba52e Unmount pass_through path last.
There've been reports of issues where, when a volume is ejected, the
MediaProvider process gets killed. This happens because the
MediaProvider has a file open on the volume (eg, during a scan). We do
abort the scan when the volume is ejected, however this could take some
time. So, we give MediaProvider a bit more time before getting killed,
by only looking for files open on the pass_through paths last. This
order anyway seems to make more sense - ideally we kill apps using
external storage before we unmount the pass_through path underlying it.

Bug: 171367622
Test: atets AdoptableHostTest
Change-Id: Ie8eacaa72a80ff8161ecf1e8c0243afcd890ee39
2020-11-19 09:08:50 +01:00
Martijn Coenen
87869c2b77 Merge "Call earlyBootEnded from vdc." am: 17ebcf7f99
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1496221

Change-Id: Ib084a4c16c790e274232fd4056b3af4b0e39fff7
2020-11-18 08:09:07 +00:00
Martijn Coenen
17ebcf7f99 Merge "Call earlyBootEnded from vdc." 2020-11-18 07:51:31 +00:00
Eric Biggers
79f6bce6ba Merge "Switch to exfatprogs compatible fsck parameter" am: dfd36fe6b6
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1441937

Change-Id: Ibd14595638ab5daf7965043d64cc5c06dddd1b7c
2020-11-12 17:48:16 +00:00
Eric Biggers
dfd36fe6b6 Merge "Switch to exfatprogs compatible fsck parameter" 2020-11-12 17:33:16 +00:00
Martijn Coenen
eed957f6a4 Call earlyBootEnded from vdc.
This allows us to determine the place where early boot ends from init.
It also allows fixing a bug where early boot wasn't ended previously on
devices without metadata encryption.

Bug: 168585635
Bug: 173005594
Test: inspect logs
Change-Id: I78775672a7d3c140e007235a10fb1d1bc816fcee
2020-11-12 11:03:27 +01:00
LuK1337
b8e07b20ad Switch to exfatprogs compatible fsck parameter
exfatprogs accepts 'y' for no interaction repair.

Change-Id: I2c436816a293a36fc9f0cd635cdb9ca3b5f88bfc
2020-11-11 19:45:05 +01:00
Eric Biggers
689b4e7110 Merge "KeyStorage: rework key upgrade handling" am: d5de2f22b7
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1483694

Change-Id: Ib6c052565b3fe79eeb928daa564a7431f89aed22
2020-11-10 01:11:13 +00:00
Eric Biggers
d5de2f22b7 Merge "KeyStorage: rework key upgrade handling" 2020-11-10 00:39:24 +00:00
Eric Biggers
f74373b177 KeyStorage: rework key upgrade handling
Remove the error-prone 'keepOld' parameter, and instead make begin()
(renamed to BeginKeymasterOp()) do all the key upgrade handling.

Don't handle /data and /metadata differently anymore.  Previously, when
a checkpoint is active, key blob files were replaced on /data
immediately; only the actual Keymaster key deletion was delayed until
checkpoint commit.  But it's easier to just delay the key blob file
replacement too, as we have to implement that for /metadata anyway.

Also be more vigilant about deleting any leftover upgraded keys.

Test: Tested on bramble using an OTA rvc-d1-release => master.  In OTA
      success case, verified via logcat that the keys were upgraded and
      then were committed after the boot succeeded.  In OTA failure
      case, verified that the device still boots -- i.e., the old keys
      weren't lost.  Verified that in either case, no
      keymaster_key_blob_upgraded files were left over.  Finally, also
      tried 'pm create-user' and 'pm remove-user' and verified via
      logcat that the Keymaster keys still get deleted.
Change-Id: Ic9c3e63e0bcae0c608fc79050ca4a1676b3852ee
2020-11-05 19:58:26 -08:00
Eric Biggers
22f226245a Merge "EncryptInplace: fsync cryptofd before reporting success" am: e244a15f34
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1488376

Change-Id: I10ff2f5cccf00fbd3cbac93059ce7f069911e9c4
2020-11-05 19:09:37 +00:00
Eric Biggers
e244a15f34 Merge "EncryptInplace: fsync cryptofd before reporting success" 2020-11-05 18:51:13 +00:00
Eric Biggers
1ba8865fec EncryptInplace: fsync cryptofd before reporting success
fsync() the cryptofd when done writing to it.  Without this, any
remaining dirty pages in the crypto_blkdev's page cache (which there
might be a lot of, even as much as all the data that was written) won't
be flushed to disk until the cryptofd is closed, which ignores I/O
errors and is also after we already reported 100% completion.

There wasn't an fsync() in the original version either, so we've been
getting by without it, but it seems it should be there.

Change-Id: Idd1be3ae67ce96ecf3946b9efb9fc57414f5805a
2020-11-04 19:24:19 -08:00
Eric Biggers
ee175c954a Merge changes from topic "encryptinplace-cleanup" am: 91e4f1dd76
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1467990

Change-Id: I50e47bd1cb102b9013542b0676258a79ac44b6b1
2020-11-04 19:15:05 +00:00
Eric Biggers
91e4f1dd76 Merge changes from topic "encryptinplace-cleanup"
* changes:
  Refactor EncryptInplace.cpp
  Correctly calculate tot_used_blocks on ext4 with uninit_bg
  Fix memory leak of f2fs_info
  Remove special handling for missing crypto_blkdev
  Check return value of create_crypto_blk_dev()
  Remove unused support for partial encryption
2020-11-04 18:47:05 +00:00
Eric Biggers
f038c5f5e1 Refactor EncryptInplace.cpp
Refactor EncryptInplace.cpp to simplify and improve it a lot.  This is
everything that didn't fit into prior commits, including:

- Share a lot more code between ext4, f2fs, and full encryption.

- Improve the log messages.  Most importantly, don't spam the log with
  huge numbers of messages, and don't log errors in expected cases.
  Note: generate_f2fs_info() is still too noisy, but that's part of
  "system/extras", not vold, so this change doesn't change that.

- When possible, do 32K reads/writes for f2fs and for full encryption,
  not just for ext4.  This might improve performance.

- Take advantage of C++ functionality.

- Be more careful about edge cases.  E.g. if the calculation of the
  number of blocks to encrypt was wrong, don't set vold.encrypt_progress
  to > 99 until we're actually done.

The net change is over 200 lines removed.

Before-after comparison of log when enabling metadata encryption:

ext4 before:
    I vold    : Beginning inplace encryption, nr_sec: 16777216
    D vold    : cryptfs_enable_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, 0)
    D vold    : Opening/dev/block/by-name/userdata
    D vold    : Opening/dev/block/dm-8
    I vold    : Encrypting ext4 filesystem in place...
    [omitted 6387 log messages]
    I vold    : Encrypted to sector 822084608
    D vold    : cryptfs_enable_inplace_ext4 success
    I vold    : Inplace encryption complete

ext4 after:
    D vold    : encrypt_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, false)
    D vold    : ext4 filesystem has 64 block groups
    I vold    : Encrypting ext4 filesystem on /dev/block/by-name/userdata in-place via /dev/block/dm-8
    I vold    : 50327 blocks (206 MB) of 2097152 blocks are in-use
    D vold    : Encrypted 10000 of 50327 blocks
    D vold    : Encrypted 20000 of 50327 blocks
    D vold    : Encrypted 30000 of 50327 blocks
    D vold    : Encrypted 40000 of 50327 blocks
    D vold    : Encrypted 50000 of 50327 blocks
    D vold    : Encrypted 50327 of 50327 blocks
    I vold    : Successfully encrypted ext4 filesystem on /dev/block/by-name/userdata

f2fs before:
    I vold    : Beginning inplace encryption, nr_sec: 16777216
    D vold    : cryptfs_enable_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, 0)
    D vold    : Opening/dev/block/by-name/userdata
    D vold    : Opening/dev/block/dm-8
    E vold    : Reading ext4 extent caused an exception
    D vold    : cryptfs_enable_inplace_ext4()=-1
    [omitted logspam from f2fs_sparseblock]
    I vold    : Encrypting from block 0
    I vold    : Encrypted to block 15872
    I vold    : Encrypting from block 16384
    I vold    : Encrypted to block 16385
    I vold    : Encrypting from block 17408
    I vold    : Encrypted to block 17412
    D vold    : cryptfs_enable_inplace_f2fs success
    I vold    : Inplace encryption complete

f2fs after:
    D vold    : encrypt_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, false)
    [omitted logspam from f2fs_sparseblock]
    I vold    : Encrypting f2fs filesystem on /dev/block/by-name/userdata in-place via /dev/block/dm-8
    I vold    : 15880 blocks (65 MB) of 2097152 blocks are in-use
    D vold    : Encrypted 10000 of 15880 blocks
    D vold    : Encrypted 15880 of 15880 blocks
    I vold    : Successfully encrypted f2fs filesystem on /dev/block/by-name/userdata

Test: Booted Cuttlefish with metadata encryption enabled and with the
      userdata filesystem using (1) ext4, (2) f2fs, and (3) f2fs but
      with EncryptInplace.cpp patched to not recognize the filesystem
      and fall back to the "full" encryption case.  Checked that the log
      messages were as expected and that /data was mounted.

      I've had no luck testing FDE yet; it doesn't work even without
      these changes.  Suggestions appreciated...

Change-Id: I08fc8465f7962abd698904b5466f3ed080d53953
2020-11-03 14:16:32 -08:00
Eric Biggers
7e70d6939d Correctly calculate tot_used_blocks on ext4 with uninit_bg
The calculated number of blocks to encrypt is too high on ext4
filesystems that have the uninit_bg feature.  This is because the
calculation assumes that all blocks not counted in bg_free_blocks_count
need to encrypted.  But actually, uninitialized block groups have inode
blocks which vold doesn't encrypt since they are uninitialized, but they
are "allocated" and thus reduce bg_free_blocks_count.

Therefore, add a helper function num_base_meta_blocks_in_group() which
returns the number of blocks to encrypt in an uninitialized block group.
Use it both for the encryption and for calculating 'tot_used_blocks'.

Also compute 'tot_used_blocks' additively rather than subtractively, as
this is easier to understand.

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: I4d2cb40291da67dd1bafd61289ccb9e6343bfda3
2020-11-03 14:11:01 -08:00
Eric Biggers
b3ba087d9c Fix memory leak of f2fs_info
'struct f2fs_info' from system/extras/f2fs_utils is supposed to be
freed using free_f2fs_info(), not just free().

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: If6e75e5c604b40be24538b156a37cc76f4f0d4f7
2020-11-03 14:11:01 -08:00
Eric Biggers
69520d2d39 Remove special handling for missing crypto_blkdev
This logic is no longer necessary, since the code that creates the
crypto_blkdev (create_crypto_blk_dev() in MetadataCrypt.cpp or in
cryptfs.cpp) now waits for the block device to appear before continuing.

It's also worth noting that the retry loop was only present for ext4,
not for f2fs, yet most Android devices are using f2fs these days.

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: I173ca6cc187a810e008990dfa22aede58632db25
2020-11-03 14:11:01 -08:00
Eric Biggers
88f993b4a8 Check return value of create_crypto_blk_dev()
cryptfs_enable_internal() forgot to check the return value of
create_crypto_blk_dev(), so it was continuing to
cryptfs_enable_inplace() when creating the dm-crypt device failed, which
doesn't make sense.

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: If9f20069d0f084150aa887a350f7c0c31a6d80f2
2020-11-03 14:11:00 -08:00
Eric Biggers
c01995ea3b Remove unused support for partial encryption
Commit 87999173dd ("Don't corrupt ssd when encrypting and power
fails") added a lot of code to handle pausing in-place conversion from
unencrypted => FDE when the battery was low, and resuming it later.

It was eventually decided that this wasn't needed, and commit
7e17e2d226 ("Don't worry about battery levels when encrypting")
removed the checks for low battery.

This made the partial encryption code unused.  So remove it.

Note that this was cluttering up the metadata encryption code too, since
EncryptInplace.cpp is now shared by both FDE and metadata encryption.

Bug: 16868177
Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: Ibd2eb08a2aa15938097abcb8a67b5a813c4d76c7
2020-11-03 14:11:00 -08:00
Eric Biggers
5a9feb48fa Merge changes I8d2bd67d,I704522b2 am: 27f3ab89d0
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1484498

Change-Id: I28ef08b0fe9b3c94b02769f43e21f574e524da57
2020-11-03 17:48:09 +00:00
Eric Biggers
27f3ab89d0 Merge changes I8d2bd67d,I704522b2
* changes:
  FsCrypt: silently skip "." and ".." when loading keys
  Utils: add IsDotOrDotDot() and use it in the appropriate places
2020-11-03 17:02:46 +00:00
Eric Biggers
1d692f3b7b Merge "KeyUtil: don't use keepOld=true for system DE and volume keys" am: 4a969dba60
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1480696

Change-Id: I45f11f64c84b540a27cdc39af493a2ee4e552bce
2020-11-03 00:08:56 +00:00
Eric Biggers
6b84039847 FsCrypt: silently skip "." and ".." when loading keys
Avoid logging useless messages like:

    D vold    : Skipping non-key .
    D vold    : Skipping non-key ..
    D vold    : Skipping non-de-key .
    D vold    : Skipping non-de-key ..

Change-Id: I8d2bd67d554605a5ab9faadd3730870dfe0881f6
2020-11-02 15:47:42 -08:00
Eric Biggers
7bcf427369 Utils: add IsDotOrDotDot() and use it in the appropriate places
Change-Id: I704522b26acfb3e7c423d9a14d69ede513b50482
2020-11-02 15:47:24 -08:00
Eric Biggers
4a969dba60 Merge "KeyUtil: don't use keepOld=true for system DE and volume keys" 2020-11-02 23:31:23 +00:00
Eric Biggers
c493903732 KeyUtil: don't use keepOld=true for system DE and volume keys
Commit 77df7f207d / http://aosp/1217657 ("Refactor to use
EncryptionPolicy everywhere we used to use raw_ref") unintentionally
made fscrypt_initialize_systemwide_keys() start specifying keepOld=true
(via default parameter value) when retrieving the system DE key, and
likewise for read_or_create_volkey() and volume keys.

As a result, if the associated Keymaster key needs to be upgraded, the
upgraded key blob gets written to "keymaster_key_blob_upgraded", but it
doesn't replace the original "keymaster_key_blob", nor is the original
key deleted from Keymaster.  This happens at every boot, eventually
resulting in the RPMB partition in Keymaster becoming full.

Only the metadata encryption key ever needs keepOld=true, since it's the
only key that isn't stored in /data, and the purpose of keepOld=true is
to allow a key that isn't stored in /data to be committed or rolled back
when a userdata checkpoint is committed or rolled back.

So, fix this bug by removing the default value of keepOld, and
specifying false everywhere except the metadata encryption key.

Note that when an affected device gets this fix, it will finally upgrade
its system DE key correctly.  However, this fix doesn't free up space in
Keymaster that was consumed by this bug.

Test: On bramble:
  - Flashed rvc-d1-dev build, with wiping userdata
  - Flashed a newer build, without wiping userdata
  - Log expectedly shows key upgrades:
        $ adb logcat | grep 'Upgrading key'
        D vold    : Upgrading key: /metadata/vold/metadata_encryption/key
        D vold    : Upgrading key: /data/unencrypted/key
        D vold    : Upgrading key: /data/misc/vold/user_keys/de/0
        D vold    : Upgrading key: /data/misc/vold/user_keys/ce/0/current
  - Rebooted
  - Log unexpectedly shows the system DE key being upgraded again:
        $ adb logcat | grep 'Upgrading key'
        D vold    : Upgrading key: /data/unencrypted/key
  - "keymaster_key_blob_upgraded" unexpectedly still exists:
        $ adb shell find /data /metadata -name keymaster_key_blob_upgraded
        /data/unencrypted/key/keymaster_key_blob_upgraded
  - Applied this fix and flashed, without wiping userdata
  - Log shows system DE key being upgraded (expected because due to the
    bug, the upgraded key didn't replace the original one before)
        $ adb logcat | grep 'Upgrading key'
        D vold    : Upgrading key: /data/unencrypted/key
  - "keymaster_key_blob_upgraded" expectedly no longer exists
        $ adb shell find /data /metadata -name keymaster_key_blob_upgraded
  - Rebooted
  - Log expectedly doesn't show any more key upgrades
        $ adb logcat | grep 'Upgrading key'
Bug: 171944521
Bug: 172019387
Change-Id: I42d3f5fbe32cb2ec229f4b614cfb271412a3ed29
2020-10-30 14:53:43 -07:00
Eric Biggers
524e094991 Merge "Fix argument type for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS" am: 569fb8365e
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1472321

Change-Id: I5cb2651ee48ab2a9afb9ec5ded8259cd006ecbfe
2020-10-28 16:26:09 +00:00
Eric Biggers
569fb8365e Merge "Fix argument type for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS" 2020-10-28 16:16:26 +00:00
Eric Biggers
f9d9ac29d0 Fix argument type for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS
These ioctls take a pointer to an 'int' (or an 'unsigned int', it
doesn't matter), not an 'unsigned long'.  See 'man ioctl_iflags'.
Presumably it happened to work anyway because Android only runs on
little endian platforms.

Bug: 146419093
Bug: 163453310
Change-Id: I73099dafd4ee8d497c0a754149271871a37454f6
Signed-off-by: Eric Biggers <ebiggers@google.com>
2020-10-28 04:48:09 +00:00
Yo Chiang
3ae0c6737c Merge "Add IVold::destroyDsuMetadataKey()" am: cb581cc8de
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1452395

Change-Id: I6241d71e331eebe0222696a052e40d2222a9c537
2020-10-26 05:49:35 +00:00
Yo Chiang
cb581cc8de Merge "Add IVold::destroyDsuMetadataKey()" 2020-10-26 05:27:36 +00:00
Treehugger Robot
19adff5f27 Merge "Set media folder +F for adopted storage as well" am: 739ca2c298
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1397560

Change-Id: I90d0e56e260094661217dd20135f9fde6da96e98
2020-10-21 23:12:51 +00:00
Treehugger Robot
739ca2c298 Merge "Set media folder +F for adopted storage as well" 2020-10-21 22:04:11 +00:00
Daniel Rosenberg
cc874804dd Set media folder +F for adopted storage as well
We previously only set +F for /data/media, but adopted storage needs
this as well. Instead we add support for adding attrs to PrepareDir.

Bug: 163453310
Test: sm set-virtual-disk true
      follow UI setup and confirm +F on /mnt/expand/*/media
Change-Id: I08f13b57a4de3538e88b38eb95b0ac115a5a5ce8
Merged-In: I08f13b57a4de3538e88b38eb95b0ac115a5a5ce8
2020-10-20 18:55:54 -07:00
Eric Biggers
b6d1a31949 Merge "vold: Generate storage key without rollback resistance" am: 4f5e9c196f
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1426109

Change-Id: Ibf06476010946d799c4b1fa5143ed4eac96f301a
2020-10-20 23:45:41 +00:00
Eric Biggers
4f5e9c196f Merge "vold: Generate storage key without rollback resistance" 2020-10-20 23:17:22 +00:00
Gaurav Kashyap
75736a8811 vold: Generate storage key without rollback resistance
Generate a storage key without rollback_resistance when device doesnt
support the corresponding tag.

Bug: 168527558

Change-Id: Iaf27c64dba627a31c9cbd9178458bf6785d00251
2020-10-20 16:19:33 +00:00
Eric Biggers
8671044a64 Merge changes Idc575106,Id6457a2b am: 3e0e53dbe5
Original change: https://android-review.googlesource.com/c/platform/system/vold/+/1462712

Change-Id: If01239e184cd19de660fa158d599c9826ea0cf71
2020-10-19 16:26:21 +00:00